public inbox for linux-cifs@vger.kernel.org
 help / color / mirror / Atom feed
From: nspmangalore@gmail.com
To: smfrench@gmail.com, pc@manguebit.com, bharathsm.hsk@gmail.com,
	linux-cifs@vger.kernel.org
Cc: Shyam Prasad N <sprasad@microsoft.com>
Subject: [PATCH 10/14] cifs: reconnect work should have reference on server struct
Date: Mon, 30 Oct 2023 11:00:16 +0000	[thread overview]
Message-ID: <20231030110020.45627-10-sprasad@microsoft.com> (raw)
In-Reply-To: <20231030110020.45627-1-sprasad@microsoft.com>

From: Shyam Prasad N <sprasad@microsoft.com>

The delayed work for reconnect takes server struct
as a parameter. But it does so without holding a ref
to it. Normally, this may not show a problem as
the reconnect work is only cancelled on umount.

However, since we now plan to support scaling down of
channels, and the scale down can happen from reconnect
work itself, we need to fix it.

This change takes a reference on the server struct
before it is passed to the delayed work. And drops
the reference in the delayed work itself. Or if
the delayed work is successfully cancelled, by the
process that cancels it.

Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
 fs/smb/client/connect.c | 27 +++++++++++++++++++++------
 fs/smb/client/smb2pdu.c | 23 +++++++++++++----------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 184075da5c6e..e71aa33bf026 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -389,7 +389,13 @@ static int __cifs_reconnect(struct TCP_Server_Info *server,
 			spin_unlock(&server->srv_lock);
 			cifs_swn_reset_server_dstaddr(server);
 			cifs_server_unlock(server);
-			mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+
+			/* increase ref count which reconnect work will drop */
+			spin_lock(&cifs_tcp_ses_lock);
+			server->srv_count++;
+			spin_unlock(&cifs_tcp_ses_lock);
+			if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0))
+				cifs_put_tcp_session(server, false);
 		}
 	} while (server->tcpStatus == CifsNeedReconnect);
 
@@ -519,7 +525,13 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server)
 		spin_unlock(&server->srv_lock);
 		cifs_swn_reset_server_dstaddr(server);
 		cifs_server_unlock(server);
-		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+
+		/* increase ref count which reconnect work will drop */
+		spin_lock(&cifs_tcp_ses_lock);
+		server->srv_count++;
+		spin_unlock(&cifs_tcp_ses_lock);
+		if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0))
+			cifs_put_tcp_session(server, false);
 	} while (server->tcpStatus == CifsNeedReconnect);
 
 	mutex_lock(&server->refpath_lock);
@@ -1601,16 +1613,19 @@ cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
 
 	cancel_delayed_work_sync(&server->echo);
 
-	if (from_reconnect)
+	if (from_reconnect) {
 		/*
 		 * Avoid deadlock here: reconnect work calls
 		 * cifs_put_tcp_session() at its end. Need to be sure
 		 * that reconnect work does nothing with server pointer after
 		 * that step.
 		 */
-		cancel_delayed_work(&server->reconnect);
-	else
-		cancel_delayed_work_sync(&server->reconnect);
+		if (cancel_delayed_work(&server->reconnect))
+			cifs_put_tcp_session(server, from_reconnect);
+	} else {
+		if (cancel_delayed_work_sync(&server->reconnect))
+			cifs_put_tcp_session(server, from_reconnect);
+	}
 
 	spin_lock(&server->srv_lock);
 	server->tcpStatus = CifsExiting;
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index c75a80bb6d9e..b7665155f4e2 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3852,12 +3852,6 @@ void smb2_reconnect_server(struct work_struct *work)
 		}
 		spin_unlock(&ses->chan_lock);
 	}
-	/*
-	 * Get the reference to server struct to be sure that the last call of
-	 * cifs_put_tcon() in the loop below won't release the server pointer.
-	 */
-	if (tcon_exist || ses_exist)
-		server->srv_count++;
 
 	spin_unlock(&cifs_tcp_ses_lock);
 
@@ -3905,13 +3899,17 @@ void smb2_reconnect_server(struct work_struct *work)
 
 done:
 	cifs_dbg(FYI, "Reconnecting tcons and channels finished\n");
-	if (resched)
+	if (resched) {
 		queue_delayed_work(cifsiod_wq, &server->reconnect, 2 * HZ);
+		mutex_unlock(&pserver->reconnect_mutex);
+
+		/* no need to put tcp session as we're retrying */
+		return;
+	}
 	mutex_unlock(&pserver->reconnect_mutex);
 
 	/* now we can safely release srv struct */
-	if (tcon_exist || ses_exist)
-		cifs_put_tcp_session(server, 1);
+	cifs_put_tcp_session(server, true);
 }
 
 int
@@ -3931,7 +3929,12 @@ SMB2_echo(struct TCP_Server_Info *server)
 	    server->ops->need_neg(server)) {
 		spin_unlock(&server->srv_lock);
 		/* No need to send echo on newly established connections */
-		mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
+		spin_lock(&cifs_tcp_ses_lock);
+		server->srv_count++;
+		spin_unlock(&cifs_tcp_ses_lock);
+		if (mod_delayed_work(cifsiod_wq, &server->reconnect, 0))
+			cifs_put_tcp_session(server, false);
+
 		return rc;
 	}
 	spin_unlock(&server->srv_lock);
-- 
2.34.1


  parent reply	other threads:[~2023-10-30 11:00 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-30 11:00 [PATCH 01/14] cifs: print server capabilities in DebugData nspmangalore
2023-10-30 11:00 ` [PATCH 02/14] cifs: add xid to query server interface call nspmangalore
2023-10-31  5:35   ` Bharath SM
2023-10-30 11:00 ` [PATCH 03/14] cifs: reconnect helper should set reconnect for the right channel nspmangalore
2023-10-31 15:27   ` Paulo Alcantara
2023-10-31 18:29     ` Steve French
2023-10-30 11:00 ` [PATCH 04/14] cifs: do not reset chan_max if multichannel is not supported at mount nspmangalore
2023-11-01  2:57   ` Steve French
2023-11-01  3:14   ` Steve French
2023-10-30 11:00 ` [PATCH 05/14] cifs: force interface update before a fresh session setup nspmangalore
2023-11-01  3:14   ` Steve French
2023-10-30 11:00 ` [PATCH 06/14] cifs: handle cases where a channel is closed nspmangalore
2023-11-01  3:09   ` Steve French
2023-11-02 12:26     ` Shyam Prasad N
2023-10-30 11:00 ` [PATCH 07/14] cifs: distribute channels across interfaces based on speed nspmangalore
2023-10-30 11:00 ` [PATCH 08/14] cifs: account for primary channel in the interface list nspmangalore
2023-11-08 15:44   ` Paulo Alcantara
2023-11-08 18:16     ` Steve French
2023-11-08 19:03       ` Paulo Alcantara
2023-10-30 11:00 ` [PATCH 09/14] cifs: add a back pointer to cifs_sb from tcon nspmangalore
2023-11-01  3:30   ` Steve French
2023-11-03 21:03   ` Paulo Alcantara
2023-11-06 16:12     ` Shyam Prasad N
2023-11-06 17:04       ` Shyam Prasad N
     [not found]         ` <CAH2r5msQLTcdiHBrOKd+q6LPPHW_Jj3QbpFZyZ48CJbrtDqC5w@mail.gmail.com>
     [not found]           ` <CAH2r5mt4hC5x2w2D46y13j_OtjkJk9_ZaeGXbb7YKukffBk2LQ@mail.gmail.com>
2023-11-06 19:36             ` Fwd: " Steve French
2023-11-08 15:24         ` Paulo Alcantara
2023-11-08 16:11           ` Steve French
2023-10-30 11:00 ` nspmangalore [this message]
2023-11-16 17:10   ` [PATCH 10/14] cifs: reconnect work should have reference on server struct Paulo Alcantara
     [not found]     ` <CAH2r5mtDeP323Z8=9WjCCYVVb9B2AmO5Q4PDtcMz8wxVUCVRBA@mail.gmail.com>
2023-11-16 19:35       ` Paulo Alcantara
2023-10-30 11:00 ` [PATCH 11/14] cifs: handle when server starts supporting multichannel nspmangalore
2023-11-01  3:30   ` Steve French
2023-11-01 15:52   ` Paulo Alcantara
2023-11-04  7:50     ` Shyam Prasad N
2023-11-02 20:28   ` Paulo Alcantara
2023-11-03  0:43     ` Steve French
2023-11-03 20:32       ` Paulo Alcantara
     [not found]       ` <notmuch-sha1-c3bfa7f4ae0bb24c5ee7cfddb408c2fbeca5d8f7>
2023-11-08 16:02         ` Paulo Alcantara
2023-11-08 19:25           ` Steve French
2023-11-08 19:31             ` Paulo Alcantara
2023-10-30 11:00 ` [PATCH 12/14] cifs: handle when server stops " nspmangalore
2023-11-08 16:35   ` Paulo Alcantara
     [not found]   ` <notmuch-sha1-9ed0289358ca5c90903408ad9c0ac0310afee598>
2023-11-08 19:13     ` Paulo Alcantara
2023-11-08 19:41       ` Paulo Alcantara
2023-11-09 11:44         ` Shyam Prasad N
2023-11-09 13:28           ` Paulo Alcantara
2023-11-09 13:49             ` Shyam Prasad N
2023-11-10  4:09               ` Shyam Prasad N
2023-11-11 17:23                 ` Paulo Alcantara
2023-11-12 18:52                   ` Steve French
     [not found]                   ` <CAH2r5mvG3zLBxknPOuaz9=GarZO6n6bhcduiZHHfiqVYZYJiVQ@mail.gmail.com>
2023-11-12 19:32                     ` Paulo Alcantara
2023-10-30 11:00 ` [PATCH 13/14] cifs: display the endpoint IP details in DebugData nspmangalore
2023-10-31 15:18   ` Paulo Alcantara
     [not found]   ` <notmuch-sha1-260ef7fe7af7face0e1486229c0fda5149fe14e2>
2023-11-01 14:12     ` Paulo Alcantara
2023-11-01 14:19       ` Steve French
2023-11-04  7:44       ` Shyam Prasad N
2023-11-04 19:00         ` Paulo Alcantara
2023-10-30 12:34 ` [PATCH 01/14] cifs: print server capabilities " Bharath SM
2023-10-30 12:40   ` Shyam Prasad N
2023-10-30 12:51     ` Shyam Prasad N
2023-10-30 14:54 ` Steve French

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231030110020.45627-10-sprasad@microsoft.com \
    --to=nspmangalore@gmail.com \
    --cc=bharathsm.hsk@gmail.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=pc@manguebit.com \
    --cc=smfrench@gmail.com \
    --cc=sprasad@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox