I looked at this problem in more detail. Here's a summary of what happened: Before my recent changes, when mchan was used, and a negotiate/sess-setup happened, all other channels were paused. So things were a lot simpler during a connect/reconnect. What I did with my recent changes is to allow I/O to flow in other channels when connect/reconnect happened on one of the channels. This meant that there could be multiple channels that do negotiate/session setup for the same session in parallel. i.e. first channel would create a new session. Other channels would bind to the same session. This meant that the server->tcpStatus needed to indicate an ongoing sess setup. So that multiple channels could do session setup in parallel. Unfortunately, I did not account for multiuser scenario, which does the reverse. i.e. uses the same server for different tcp sessions. Here's a patch I propose to fix this issue. Please review and let me know what you think. https://github.com/sprasad-microsoft/smb3-kernel-client/commit/34333e9de1526c46e9ae5ff9a54f0199b827fd0e.patch Essentially, I'm doing 3 changes in this patch: 1. Making sure that tcpStatus is only till the end of tcp connection establishment. This means that tcpStatus reaches CifsGood when the tcp connection is good 2. Once cifs_negotiate_protocol starts, anything done will affect the session state, and should not modify tcpStatus. 3. To detect the small window between tcp connection setup and before negotiate, use need_neg() On Thu, Mar 17, 2022 at 9:14 AM ronnie sahlberg <ronniesahlberg@xxxxxxxxx> wrote: > > On Thu, Mar 17, 2022 at 1:20 PM Steve French <smfrench@xxxxxxxxx> wrote: > > > > cifssmb3: fix incorrect session setup check for multiuser mounts > > If it fixes multiuser then Acked-by me. > We are so close to rc8 that we can not do intrusive changes, so if > it fixes it short term. > For longer term, post rc8 we need to rewrite the statemaching completely > and separate out "what happens in server->tcpStatus" as one statemachine and > "what happens in server->status" as a separate one. Right now it is a mess. > > > > > > A recent change to how the SMB3 server (socket) and session status > > is managed regressed multiuser mounts by changing the check > > for whether session setup is needed to the socket (TCP_Server_info) > > structure instead of the session struct (cifs_ses). Add additional > > check in cifs_setup_sesion to fix this. > > > > Fixes: 73f9bfbe3d81 ("cifs: maintain a state machine for tcp/smb/tcon sessions") > > Reported-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx> > > Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx> > > --- > > fs/cifs/connect.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c > > index 053cb449eb16..d3020abfe404 100644 > > --- a/fs/cifs/connect.c > > +++ b/fs/cifs/connect.c > > @@ -3924,7 +3924,8 @@ cifs_setup_session(const unsigned int xid, > > struct cifs_ses *ses, > > > > /* only send once per connect */ > > spin_lock(&cifs_tcp_ses_lock); > > - if (server->tcpStatus != CifsNeedSessSetup) { > > + if ((server->tcpStatus != CifsNeedSessSetup) && > > + (ses->status == CifsGood)) { > > spin_unlock(&cifs_tcp_ses_lock); > > return 0; > > } > > > > -- > > Thanks, > > > > Steve -- Regards, Shyam
