Understood. Here's the revised version, without the unnecessary line change.
On Wed, Feb 10, 2021 at 5:17 PM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
> Thanks for making the changes. Please avoid unnecessary changes like:
>
> if (server->tcpStatus == CifsNeedReconnect
> - || server->tcpStatus == CifsExiting)
> + || server->tcpStatus == CifsExiting)
> return;
>
> as they don't bring any logic but complicate further backporting.
>
> Other than that the patch and the whole series look good:
>
> Reviewed-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
> --
> Best regards,
> Pavel Shilovsky
>
> вт, 9 февр. 2021 г. в 22:09, Shyam Prasad N <nspmangalore@xxxxxxxxx>:
>
>
> >
> > Hi Pavel,
> >
> > Thanks for the review. Attaching updated patch.
> >
> > Regards,
> > Shyam
> >
> > On Tue, Feb 9, 2021 at 12:05 PM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
> > >
> > > 108 if (server->tcpStatus == CifsNeedReconnect
> > > 109 || server->tcpStatus == CifsExiting)
> > > 110 return;
> > >
> > > If you remove this check then the dmesg log may be populated by below
> > > VFS logs. THe intent of this check was to avoid printing them because
> > > they don't make sense in reconnect/umount situations and may confuse
> > > users.
> > >
> > > 111
> > > 112 switch (rc) {
> > > 113 case -1:
> > > 114 /* change_conf hasn't been executed */
> > > 115 break;
> > > 116 case 0:
> > > 117 cifs_server_dbg(VFS, "Possible client or server
> > > bug - zero credits\n");
> > > 118 break;
> > > 119 case 1:
> > > 120 cifs_server_dbg(VFS, "disabling echoes and oplocks\n");
> > > 121 break;
> > > 122 case 2:
> > > 123 cifs_dbg(FYI, "disabling oplocks\n");
> > > 124 break;
> > > 125 default:
> > > 126 trace_smb3_add_credits(server->CurrentMid,
> > > 127 server->hostname, rc, add);
> > > 128 cifs_dbg(FYI, "%s: added %u credits total=%d\n",
> > > __func__, add, rc);
> > > 129 }
> > >
> > >
> > > I would also suggest to rename sin_flight to just "in_flight" :)
> > >
> > > --
> > > Best regards,
> > > Pavel Shilovsky
> > >
> > > сб, 6 февр. 2021 г. в 20:18, Shyam Prasad N <nspmangalore@xxxxxxxxx>:
> > > >
> > > > Forgot to attach again. :)
> > > >
> > > > On Sat, Feb 6, 2021 at 8:18 PM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
> > > > >
> > > > > Here's an updated version with some formatting changes per checkpatch.pl.
> > > > > @Pavel Shilovsky @ronnie sahlberg Hoping that one of you can review
> > > > > this. Particularly the above point.
> > > > >
> > > > > Regards,
> > > > > Shyam
> > > > >
> > > > > On Thu, Feb 4, 2021 at 12:13 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
> > > > > >
> > > > > > @@ -97,17 +99,25 @@ smb2_add_credits(struct TCP_Server_Info *server,
> > > > > > - if (server->tcpStatus == CifsNeedReconnect
> > > > > > - || server->tcpStatus == CifsExiting)
> > > > > > - return;
> > > > > >
> > > > > > @Pavel Shilovsky This check prevented a tracepoint from getting
> > > > > > printed. I do not see much value in these lines, since all we do is
> > > > > > print the tracepoint and exit. Hence removing it. Please let me know
> > > > > > if that is not okay.
> > > > > >
> > > > > > On Thu, Feb 4, 2021 at 12:09 AM Shyam Prasad N <nspmangalore@xxxxxxxxx> wrote:
> > > > > > >
> > > > > > > --
> > > > > > > Regards,
> > > > > > > Shyam
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Regards,
> > > > > > Shyam
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Regards,
> > > > > Shyam
> > > >
> > > >
> > > >
> > > > --
> > > > Regards,
> > > > Shyam
> >
> >
> >
> > --
> > Regards,
> > Shyam
--
Regards,
Shyam
Attachment:
0003-cifs-Identify-a-connection-by-a-conn_id.patch
Description: Binary data
