diff mbox series

[SMB3] 3 multichannel patches from Shyam

Message ID CAH2r5muPSOg3Pxfj_CUOfE540eQuwNwMQde=b2BGKVasxKcm1g@mail.gmail.com (mailing list archive)
State New, archived
Headers show
Series [SMB3] 3 multichannel patches from Shyam | expand

Commit Message

Steve French May 24, 2022, 7:20 p.m. UTC 
Attached are three multichannel patches from Shyam (am still reviewing
the fourth, also did minor cleanup on patch 3)

patch 1:
    cifs: avoid parallel session setups on same channel

    After allowing channels to reconnect in parallel, it now
    becomes important to take care that multiple processes do not
    call negotiate/session setup in parallel on the same channel.

    This change avoids that by marking a channel as "in_reconnect".
    During session setup if the channel in question has this flag
    set, we return immediately.

patch 2:
    cifs: use new enum for ses_status

    ses->status today shares statusEnum with server->tcpStatus.
    This has been confusing, and tcon->status has deviated to use
    a new enum. Follow suit and use new enum for ses_status as well.

patch 3:
    Recent changes to multichannel to allow channel reconnects to
    work in parallel and independent of each other did so by
    making use of tcpStatus for the connection, and status for the
    session. However, this did not take into account the multiuser
    scenario, where same connection is used by multiple connections.

    However, tcpStatus should be tracked only till the end of
    negotiate exchange, and not used for session setup. This change
    fixes this.
diff mbox series

Patch

From dd3cd8709ed5f4ae8998e0cd44c05bd26bc879e8 Mon Sep 17 00:00:00 2001
From: Shyam Prasad N <sprasad@microsoft.com>
Date: Thu, 7 Apr 2022 13:15:49 +0000
Subject: [PATCH 2/3] cifs: use new enum for ses_status

ses->status today shares statusEnum with server->tcpStatus.
This has been confusing, and tcon->status has deviated to use
a new enum. Follow suit and use new enum for ses_status as well.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifs_debug.c    |  4 ++--
 fs/cifs/cifsglob.h      | 15 +++++++++++----
 fs/cifs/cifssmb.c       |  2 +-
 fs/cifs/connect.c       | 34 +++++++++++++++++-----------------
 fs/cifs/misc.c          |  2 +-
 fs/cifs/smb2pdu.c       |  2 +-
 fs/cifs/smb2transport.c |  4 ++--
 fs/cifs/transport.c     |  8 ++++----
 8 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index 0effc4c95077..c098735d41c6 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -387,7 +387,7 @@  static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 				(ses->serverNOS == NULL)) {
 				seq_printf(m, "\n\t%d) Address: %s Uses: %d Capability: 0x%x\tSession Status: %d ",
 					i, ses->ip_addr, ses->ses_count,
-					ses->capabilities, ses->status);
+					ses->capabilities, ses->ses_status);
 				if (ses->session_flags & SMB2_SESSION_FLAG_IS_GUEST)
 					seq_printf(m, "Guest ");
 				else if (ses->session_flags & SMB2_SESSION_FLAG_IS_NULL)
@@ -399,7 +399,7 @@  static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
 					"\n\tSMB session status: %d ",
 				i, ses->ip_addr, ses->serverDomain,
 				ses->ses_count, ses->serverOS, ses->serverNOS,
-				ses->capabilities, ses->status);
+				ses->capabilities, ses->ses_status);
 			}
 
 			seq_printf(m, "\n\tSecurity type: %s ",
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 011c440bbd98..711cf51ac14f 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -106,7 +106,7 @@ 
  * CIFS vfs client Status information (based on what we know.)
  */
 
-/* associated with each tcp and smb session */
+/* associated with each connection */
 enum statusEnum {
 	CifsNew = 0,
 	CifsGood,
@@ -114,8 +114,15 @@  enum statusEnum {
 	CifsNeedReconnect,
 	CifsNeedNegotiate,
 	CifsInNegotiate,
-	CifsNeedSessSetup,
-	CifsInSessSetup,
+};
+
+/* associated with each smb session */
+enum ses_status_enum {
+	SES_NEW = 0,
+	SES_GOOD,
+	SES_EXITING,
+	SES_NEED_RECON,
+	SES_IN_SETUP
 };
 
 /* associated with each tree connection to the server */
@@ -930,7 +937,7 @@  struct cifs_ses {
 	struct mutex session_mutex;
 	struct TCP_Server_Info *server;	/* pointer to server info */
 	int ses_count;		/* reference counter */
-	enum statusEnum status;  /* updates protected by cifs_tcp_ses_lock */
+	enum ses_status_enum ses_status;  /* updates protected by cifs_tcp_ses_lock */
 	unsigned overrideSecFlg;  /* if non-zero override global sec flags */
 	char *serverOS;		/* name of operating system underlying server */
 	char *serverNOS;	/* name of network operating system of server */
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index a9dccd10e885..6371b9eebdad 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -75,7 +75,7 @@  cifs_mark_open_files_invalid(struct cifs_tcon *tcon)
 
 	/* only send once per connect */
 	spin_lock(&cifs_tcp_ses_lock);
-	if ((tcon->ses->status != CifsGood) || (tcon->status != TID_NEED_RECON)) {
+	if ((tcon->ses->ses_status != SES_GOOD) || (tcon->status != TID_NEED_RECON)) {
 		spin_unlock(&cifs_tcp_ses_lock);
 		return;
 	}
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index da1579fba496..df4bcc581559 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -241,7 +241,7 @@  cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
 		if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses))
 			goto next_session;
 
-		ses->status = CifsNeedReconnect;
+		ses->ses_status = SES_NEED_RECON;
 
 		list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
 			tcon->need_reconnect = true;
@@ -1828,7 +1828,7 @@  cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
 
 	spin_lock(&cifs_tcp_ses_lock);
 	list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
-		if (ses->status == CifsExiting)
+		if (ses->ses_status == SES_EXITING)
 			continue;
 		if (!match_session(ses, ctx))
 			continue;
@@ -1848,7 +1848,7 @@  void cifs_put_smb_ses(struct cifs_ses *ses)
 	cifs_dbg(FYI, "%s: ses_count=%d\n", __func__, ses->ses_count);
 
 	spin_lock(&cifs_tcp_ses_lock);
-	if (ses->status == CifsExiting) {
+	if (ses->ses_status == SES_EXITING) {
 		spin_unlock(&cifs_tcp_ses_lock);
 		return;
 	}
@@ -1864,13 +1864,13 @@  void cifs_put_smb_ses(struct cifs_ses *ses)
 	/* ses_count can never go negative */
 	WARN_ON(ses->ses_count < 0);
 
-	if (ses->status == CifsGood)
-		ses->status = CifsExiting;
+	if (ses->ses_status == SES_GOOD)
+		ses->ses_status = SES_EXITING;
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	cifs_free_ipc(ses);
 
-	if (ses->status == CifsExiting && server->ops->logoff) {
+	if (ses->ses_status == SES_EXITING && server->ops->logoff) {
 		xid = get_xid();
 		rc = server->ops->logoff(xid, ses);
 		if (rc)
@@ -2090,7 +2090,7 @@  cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
 	ses = cifs_find_smb_ses(server, ctx);
 	if (ses) {
 		cifs_dbg(FYI, "Existing smb sess found (status=%d)\n",
-			 ses->status);
+			 ses->ses_status);
 
 		spin_lock(&ses->chan_lock);
 		if (cifs_chan_needs_reconnect(ses, server)) {
@@ -4001,11 +4001,13 @@  cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 	spin_unlock(&ses->chan_lock);
 
 	spin_lock(&cifs_tcp_ses_lock);
-	if (ses->status == CifsExiting) {
+	if (ses->ses_status == SES_EXITING) {
 		spin_unlock(&cifs_tcp_ses_lock);
 		return 0;
 	}
-	ses->status = CifsInSessSetup;
+
+	if (!is_binding)
+		ses->ses_status = SES_IN_SETUP;
 	spin_unlock(&cifs_tcp_ses_lock);
 
 	if (!is_binding) {
@@ -4031,15 +4033,13 @@  cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
 	if (rc) {
 		cifs_server_dbg(VFS, "Send error in SessSetup = %d\n", rc);
 		spin_lock(&cifs_tcp_ses_lock);
-		if (ses->status == CifsInSessSetup)
-			ses->status = CifsNeedSessSetup;
+		if (ses->ses_status == SES_IN_SETUP)
+			ses->ses_status = SES_NEED_RECON;
 		spin_unlock(&cifs_tcp_ses_lock);
 	} else {
 		spin_lock(&cifs_tcp_ses_lock);
-		if (ses->status == CifsInSessSetup)
-			ses->status = CifsGood;
-		/* Even if one channel is active, session is in good state */
-		ses->status = CifsGood;
+		if (ses->ses_status == SES_IN_SETUP)
+			ses->ses_status = SES_GOOD;
 		spin_unlock(&cifs_tcp_ses_lock);
 
 		spin_lock(&ses->chan_lock);
@@ -4509,7 +4509,7 @@  int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
 
 	/* only send once per connect */
 	spin_lock(&cifs_tcp_ses_lock);
-	if (tcon->ses->status != CifsGood ||
+	if (tcon->ses->ses_status != SES_GOOD ||
 	    (tcon->status != TID_NEW &&
 	    tcon->status != TID_NEED_TCON)) {
 		spin_unlock(&cifs_tcp_ses_lock);
@@ -4577,7 +4577,7 @@  int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
 
 	/* only send once per connect */
 	spin_lock(&cifs_tcp_ses_lock);
-	if (tcon->ses->status != CifsGood ||
+	if (tcon->ses->ses_status != SES_GOOD ||
 	    (tcon->status != TID_NEW &&
 	    tcon->status != TID_NEED_TCON)) {
 		spin_unlock(&cifs_tcp_ses_lock);
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index a5b5b15e658a..e869c2a51034 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -69,7 +69,7 @@  sesInfoAlloc(void)
 	ret_buf = kzalloc(sizeof(struct cifs_ses), GFP_KERNEL);
 	if (ret_buf) {
 		atomic_inc(&sesInfoAllocCount);
-		ret_buf->status = CifsNew;
+		ret_buf->ses_status = SES_NEW;
 		++ret_buf->ses_count;
 		INIT_LIST_HEAD(&ret_buf->smb_ses_list);
 		INIT_LIST_HEAD(&ret_buf->tcon_list);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index f5321a3500f3..084be3a90198 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -179,7 +179,7 @@  smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
 		}
 	}
 	spin_unlock(&cifs_tcp_ses_lock);
-	if ((!tcon->ses) || (tcon->ses->status == CifsExiting) ||
+	if ((!tcon->ses) || (tcon->ses->ses_status == SES_EXITING) ||
 	    (!tcon->ses->server) || !server)
 		return -EIO;
 
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index 01b732641edb..55e79f6ee78d 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -780,7 +780,7 @@  smb2_get_mid_entry(struct cifs_ses *ses, struct TCP_Server_Info *server,
 		return -EAGAIN;
 	}
 
-	if (ses->status == CifsNew) {
+	if (ses->ses_status == SES_NEW) {
 		if ((shdr->Command != SMB2_SESSION_SETUP) &&
 		    (shdr->Command != SMB2_NEGOTIATE)) {
 			spin_unlock(&cifs_tcp_ses_lock);
@@ -789,7 +789,7 @@  smb2_get_mid_entry(struct cifs_ses *ses, struct TCP_Server_Info *server,
 		/* else ok - we are setting up session */
 	}
 
-	if (ses->status == CifsExiting) {
+	if (ses->ses_status == SES_EXITING) {
 		if (shdr->Command != SMB2_LOGOFF) {
 			spin_unlock(&cifs_tcp_ses_lock);
 			return -EAGAIN;
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index c667e6ddfe2f..05eca41e3b1e 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -726,7 +726,7 @@  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
 			struct mid_q_entry **ppmidQ)
 {
 	spin_lock(&cifs_tcp_ses_lock);
-	if (ses->status == CifsNew) {
+	if (ses->ses_status == SES_NEW) {
 		if ((in_buf->Command != SMB_COM_SESSION_SETUP_ANDX) &&
 			(in_buf->Command != SMB_COM_NEGOTIATE)) {
 			spin_unlock(&cifs_tcp_ses_lock);
@@ -735,7 +735,7 @@  static int allocate_mid(struct cifs_ses *ses, struct smb_hdr *in_buf,
 		/* else ok - we are setting up session */
 	}
 
-	if (ses->status == CifsExiting) {
+	if (ses->ses_status == SES_EXITING) {
 		/* check if SMB session is bad because we are setting it up */
 		if (in_buf->Command != SMB_COM_LOGOFF_ANDX) {
 			spin_unlock(&cifs_tcp_ses_lock);
@@ -1187,7 +1187,7 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 	 * Compounding is never used during session establish.
 	 */
 	spin_lock(&cifs_tcp_ses_lock);
-	if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
+	if ((ses->ses_status == SES_NEW) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
 		spin_unlock(&cifs_tcp_ses_lock);
 
 		mutex_lock(&server->srv_mutex);
@@ -1260,7 +1260,7 @@  compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 	 * Compounding is never used during session establish.
 	 */
 	spin_lock(&cifs_tcp_ses_lock);
-	if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
+	if ((ses->ses_status == SES_NEW) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
 		struct kvec iov = {
 			.iov_base = resp_iov[0].iov_base,
 			.iov_len = resp_iov[0].iov_len
-- 
2.34.1