All of lore.kernel.org
 help / color / mirror / Atom feed
From: Enzo Matsumiya <ematsumiya@suse.de>
To: linux-cifs@vger.kernel.org
Cc: pc@manguebit.org, linkinjeon@kernel.org,
	ronniesahlberg@gmail.com, sprasad@microsoft.com, tom@talpey.com,
	bharathsm@microsoft.com, henrique.carvalho@suse.com,
	Enzo Matsumiya <ematsumiya@suse.de>
Subject: [PATCH v3 1/2] smb: client: fast fail sends if need to reconnect
Date: Tue, 29 Sep 2026 14:03:57 -0300	[thread overview]
Message-ID: <20260929170358.270612-1-ematsumiya@suse.de> (raw)

smb_send_kvec() keeps retrying when server needs to reconnect, which
makes no sense as cifs_reconnect() will never run in parallel (because
both need server mutex).
IOW, retrying will never succeed, but only delay reconnects further.

Bail out early from smb_send_kvec() when need to reconnect.

Also shutdown socket queues when CifsNeedReconnect is first detected,
so any sends/receives fails immediately (and don't wait for socket to
timeout).

Add TCP_Server_Info::need_sock_shutdown to ensure socket is shutdown
only once.

kernel_sock_shutdown() will be called from whichever task detects it
first;  cifsd (cifs_tcp_ses_needs_reconnect()), or sending tasks,
through cifs_call_async() or compound_send_recv().

Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
v2 -> v3 (patch refactor, change strategy but keep concept):
- remove TCP_Server_Info::mutex_owner (covers sashiko report)
- shutdown socket directly from sender functions (when need to reconnect)
- bail out from smb_send_kvec() when need to reconnect only if rc <= 0
  (from sasiko report)

v1 -> v2 (fix issues detected by sashiko):
- move kernel_sock_shutdown() call out of cifs_tcp_ses_lock
- use server mutex to check/shutdown socket


 fs/smb/client/cifsglob.h  |  1 +
 fs/smb/client/connect.c   | 22 +++++++++++++++++++---
 fs/smb/client/transport.c | 30 ++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 79e4e84f8985..e49140d5c4a4 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -802,6 +802,7 @@ struct TCP_Server_Info {
 	bool	posix_ext_supported;
 	struct delayed_work reconnect; /* reconnect workqueue job */
 	struct mutex reconnect_mutex; /* prevent simultaneous reconnects */
+	bool need_sock_shutdown; /* true when CifsNeedReconnect was first set by non-cifsd task */
 	unsigned long echo_interval;
 
 	/*
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 28e1ddeb6182..733d50b042fd 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -126,12 +126,14 @@ void smb2_query_server_interfaces(struct work_struct *work)
 	queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
 			   (SMB_INTERFACE_POLL_INTERVAL * HZ));
 }
-
 #define set_need_reco(server) \
 do { \
 	spin_lock(&server->srv_lock); \
-	if (server->tcpStatus != CifsExiting) \
+	if (server->tcpStatus != CifsExiting) { \
+		if (server->tcpStatus != CifsNeedReconnect) \
+			server->need_sock_shutdown = true; \
 		server->tcpStatus = CifsNeedReconnect; \
+	} \
 	spin_unlock(&server->srv_lock); \
 } while (0)
 
@@ -353,6 +355,8 @@ cifs_abort_connection(struct TCP_Server_Info *server)
 
 static bool cifs_tcp_ses_needs_reconnect(struct TCP_Server_Info *server, int num_targets)
 {
+	bool shutdown;
+
 	spin_lock(&server->srv_lock);
 	server->nr_targets = num_targets;
 	if (server->tcpStatus == CifsExiting) {
@@ -365,9 +369,21 @@ static bool cifs_tcp_ses_needs_reconnect(struct TCP_Server_Info *server, int num
 	cifs_dbg(FYI, "Mark tcp session as need reconnect\n");
 	trace_smb3_reconnect(server->current_mid, server->conn_id,
 			     server->hostname);
-	server->tcpStatus = CifsNeedReconnect;
 
+	/* Cover cases where sender tasks didn't manage to shutdown the socket for some reason */
+	shutdown = (server->tcpStatus != CifsNeedReconnect || server->need_sock_shutdown);
+	server->need_sock_shutdown = false;
+	server->tcpStatus = CifsNeedReconnect;
 	spin_unlock(&server->srv_lock);
+
+	if (shutdown) {
+		cifs_server_lock(server);
+		if (server->ssocket)
+			/* Don't release it here/yet! */
+			kernel_sock_shutdown(server->ssocket, SHUT_RDWR);
+		cifs_server_unlock(server);
+	}
+
 	return true;
 }
 
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 6e21b5f8754a..a1ae8548cf16 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -179,7 +179,18 @@ smb_send_kvec(struct TCP_Server_Info *server, struct msghdr *smb_msg,
 		 * to avoid unnecessary reconnects.
 		 */
 		rc = sock_sendmsg(ssocket, smb_msg);
+
+		/*
+		 * If need to reconnect, blame it for any non-interrupt error and bail out early,
+		 * even if -EAGAIN;  reconnect will never happen in parallel as cifs_reconnect()
+		 * needs server mutex (which we're already holding), so it's pointless to retry.
+		 */
+		if (unlikely(rc <= 0 && server->tcpStatus == CifsNeedReconnect &&
+			     !is_interrupt_error(rc)))
+			return -ECONNRESET;
+
 		if (rc == -EAGAIN || unlikely(rc == -EINTR && task_work_pending(current))) {
+
 			retries++;
 			if (retries >= 14 ||
 			    (!server->noblocksnd && (retries > 2))) {
@@ -654,6 +665,23 @@ int wait_for_response(struct TCP_Server_Info *server, struct mid_q_entry *mid)
 	return 0;
 }
 
+static inline void cond_sock_shutdown(struct TCP_Server_Info *server)
+{
+	bool shutdown;
+
+	lockdep_assert_held(&server->_srv_mutex);
+
+	spin_lock(&server->srv_lock);
+	/* No need to check status, need_sock_shutdown is only set if CifsNeedReconnect */
+	shutdown = (server->need_sock_shutdown && server->ssocket);
+	server->need_sock_shutdown = false;
+	spin_unlock(&server->srv_lock);
+
+	if (shutdown)
+		/* Don't release it here/yet! */
+		kernel_sock_shutdown(server->ssocket, SHUT_RDWR);
+}
+
 /*
  * Send a SMB request and set the callback function in the mid to handle
  * the result. Caller is responsible for dealing with timeouts.
@@ -724,6 +752,7 @@ cifs_call_async(struct TCP_Server_Info *server, struct smb_rqst *rqst,
 		revert_current_mid(server, mid->credits);
 		server->sequence_number -= 2;
 		delete_mid(server, mid);
+		cond_sock_shutdown(server);
 	}
 
 	cifs_server_unlock(server);
@@ -982,6 +1011,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
 			delete_mid(server, mid[i]);
 			cancelled_mid[i] = true;
 		}
+		cond_sock_shutdown(server);
 	}
 
 	cifs_server_unlock(server);
-- 
2.55.0


             reply	other threads:[~2026-09-29 17:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-29 17:03 Enzo Matsumiya [this message]
2026-09-29 17:03 ` [PATCH v3 2/2] smb: client: prevent premature discard of requests if reconnecting Enzo Matsumiya
2026-10-06 19:48   ` Paulo Alcantara
2026-10-06 21:00     ` Enzo Matsumiya
2026-10-06 22:51       ` Paulo Alcantara
2026-10-07 15:04         ` Enzo Matsumiya
2026-10-07 23:25           ` Paulo Alcantara
2026-10-07 17:02         ` Enzo Matsumiya
2026-10-07 23:35           ` Paulo Alcantara
2026-10-06 18:14 ` [PATCH v3 1/2] smb: client: fast fail sends if need to reconnect Paulo Alcantara
2026-10-06 18:52   ` Enzo Matsumiya
2026-10-08 18:30     ` Enzo Matsumiya

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=20260929170358.270612-1-ematsumiya@suse.de \
    --to=ematsumiya@suse.de \
    --cc=bharathsm@microsoft.com \
    --cc=henrique.carvalho@suse.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=pc@manguebit.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=sprasad@microsoft.com \
    --cc=tom@talpey.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.