Re: [PATCH 11/14] cifs: handle when server starts supporting multichannel

Shyam Prasad N
2023-11-04T07:50:53Z

On Wed, Nov 1, 2023 at 9:22 PM Paulo Alcantara <pc@manguebit.com> wrote:
>
> nspmangalore@gmail.com writes:
>
> > From: Shyam Prasad N <sprasad@microsoft.com>
> >
> > When the user mounts with multichannel option, but the
> > server does not support it, there can be a time in future
> > where it can be supported.
> >
> > With this change, such a case is handled.
> >
> > Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> > ---
> > fs/smb/client/cifsproto.h | 4 ++++
> > fs/smb/client/connect.c | 6 +++++-
> > fs/smb/client/smb2pdu.c | 31 ++++++++++++++++++++++++++++---
> > 3 files changed, 37 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> > index 65c84b3d1a65..5a4c1f1e0d91 100644
> > --- a/fs/smb/client/cifsproto.h
> > +++ b/fs/smb/client/cifsproto.h
> > @@ -132,6 +132,10 @@ extern int SendReceiveBlockingLock(const unsigned int xid,
> > struct smb_hdr *in_buf,
> > struct smb_hdr *out_buf,
> > int *bytes_returned);
> > +
> > +void
> > +smb2_query_server_interfaces(struct work_struct *work);
> > +
>
> Why are you exporting this? smb2_query_server_interfaces() seems to be
> used only in fs/smb/client/connect.c.

In an earlier version of this change, I was calling
smb2_query_server_interfaces from smb2_reconnect when multichannel is
reenabled.
That needed the export. However, I had to change that.
I will reset this back.

>
> > void
> > cifs_signal_cifsd_for_reconnect(struct TCP_Server_Info *server,
> > bool all_channels);
> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > index e71aa33bf026..149cde77500e 100644
> > --- a/fs/smb/client/connect.c
> > +++ b/fs/smb/client/connect.c
> > @@ -116,7 +116,8 @@ static int reconn_set_ipaddr_from_hostname(struct TCP_Server_Info *server)
> > return rc;
> > }
> >
> > -static void smb2_query_server_interfaces(struct work_struct *work)
> > +void
> > +smb2_query_server_interfaces(struct work_struct *work)
> > {
>
> Ditto.
>
> > int rc;
> > int xid;
> > @@ -134,6 +135,9 @@ static void smb2_query_server_interfaces(struct work_struct *work)
> > if (rc) {
> > cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n",
> > __func__, rc);
> > +
> > + if (rc == -EOPNOTSUPP)
> > + return;
>
> Maybe also get rid of cifs_dbg() when rc == -EOPNOTSUPP?
>
> > }
> >
> > queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
> > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > index b7665155f4e2..2617437a4627 100644
> > --- a/fs/smb/client/smb2pdu.c
> > +++ b/fs/smb/client/smb2pdu.c
> > @@ -163,6 +163,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> > int rc = 0;
> > struct nls_table *nls_codepage = NULL;
> > struct cifs_ses *ses;
> > + int xid;
> >
> > /*
> > * SMB2s NegProt, SessSetup, Logoff do not have tcon yet so
> > @@ -307,17 +308,41 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
> > tcon->need_reopen_files = true;
> >
> > rc = cifs_tree_connect(0, tcon, nls_codepage);
> > - mutex_unlock(&ses->session_mutex);
> >
> > cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
> > if (rc) {
> > /* If sess reconnected but tcon didn't, something strange ... */
> > + mutex_unlock(&ses->session_mutex);
> > cifs_dbg(VFS, "reconnect tcon failed rc = %d\n", rc);
> > goto out;
> > }
> >
> > - if (smb2_command != SMB2_INTERNAL_CMD)
> > - mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
>
> Why are you removing this optimisation? For example, session/tcon
> reconnect will no longer be triggered after returning back to userspace,
> only when next I/O comes in.

--
Regards,
Shyam