Looks good to me.
Reviewed-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
On Mon, Feb 22, 2021 at 9:21 AM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
> Both patches look good. Thanks!
>
> Reviewed-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
> --
> Best regards,
> Pavel Shilovsky
>
> вс, 21 февр. 2021 г. в 22:44, Rohith <rohiths.msft@xxxxxxxxx>:
> >
> > Thanks Steve. Will make sure to run checkpatch for further patches.
> >
> >
> >
> > Regards,
> >
> > Rohith
> >
> >
> >
> > Sent from Mail for Windows 10
> >
> >
> >
> > From: Steve French
> > Sent: Sunday, February 21, 2021 6:26 AM
> > To: Pavel Shilovsky
> > Cc: Rohith; linux-cifs@xxxxxxxxxxxxxxx; Shyam Prasad N; sribhat.msa@xxxxxxxxxxx
> > Subject: Re: TCON reconnect during STATUS_NETWORK_NAME_DELETED
> >
> >
> >
> > The remaining suggestion of Pavel's is included in this trivial cleanup.
> >
> >
> >
> > Trivial change to clarify code in smb2_is_network_name_deleted
> >
> > Suggested-by: Pavel Shilovsky <pshilov@xxxxxxxxxxxxx>
> > Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
> > ---
> > fs/cifs/smb2ops.c | 29 +++++++++++++++--------------
> > 1 file changed, 15 insertions(+), 14 deletions(-)
> >
> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > index aee9da88a333..b2191babce26 100644
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -2459,23 +2459,24 @@ smb2_is_network_name_deleted(char *buf, struct TCP_Server_Info *server)
> > struct cifs_ses *ses;
> > struct cifs_tcon *tcon;
> >
> > - if (shdr->Status == STATUS_NETWORK_NAME_DELETED) {
> > - spin_lock(&cifs_tcp_ses_lock);
> > - list_for_each(tmp, &server->smb_ses_list) {
> > - ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
> > - list_for_each(tmp1, &ses->tcon_list) {
> > - tcon = list_entry(tmp1, struct cifs_tcon, tcon_list);
> > - if (tcon->tid == shdr->TreeId) {
> > - tcon->need_reconnect = true;
> > - spin_unlock(&cifs_tcp_ses_lock);
> > - pr_warn_once("Server share %s deleted.\n",
> > - tcon->treeName);
> > - return;
> > - }
> > + if (shdr->Status != STATUS_NETWORK_NAME_DELETED)
> > + return;
> > +
> > + spin_lock(&cifs_tcp_ses_lock);
> > + list_for_each(tmp, &server->smb_ses_list) {
> > + ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
> > + list_for_each(tmp1, &ses->tcon_list) {
> > + tcon = list_entry(tmp1, struct cifs_tcon, tcon_list);
> > + if (tcon->tid == shdr->TreeId) {
> > + tcon->need_reconnect = true;
> > + spin_unlock(&cifs_tcp_ses_lock);
> > + pr_warn_once("Server share %s deleted.\n",
> > + tcon->treeName);
> > + return;
> > }
> > }
> > - spin_unlock(&cifs_tcp_ses_lock);
> > }
> > + spin_unlock(&cifs_tcp_ses_lock);
> > }
> >
> > static int
> >
> >
> >
> > On Sat, Feb 20, 2021 at 5:38 PM Steve French <smfrench@xxxxxxxxx> wrote:
> >
> > I corrected various checkpatch errors - including two of those Pavel noted, and tentatively merged into cifs-2.6.git for-next. See attached updated patch.
> >
> >
> >
> > Will fix the indentation next.
> >
> >
> >
> > On Sat, Feb 20, 2021 at 12:18 PM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
> >
> > чт, 18 февр. 2021 г. в 08:10, Rohith <rohiths.msft@xxxxxxxxx>:
> > >
> > > Hi Pavel,
> > >
> > >
> > >
> > > Addressed review comments. Can you please take a look.
> > >
> > >
> > >
> > > >> Btw, I think is_status_io timeout should be called in this loop for
> > >
> > > >> every mid not outside the loop (sorry, missed that in the original
> > >
> > > >> review). Could you please fix that separately?
> > >
> > >
> > >
> > > Yes, will send out different patch for status_io_timeout.
> >
> > Thanks!
> >
> > The patch looks good. Please find my minor comments below:
> >
> > 1.
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 10fe6d6d2dee..b2f51546c2ef 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -993,6 +993,10 @@ cifs_demultiplex_thread(void *p)
> > if (mids[i] != NULL) {
> > mids[i]->resp_buf_size = server->pdu_size;
> >
> > + if (server->ops->is_network_name_deleted) {
> > + server->ops->is_network_name_deleted(bufs[i],
> > + server);
> > + }
> >
> > ^^^
> > remove extra {}
> >
> > 2.
> > +static void
> > +smb2_is_network_name_deleted(char *buf, struct TCP_Server_Info *server)
> > +{
> > + struct smb2_sync_hdr *shdr = (struct smb2_sync_hdr *)buf;
> >
> > + struct list_head *tmp, *tmp1;
> > + struct cifs_ses *ses;
> > + struct cifs_tcon *tcon;
> > +
> > + if (shdr->Status == STATUS_NETWORK_NAME_DELETED) {
> >
> > ^^^
> > let's do the opposite check: if status is not
> > STATUS_NETWORK_NAME_DELETED then return immediately from the function.
> > This will remove overall indentation for the nested for loops.
> >
> > 3.
> > @@ -4605,6 +4632,10 @@ static void smb2_decrypt_offload(struct
> > work_struct *work)
> > #ifdef CONFIG_CIFS_STATS2
> > mid->when_received = jiffies;
> > #endif
> > + if (dw->server->ops->is_network_name_deleted) {
> > + dw->server->ops->is_network_name_deleted(dw->buf,
> > + dw->server);
> > + }
> >
> > ^^^
> > remove extra {}
> >
> > --
> > Best regards,
> > Pavel Shilovsky
> >
> >
> >
> >
> > --
> >
> > Thanks,
> >
> > Steve
> >
> >
> >
> >
> > --
> >
> > Thanks,
> >
> > Steve
> >
> >
--
Regards,
Shyam
