mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v6 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient
@ 2026-08-29 11:35 Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 1/5] ceph: use READ_ONCE/WRITE_ONCE for oldest_tid Xiubo Li via B4 Relay
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-08-29 11:35 UTC (permalink / raw)
  To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
  Cc: ceph-devel, linux-kernel, Xiubo Li

This series reduces mdsc->mutex hold times from hundreds of
microseconds to tens of microseconds on the hot request-submit and
reply-handling paths.

The approach is incremental:
  1. Convert oldest_tid to atomic64_t so that __prepare_send_request()
     and __send_request() no longer need the mutex.
  2. Replace the request_tree rbtree with an xarray for O(1) lookups
     and internally-locked iteration.
  3. Add a dedicated wait_list_lock spinlock so wait-list operations
     no longer depend on the global mutex.
  4. Move mdsc->mutex acquisition inside __do_request(), then release
     it during the send phase (message construction and path walking),
     leaving only the brief setup/teardown under the lock.
  5. Narrow the mutex scope in replay_unsafe_requests() similarly.

Tested with concurrent readdir + stat on a 5000-file directory
(32 threads).  bpftrace measurements show:

                 before         after
  __do_request   354-2327 us    10-68 us    (34x)
  handle_reply   51-416 us     10-32 us    (13x)
  submit_request 89-211 us     10-41 us    (5x)

Benchmark results from test_i_caps (aggregate cache test):

  | Test                | Before   | After    | Improvement |
  |---------------------|----------|----------|-------------|
  | stat storm ST       | 692k/s   | 1,420k/s | +105% (2.1x)|
  | stat storm MT       | 722k/s   | 1,373k/s | +90%  (1.9x)|
  | open/close          | 403k/s   | 524k/s   | +30%        |
  | stat hot (cache)    | 353k/s   | 540k/s   | +53%        |
  | readdir             | 877/s    | 832/s    | ~0%         |
  | Total time          | 195s     | 121s     | -38%  (1.6x)|

The 2x stat throughput gain comes from __do_request() no longer
holding mdsc->mutex during the __send_request() phase, so dentry
path walking and message encoding in create_request_message()
run outside the lock.

No functional changes intended.

Testing for 24+ hours:

The runtime validation ran on a vstart cluster (3 active + 3 standby
MDS) with a 32-thread metadata load generator and a kernel built with
CONFIG_PROVE_LOCKING, CONFIG_DEBUG_LIST and CONFIG_DETECT_HUNG_TASK:

- fsstress + dbench smoke runs: clean.
- r1, single-MDS failover churn: 108 failovers across 4 runs, every
  session reconnected, no lost wakeups, no hangs.
- r2, double "ceph mds fail" chaos: ~90 double-fail cycles across
  2 runs, zero hangs, zero kernel warnings, clean teardown.
- r3, double-fail plus victim processes SIGKILLed every 50-400 ms
  (5084 kills in one run) to exercise the request abort path: zero
  warnings, zero list-corruption reports, zero refcount issues.
- r4 (double mds fail + evict churn + 8 victim processes killed at
  200-2000 ms): one 32-minute run FAILED -- a worker was stuck in
  truncate for ~15 minutes at shutdown, the join oracle's first hit
  across all runs.  Live-kernel forensics (drgn against /proc/kcore)
  showed the wedge is unrelated to this series: the double-fail had
  dropped the inode's dirty caps in the session-loss path (caps.c)
  without cleaning up the page wrbuffer refs, so the ceph_inode_work()
  kworker spun forever in __ceph_do_pending_vmtruncate()'s flush
  branch -- with no dirty caps, ceph_writepages_start() returns
  -ENODATA without writing, the refs never drain, and the loop holds
  i_truncate_mutex indefinitely, starving concurrent truncates (and,
  via setattr's inode lock and vfs_unlink()'s target-inode lock,
  unlinks).  Every link of that chain is in caps.c/inode.c/addr.c/VFS
  code untouched by these patches; it is a pre-existing consistency
  bug (dirty caps dropped without wrbuffer-ref cleanup on session
  loss, plus an unbounded flush loop) to be fixed separately.  (The
  r4 evict trigger was skipped in this run as debugfs was not
  mounted; session eviction itself had been exercised in earlier
  testing.)

No lockdep reports, list corruption, refcount warnings, or hung tasks
across the whole campaign.  The campaign also exposed two pre-existing
mainline bugs (a NULL oldest-snapc dereference in the writeback path
and a request abort-path use-after-free), both reproducible without
this series on a vanilla 7.2 kernel and fixed separately.

The whole test code could be find in:
https://tracker.ceph.com/issues/80086
Or
https://tracker.ceph.com/issues/80084

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
Changes in v6:
- Only patch 5/5 is changed; patches 1-4 are identical to v5.
- Replace the ad-hoc r_attempts dispatch gate with an explicit
  ownership protocol.  __do_request() and replay_unsafe_requests()
  claim CEPH_MDS_R_DISPATCHING under mdsc->mutex before the unlocked
  prepare/send window and release it on exit, so only one context ever
  rebuilds and sends a request's message.  A claim consumes any
  pending CEPH_MDS_R_RESEND.
- cleanup_session_requests() and handle_forward() set
  CEPH_MDS_R_RESEND under mdsc->mutex when they see DISPATCHING held,
  instead of relying on r_attempts == 0 to trigger a resend.  Fixes
  the v5 race where cleanup_session_requests() could zero r_attempts
  while a sender was between the mutex release and __send_request(),
  letting a second __do_request() in and double ceph_msg_put() the
  request message.
- On release, the owner consumes a pending CEPH_MDS_R_RESEND
  and re-dispatches the request through __do_request() (restart label)
  as long as it is still registered in the request xarray.
- __wake_requests() now takes mdsc->mutex and splices the wait
  list under it, pinning a reference on each request in the same
  critical section as the list_del_init().  This fixes the v5 lockless
  walk of the spliced list against concurrent list_del_init() by the
  park paths, and serializes the SESSION_OPEN wake in handle_session()
  against the park decision in __do_request().
- Collector exclusivity — kick_requests(), __wake_requests()
  and replay_unsafe_requests() skip any request whose r_aux_item is
  non-empty (checked under mdsc->mutex), so two concurrent collectors
  can no longer add the same node to their local lists.
- The re-park paths in __do_request() now list_del_init() the
  request before list_add()ing it onto a wait list, so re-parking is
  idempotent (needed now that a request can re-enter __do_request()
  through the RESEND restart loop).
- __do_request() refuses to dispatch GOT_UNSAFE requests,
  clearing any pending RESEND/DISPATCHING bits, so an unsafe request
  is never parked on a wait list.  Together with the exclusivity
  predicate this guarantees a request is never both wait-listed and on
  session->s_unsafe, and replay_unsafe_requests() remains the only
  replayer of unsafe requests.
- replay_unsafe_requests() claims DISPATCHING in both collect
  loops, and after __send_request() consumes a pending RESEND and
  re-dispatches through __do_request().
- send_mds_reconnect() drops snap_rwsem before calling
  __wake_requests(), which now takes mdsc->mutex internally (lock
  order).
- Link to v5: https://patch.msgid.link/20260818-ceph-mdsc-mutex-optimization-v5-0-7d335a3a1d0b@clyso.com

Changes in v5:
- patch 4: add INIT_LIST_HEAD(&req->r_aux_item) in ceph_mdsc_create_request().
- patch 5: keep requests on session->s_unsafe during replay; use r_aux_item
  only as a walk list.
- Link to v4: https://patch.msgid.link/20260812-ceph-mdsc-mutex-optimization-v4-0-fca3b7462f94@clyso.com

Changes in v4:
- Fix ref leak in __register_request() xa_store error path
- Fix r_attempts data race: move r_attempts++ from
  __prepare_send_request() (lockless) into __do_request() and
  replay_unsafe_requests() under mdsc->mutex.  Adjust the retry
  overflow check accordingly.
- Fix kick_requests() list corruption: detach r_wait from the local
  kick_list before calling __do_request().
- Fix collect-then-replay list-node races in both kick_requests()
  and replay_unsafe_requests(): introduce r_aux_item, a dedicated
  list_head for temporary local list iteration, so that concurrent
  __unregister_request() cannot corrupt the iterator.
- Link to v3: https://patch.msgid.link/20260811-ceph-mdsc-mutex-optimization-v3-0-d031114419f4@clyso.com

Changes in v3:
- Restrict CEPH_FS to 64BIT to prevent xarray index truncation of
  u64 transaction IDs on 32-bit platforms, instead of the previous
  #if BITS_PER_LONG guard.  There are no 32-bit users.
  The survey: https://lore.kernel.org/ceph-devel/CAOJNxR+XiUxR1GNUgd8T18x2KH8kstWtpxwL-mwwE0i_qospJA@mail.gmail.com/T/#t
- Fix two plain reads of oldest_tid in the writer paths to use
  READ_ONCE() for consistency with the lockless read side.
- Replace the original replay_unsafe_requests() change with a
  proper collect-then-replay pattern: unsafe list entries and
  matching old xarray entries are gathered under the mutex with a
  reference taken, then replayed outside it.  Taking a reference
  ensures a concurrent reply handler cannot free an entry out
  from under the local-list iterator.
- Rewrite all commit messages as descriptive prose, focusing on
  the problem and the approach rather than enumerating modified
  functions.
- Link to v2: https://patch.msgid.link/20260715-ceph-mdsc-mutex-optimization-v2-0-90e81b726724@clyso.com

Changes in v2:
- Add xa_store() error handling and bail out on failure in the submit path
- Guard xarray conversion with BITS_PER_LONG==64, fall back to rbtree on 32-bit
- Fix missing mutex_unlock on early-return path in __do_request()
- Move mutex_unlock before __wake_requests() and kick_requests() calls to
  avoid recursive lock acquisition
- Pin requests with ceph_mdsc_get_request() across lockless __send_request()
  in replay_unsafe_requests() to prevent use-after-free
- Keep mdsc->mutex held on 32-bit for rb_first()/rb_next() iteration in
  replay_unsafe_requests()
- Drop stale "called under mdsc->mutex" comment on __wake_requests()
- Add benchmark results from test_i_caps (2.1x stat throughput improvement)
- Link to v1: https://patch.msgid.link/20260713-ceph-mdsc-mutex-optimization-v1-0-9ae5ac135c34@clyso.com

To: Ilya Dryomov <idryomov@gmail.com>
To: Alex Markuze <amarkuze@redhat.com>
To: Viacheslav Dubeyko <slava@dubeyko.com>
Cc: ceph-devel@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Xiubo Li (5):
      ceph: use READ_ONCE/WRITE_ONCE for oldest_tid
      ceph: replace the request_tree rbtree with an xarray keyed by r_tid
      ceph: add wait_list_lock for wait-list serialization
      ceph: move mdsc->mutex into __do_request()
      ceph: narrow mdsc->mutex scope in replay_unsafe_requests

 fs/ceph/Kconfig      |   1 +
 fs/ceph/debugfs.c    |   6 +-
 fs/ceph/mds_client.c | 530 +++++++++++++++++++++++++++++++++++++++------------
 fs/ceph/mds_client.h |  35 +++-
 4 files changed, 440 insertions(+), 132 deletions(-)
---
base-commit: dee30ce1286a0d18b14545ecac345e4cf4a80511
change-id: 20260713-ceph-mdsc-mutex-optimization-7e74ab6bbc8b

Best regards,
--  
Xiubo Li <xiubo.li@clyso.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v6 1/5] ceph: use READ_ONCE/WRITE_ONCE for oldest_tid
  2026-08-29 11:35 [PATCH v6 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient Xiubo Li via B4 Relay
@ 2026-08-29 11:35 ` Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid Xiubo Li via B4 Relay
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-08-29 11:35 UTC (permalink / raw)
  To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
  Cc: ceph-devel, linux-kernel, Xiubo Li

From: Xiubo Li <xiubo.li@clyso.com>

The oldest_client_tid sent in the MDS request header is advisory:
a stale value is harmless --- at worst the MDS may resend a reply
the client already has, or trim its completed-requests table
slightly earlier or later than optimal, both of which the protocol
handles correctly.  The field is monotonic and does not need lock
serialization to stay correct.

Switch all accesses to READ_ONCE() and WRITE_ONCE() to prevent
the compiler from tearing or inventing loads, documenting that
these lockless accesses are intentional.  This removes the last
reason the MDS request-send path had to be serialized under
mdsc->mutex.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
 fs/ceph/mds_client.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 85f8ceb10377..b84d04835329 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1309,8 +1309,8 @@ static void __register_request(struct ceph_mds_client *mdsc,
 	if (!req->r_mnt_idmap)
 		req->r_mnt_idmap = &nop_mnt_idmap;
 
-	if (mdsc->oldest_tid == 0 && req->r_op != CEPH_MDS_OP_SETFILELOCK)
-		mdsc->oldest_tid = req->r_tid;
+	if (READ_ONCE(mdsc->oldest_tid) == 0 && req->r_op != CEPH_MDS_OP_SETFILELOCK)
+		WRITE_ONCE(mdsc->oldest_tid, req->r_tid);
 
 	if (dir) {
 		struct ceph_inode_info *ci = ceph_inode(dir);
@@ -1331,14 +1331,14 @@ static void __unregister_request(struct ceph_mds_client *mdsc,
 	/* Never leave an unregistered request on an unsafe list! */
 	list_del_init(&req->r_unsafe_item);
 
-	if (req->r_tid == mdsc->oldest_tid) {
+	if (req->r_tid == READ_ONCE(mdsc->oldest_tid)) {
 		struct rb_node *p = rb_next(&req->r_node);
-		mdsc->oldest_tid = 0;
+		WRITE_ONCE(mdsc->oldest_tid, 0);
 		while (p) {
 			struct ceph_mds_request *next_req =
 				rb_entry(p, struct ceph_mds_request, r_node);
 			if (next_req->r_op != CEPH_MDS_OP_SETFILELOCK) {
-				mdsc->oldest_tid = next_req->r_tid;
+				WRITE_ONCE(mdsc->oldest_tid, next_req->r_tid);
 				break;
 			}
 			p = rb_next(p);
@@ -1767,7 +1767,7 @@ create_session_full_msg(struct ceph_mds_client *mdsc, int op, u64 seq)
 	ceph_encode_32(&p, 0);
 
 	/* version == 7, oldest_client_tid */
-	ceph_encode_64(&p, mdsc->oldest_tid);
+	ceph_encode_64(&p, READ_ONCE(mdsc->oldest_tid));
 
 	msg->front.iov_len = p - msg->front.iov_base;
 	msg->hdr.front_len = cpu_to_le32(msg->front.iov_len);
@@ -2834,7 +2834,7 @@ static struct ceph_mds_request *__get_oldest_req(struct ceph_mds_client *mdsc)
 
 static inline  u64 __get_oldest_tid(struct ceph_mds_client *mdsc)
 {
-	return mdsc->oldest_tid;
+	return READ_ONCE(mdsc->oldest_tid);
 }
 
 #if IS_ENABLED(CONFIG_FS_ENCRYPTION)
@@ -3513,9 +3513,6 @@ static void complete_request(struct ceph_mds_client *mdsc,
 	complete_all(&req->r_completion);
 }
 
-/*
- * called under mdsc->mutex
- */
 static int __prepare_send_request(struct ceph_mds_session *session,
 				  struct ceph_mds_request *req,
 				  bool drop_cap_releases)
@@ -3630,9 +3627,6 @@ static int __prepare_send_request(struct ceph_mds_session *session,
 	return 0;
 }
 
-/*
- * called under mdsc->mutex
- */
 static int __send_request(struct ceph_mds_session *session,
 			  struct ceph_mds_request *req,
 			  bool drop_cap_releases)

-- 
2.53.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v6 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid
  2026-08-29 11:35 [PATCH v6 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 1/5] ceph: use READ_ONCE/WRITE_ONCE for oldest_tid Xiubo Li via B4 Relay
@ 2026-08-29 11:35 ` Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 3/5] ceph: add wait_list_lock for wait-list serialization Xiubo Li via B4 Relay
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-08-29 11:35 UTC (permalink / raw)
  To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
  Cc: ceph-devel, linux-kernel, Xiubo Li

From: Xiubo Li <xiubo.li@clyso.com>

Replace the red-black tree that indexes pending MDS requests by
transaction ID with an xarray.  The xarray provides O(1) keyed
lookups versus the rbtree's O(log N), and its internal RCU
locking eliminates the need for an external mutex during lookups.
Iteration is also simpler, with the xarray's native iterators
replacing open-coded rb_first() / rb_next() walks.

Because xarray indices are unsigned long, a u64 transaction ID
would be truncated on 32-bit platforms.  Restrict CEPH_FS to
64BIT since there are no 32-bit users.

Validate the xarray insertion and clean up the structure at
shutdown.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
 fs/ceph/Kconfig      |   1 +
 fs/ceph/debugfs.c    |   6 +--
 fs/ceph/mds_client.c | 113 +++++++++++++++++++++++----------------------------
 fs/ceph/mds_client.h |   3 +-
 4 files changed, 56 insertions(+), 67 deletions(-)

diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
index be1c8ada5665..a703071a1089 100644
--- a/fs/ceph/Kconfig
+++ b/fs/ceph/Kconfig
@@ -2,6 +2,7 @@
 config CEPH_FS
 	tristate "Ceph distributed file system"
 	depends on INET
+	depends on 64BIT
 	select CEPH_LIB
 	select NETFS_SUPPORT
 	select FS_ENCRYPTION_ALGS if FS_ENCRYPTION
diff --git a/fs/ceph/debugfs.c b/fs/ceph/debugfs.c
index 18eb5da03411..491ead3fe1c6 100644
--- a/fs/ceph/debugfs.c
+++ b/fs/ceph/debugfs.c
@@ -87,12 +87,12 @@ static int mdsc_show(struct seq_file *s, void *p)
 	struct ceph_fs_client *fsc = s->private;
 	struct ceph_mds_client *mdsc = fsc->mdsc;
 	struct ceph_mds_request *req;
-	struct rb_node *rp;
+	unsigned long idx;
 	char *path;
 
 	mutex_lock(&mdsc->mutex);
-	for (rp = rb_first(&mdsc->request_tree); rp; rp = rb_next(rp)) {
-		req = rb_entry(rp, struct ceph_mds_request, r_node);
+	idx = 0;
+	xa_for_each(&mdsc->request_tree, idx, req) {
 
 		if (req->r_request && req->r_session)
 			seq_printf(s, "%lld\tmds%d\t", req->r_tid,
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index b84d04835329..fdaf6f56ecd3 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1257,7 +1257,6 @@ void ceph_mdsc_release_request(struct kref *kref)
 	kmem_cache_free(ceph_mds_request_cachep, req);
 }
 
-DEFINE_RB_FUNCS(request, struct ceph_mds_request, r_tid, r_node)
 
 /*
  * lookup session, bump ref if found.
@@ -1269,7 +1268,7 @@ lookup_get_request(struct ceph_mds_client *mdsc, u64 tid)
 {
 	struct ceph_mds_request *req;
 
-	req = lookup_request(&mdsc->request_tree, tid);
+	req = xa_load(&mdsc->request_tree, tid);
 	if (req)
 		ceph_mdsc_get_request(req);
 
@@ -1303,7 +1302,14 @@ static void __register_request(struct ceph_mds_client *mdsc,
 	}
 	doutc(cl, "%p tid %lld\n", req, req->r_tid);
 	ceph_mdsc_get_request(req);
-	insert_request(&mdsc->request_tree, req);
+	if (xa_is_err(xa_store(&mdsc->request_tree, req->r_tid, req,
+			       GFP_NOFS))) {
+		pr_err_client(cl, "%p tid %lld: xa_store failed\n",
+			      req, req->r_tid);
+		ceph_mdsc_put_request(req);
+		req->r_err = -ENOMEM;
+		return;
+	}
 
 	req->r_cred = get_current_cred();
 	if (!req->r_mnt_idmap)
@@ -1332,20 +1338,19 @@ static void __unregister_request(struct ceph_mds_client *mdsc,
 	list_del_init(&req->r_unsafe_item);
 
 	if (req->r_tid == READ_ONCE(mdsc->oldest_tid)) {
-		struct rb_node *p = rb_next(&req->r_node);
+		unsigned long tidx = req->r_tid + 1;
+		struct ceph_mds_request *next_req;
+
 		WRITE_ONCE(mdsc->oldest_tid, 0);
-		while (p) {
-			struct ceph_mds_request *next_req =
-				rb_entry(p, struct ceph_mds_request, r_node);
+		xa_for_each_start(&mdsc->request_tree, tidx, next_req, tidx) {
 			if (next_req->r_op != CEPH_MDS_OP_SETFILELOCK) {
 				WRITE_ONCE(mdsc->oldest_tid, next_req->r_tid);
 				break;
 			}
-			p = rb_next(p);
 		}
 	}
 
-	erase_request(&mdsc->request_tree, req);
+	xa_erase(&mdsc->request_tree, req->r_tid);
 
 	if (req->r_unsafe_dir) {
 		struct ceph_inode_info *ci = ceph_inode(req->r_unsafe_dir);
@@ -1905,7 +1910,7 @@ static void cleanup_session_requests(struct ceph_mds_client *mdsc,
 {
 	struct ceph_client *cl = mdsc->fsc->client;
 	struct ceph_mds_request *req;
-	struct rb_node *p;
+	unsigned long idx;
 
 	doutc(cl, "mds%d\n", session->s_mds);
 	mutex_lock(&mdsc->mutex);
@@ -1921,10 +1926,8 @@ static void cleanup_session_requests(struct ceph_mds_client *mdsc,
 		__unregister_request(mdsc, req);
 	}
 	/* zero r_attempts, so kick_requests() will re-send requests */
-	p = rb_first(&mdsc->request_tree);
-	while (p) {
-		req = rb_entry(p, struct ceph_mds_request, r_node);
-		p = rb_next(p);
+	idx = 0;
+	xa_for_each(&mdsc->request_tree, idx, req) {
 		if (req->r_session &&
 		    req->r_session->s_mds == session->s_mds)
 			req->r_attempts = 0;
@@ -2806,7 +2809,6 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
 	req->r_fmode = -1;
 	req->r_feature_needed = -1;
 	kref_init(&req->r_kref);
-	RB_CLEAR_NODE(&req->r_node);
 	INIT_LIST_HEAD(&req->r_wait);
 	init_completion(&req->r_completion);
 	init_completion(&req->r_safe_completion);
@@ -2826,10 +2828,9 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
  */
 static struct ceph_mds_request *__get_oldest_req(struct ceph_mds_client *mdsc)
 {
-	if (RB_EMPTY_ROOT(&mdsc->request_tree))
-		return NULL;
-	return rb_entry(rb_first(&mdsc->request_tree),
-			struct ceph_mds_request, r_node);
+	unsigned long idx = 0;
+
+	return xa_find(&mdsc->request_tree, &idx, ULONG_MAX, XA_PRESENT);
 }
 
 static inline  u64 __get_oldest_tid(struct ceph_mds_client *mdsc)
@@ -3890,12 +3891,11 @@ static void kick_requests(struct ceph_mds_client *mdsc, int mds)
 {
 	struct ceph_client *cl = mdsc->fsc->client;
 	struct ceph_mds_request *req;
-	struct rb_node *p = rb_first(&mdsc->request_tree);
+	unsigned long idx;
 
 	doutc(cl, "kick_requests mds%d\n", mds);
-	while (p) {
-		req = rb_entry(p, struct ceph_mds_request, r_node);
-		p = rb_next(p);
+	idx = 0;
+	xa_for_each(&mdsc->request_tree, idx, req) {
 		if (test_bit(CEPH_MDS_R_GOT_UNSAFE, &req->r_req_flags))
 			continue;
 		if (req->r_attempts > 0)
@@ -4748,7 +4748,7 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 				   struct ceph_mds_session *session)
 {
 	struct ceph_mds_request *req, *nreq;
-	struct rb_node *p;
+	unsigned long idx;
 
 	doutc(mdsc->fsc->client, "mds%d\n", session->s_mds);
 
@@ -4760,10 +4760,8 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 	 * also re-send old requests when MDS enters reconnect stage. So that MDS
 	 * can process completed request in clientreplay stage.
 	 */
-	p = rb_first(&mdsc->request_tree);
-	while (p) {
-		req = rb_entry(p, struct ceph_mds_request, r_node);
-		p = rb_next(p);
+	idx = 0;
+	xa_for_each(&mdsc->request_tree, idx, req) {
 		if (test_bit(CEPH_MDS_R_GOT_UNSAFE, &req->r_req_flags))
 			continue;
 		if (req->r_attempts == 0)
@@ -5609,7 +5607,7 @@ static void ceph_mdsc_reset_workfn(struct work_struct *work)
 	 */
 	{
 		struct ceph_mds_request *req;
-		struct rb_node *rn;
+		unsigned long idx;
 		u64 last_tid;
 
 		mutex_lock(&mdsc->mutex);
@@ -5617,14 +5615,12 @@ static void ceph_mdsc_reset_workfn(struct work_struct *work)
 		mutex_unlock(&mdsc->mutex);
 
 		mutex_lock(&mdsc->mutex);
-		rn = rb_first(&mdsc->request_tree);
-		while (rn) {
-			req = rb_entry(rn, struct ceph_mds_request, r_node);
-			if (req->r_tid > last_tid)
-				break;
+		idx = 0;
+		while ((req = xa_find(&mdsc->request_tree, &idx, last_tid,
+				      XA_PRESENT))) {
 			if (req->r_op == CEPH_MDS_OP_SETFILELOCK ||
 			    !(req->r_op & CEPH_MDS_OP_WRITE)) {
-				rn = rb_next(rn);
+				idx++;
 				continue;
 			}
 			ceph_mdsc_get_request(req);
@@ -5637,7 +5633,7 @@ static void ceph_mdsc_reset_workfn(struct work_struct *work)
 			ceph_mdsc_put_request(req);
 			if (time_after(jiffies, drain_deadline))
 				break;
-			rn = rb_first(&mdsc->request_tree);
+			idx = 0;  /* restart: tree may have changed */
 		}
 		mutex_unlock(&mdsc->mutex);
 
@@ -6369,7 +6365,7 @@ int ceph_mdsc_init(struct ceph_fs_client *fsc)
 	mdsc->snap_realms = RB_ROOT;
 	INIT_LIST_HEAD(&mdsc->snap_empty);
 	spin_lock_init(&mdsc->snap_empty_lock);
-	mdsc->request_tree = RB_ROOT;
+	xa_init(&mdsc->request_tree);
 	INIT_DELAYED_WORK(&mdsc->delayed_work, delayed_work);
 	mdsc->last_renew_caps = jiffies;
 	INIT_LIST_HEAD(&mdsc->cap_delay_list);
@@ -6695,34 +6691,33 @@ static void flush_mdlog_and_wait_mdsc_unsafe_requests(struct ceph_mds_client *md
 						 u64 want_tid)
 {
 	struct ceph_client *cl = mdsc->fsc->client;
-	struct ceph_mds_request *req = NULL, *nextreq;
+	struct ceph_mds_request *req;
 	struct ceph_mds_session *last_session = NULL;
-	struct rb_node *n;
+	unsigned long idx;
 
 	mutex_lock(&mdsc->mutex);
 	doutc(cl, "want %lld\n", want_tid);
-restart:
-	req = __get_oldest_req(mdsc);
-	while (req && req->r_tid <= want_tid) {
-		/* find next request */
-		n = rb_next(&req->r_node);
-		if (n)
-			nextreq = rb_entry(n, struct ceph_mds_request, r_node);
-		else
-			nextreq = NULL;
-		if (req->r_op != CEPH_MDS_OP_SETFILELOCK &&
-		    (req->r_op & CEPH_MDS_OP_WRITE)) {
+	idx = 0;
+	while ((req = xa_find(&mdsc->request_tree, &idx, want_tid,
+			      XA_PRESENT))) {
+		u64 next_tid = req->r_tid + 1;
+
+		if (req->r_op == CEPH_MDS_OP_SETFILELOCK ||
+		    !(req->r_op & CEPH_MDS_OP_WRITE)) {
+			idx = next_tid;
+			continue;
+		}
+
+		{
 			struct ceph_mds_session *s = req->r_session;
 
 			if (!s) {
-				req = nextreq;
+				idx = next_tid;
 				continue;
 			}
 
 			/* write op */
 			ceph_mdsc_get_request(req);
-			if (nextreq)
-				ceph_mdsc_get_request(nextreq);
 			s = ceph_get_mds_session(s);
 			mutex_unlock(&mdsc->mutex);
 
@@ -6740,16 +6735,9 @@ static void flush_mdlog_and_wait_mdsc_unsafe_requests(struct ceph_mds_client *md
 
 			mutex_lock(&mdsc->mutex);
 			ceph_mdsc_put_request(req);
-			if (!nextreq)
-				break;  /* next dne before, so we're done! */
-			if (RB_EMPTY_NODE(&nextreq->r_node)) {
-				/* next request was removed from tree */
-				ceph_mdsc_put_request(nextreq);
-				goto restart;
-			}
-			ceph_mdsc_put_request(nextreq);  /* won't go away */
+			/* restart from the next tid; tree may have changed */
+			idx = next_tid;
 		}
-		req = nextreq;
 	}
 	mutex_unlock(&mdsc->mutex);
 	ceph_put_mds_session(last_session);
@@ -6908,6 +6896,7 @@ static void ceph_mdsc_stop(struct ceph_mds_client *mdsc)
 	if (mdsc->mdsmap)
 		ceph_mdsmap_destroy(mdsc->mdsmap);
 	kfree(mdsc->sessions);
+	xa_destroy(&mdsc->request_tree);
 	ceph_caps_finalize(mdsc);
 
 	if (mdsc->s_cap_auths) {
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index e7a262c9c2ab..baba5dcf0def 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -331,7 +331,6 @@ typedef int (*ceph_mds_request_wait_callback_t) (struct ceph_mds_client *mdsc,
  */
 struct ceph_mds_request {
 	u64 r_tid;                   /* transaction id */
-	struct rb_node r_node;
 	struct ceph_mds_client *r_mdsc;
 
 	struct kref       r_kref;
@@ -536,7 +535,7 @@ struct ceph_mds_client {
 	u64                    last_tid;      /* most recent mds request */
 	u64                    oldest_tid;    /* oldest incomplete mds request,
 						 excluding setfilelock requests */
-	struct rb_root         request_tree;  /* pending mds requests */
+	struct xarray           request_tree;  /* pending mds requests */
 	struct delayed_work    delayed_work;  /* delayed work */
 	unsigned long    last_renew_caps;  /* last time we renewed our caps */
 	struct list_head cap_delay_list;   /* caps with delayed release */

-- 
2.53.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v6 3/5] ceph: add wait_list_lock for wait-list serialization
  2026-08-29 11:35 [PATCH v6 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 1/5] ceph: use READ_ONCE/WRITE_ONCE for oldest_tid Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid Xiubo Li via B4 Relay
@ 2026-08-29 11:35 ` Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 4/5] ceph: move mdsc->mutex into __do_request() Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests Xiubo Li via B4 Relay
  4 siblings, 0 replies; 6+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-08-29 11:35 UTC (permalink / raw)
  To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
  Cc: ceph-devel, linux-kernel, Xiubo Li

From: Xiubo Li <xiubo.li@clyso.com>

The per-MDS session wait list and the global waiting-for-map list
are currently serialized by mdsc->mutex, even though the list
operations themselves don't need the mutex's broader protection.
Introduce a dedicated spinlock to guard these lists so that
waking and kicking waiters can run outside the mutex.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
 fs/ceph/mds_client.c | 18 +++++++++++++++++-
 fs/ceph/mds_client.h |  3 +++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index fdaf6f56ecd3..5983ae6e3085 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3689,7 +3689,9 @@ static void __do_request(struct ceph_mds_client *mdsc,
 			doutc(cl, "no mdsmap, waiting for map\n");
 			trace_ceph_mdsc_suspend_request(mdsc, session, req,
 							ceph_mdsc_suspend_reason_no_mdsmap);
+			spin_lock(&mdsc->wait_list_lock);
 			list_add(&req->r_wait, &mdsc->waiting_for_map);
+			spin_unlock(&mdsc->wait_list_lock);
 			return;
 		}
 		if (!(mdsc->fsc->mount_options->flags &
@@ -3712,7 +3714,9 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		doutc(cl, "no mds or not active, waiting for map\n");
 		trace_ceph_mdsc_suspend_request(mdsc, session, req,
 						ceph_mdsc_suspend_reason_no_active_mds);
+		spin_lock(&mdsc->wait_list_lock);
 		list_add(&req->r_wait, &mdsc->waiting_for_map);
+		spin_unlock(&mdsc->wait_list_lock);
 		return;
 	}
 
@@ -3760,9 +3764,12 @@ static void __do_request(struct ceph_mds_client *mdsc,
 			if (ceph_test_mount_opt(mdsc->fsc, CLEANRECOVER)) {
 				trace_ceph_mdsc_suspend_request(mdsc, session, req,
 								ceph_mdsc_suspend_reason_rejected);
+				spin_lock(&mdsc->wait_list_lock);
 				list_add(&req->r_wait, &mdsc->waiting_for_map);
-			} else
+				spin_unlock(&mdsc->wait_list_lock);
+			} else {
 				err = -EACCES;
+			}
 			goto out_session;
 		}
 
@@ -3777,7 +3784,9 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		}
 		trace_ceph_mdsc_suspend_request(mdsc, session, req,
 						ceph_mdsc_suspend_reason_session);
+		spin_lock(&mdsc->wait_list_lock);
 		list_add(&req->r_wait, &session->s_waiting);
+		spin_unlock(&mdsc->wait_list_lock);
 		goto out_session;
 	}
 
@@ -3870,7 +3879,9 @@ static void __wake_requests(struct ceph_mds_client *mdsc,
 	struct ceph_mds_request *req;
 	LIST_HEAD(tmp_list);
 
+	spin_lock(&mdsc->wait_list_lock);
 	list_splice_init(head, &tmp_list);
+	spin_unlock(&mdsc->wait_list_lock);
 
 	while (!list_empty(&tmp_list)) {
 		req = list_entry(tmp_list.next,
@@ -3903,7 +3914,9 @@ static void kick_requests(struct ceph_mds_client *mdsc, int mds)
 		if (req->r_session &&
 		    req->r_session->s_mds == mds) {
 			doutc(cl, " kicking tid %llu\n", req->r_tid);
+			spin_lock(&mdsc->wait_list_lock);
 			list_del_init(&req->r_wait);
+			spin_unlock(&mdsc->wait_list_lock);
 			trace_ceph_mdsc_resume_request(mdsc, req);
 			__do_request(mdsc, req);
 		}
@@ -6365,6 +6378,7 @@ int ceph_mdsc_init(struct ceph_fs_client *fsc)
 	mdsc->snap_realms = RB_ROOT;
 	INIT_LIST_HEAD(&mdsc->snap_empty);
 	spin_lock_init(&mdsc->snap_empty_lock);
+	spin_lock_init(&mdsc->wait_list_lock);
 	xa_init(&mdsc->request_tree);
 	INIT_DELAYED_WORK(&mdsc->delayed_work, delayed_work);
 	mdsc->last_renew_caps = jiffies;
@@ -6447,7 +6461,9 @@ static void wait_requests(struct ceph_mds_client *mdsc)
 		mutex_lock(&mdsc->mutex);
 		while ((req = __get_oldest_req(mdsc))) {
 			doutc(cl, "timed out on tid %llu\n", req->r_tid);
+			spin_lock(&mdsc->wait_list_lock);
 			list_del_init(&req->r_wait);
+			spin_unlock(&mdsc->wait_list_lock);
 			__unregister_request(mdsc, req);
 		}
 	}
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index baba5dcf0def..0ea144ba4c7d 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -531,6 +531,9 @@ struct ceph_mds_client {
 	struct list_head        snap_empty;
 	int			num_snap_realms;
 	spinlock_t              snap_empty_lock;  /* protect snap_empty */
+	spinlock_t              wait_list_lock;   /* protect waiting_for_map
+						   * and s_waiting lists
+						   */
 
 	u64                    last_tid;      /* most recent mds request */
 	u64                    oldest_tid;    /* oldest incomplete mds request,

-- 
2.53.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v6 4/5] ceph: move mdsc->mutex into __do_request()
  2026-08-29 11:35 [PATCH v6 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient Xiubo Li via B4 Relay
                   ` (2 preceding siblings ...)
  2026-08-29 11:35 ` [PATCH v6 3/5] ceph: add wait_list_lock for wait-list serialization Xiubo Li via B4 Relay
@ 2026-08-29 11:35 ` Xiubo Li via B4 Relay
  2026-08-29 11:35 ` [PATCH v6 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests Xiubo Li via B4 Relay
  4 siblings, 0 replies; 6+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-08-29 11:35 UTC (permalink / raw)
  To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
  Cc: ceph-devel, linux-kernel, Xiubo Li

From: Xiubo Li <xiubo.li@clyso.com>

Currently every caller must hold mdsc->mutex when invoking the
request-send machinery.  Move the mutex acquisition inside
__do_request() so that callers can fire off a request without
first serializing on the global lock.  The mutex is released
before the network send phase and re-acquired only for cleanup,
so dentry traversal and message construction run concurrently
across CPUs.

This is the primary source of the observed 2x stat throughput
improvement: the per-request send path shrinks from hundreds of
microseconds to tens of microseconds once it no longer waits on
the mutex.  Wait-list draining, session-state wake-ups, and
request kicking are reworked to either use the new wait-list
spinlock or collect candidates under the mutex and process them
outside it.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
 fs/ceph/mds_client.c | 118 +++++++++++++++++++++++++++++++++++----------------
 fs/ceph/mds_client.h |   1 +
 2 files changed, 83 insertions(+), 36 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 5983ae6e3085..0b515df15081 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2813,6 +2813,7 @@ ceph_mdsc_create_request(struct ceph_mds_client *mdsc, int op, int mode)
 	init_completion(&req->r_completion);
 	init_completion(&req->r_safe_completion);
 	INIT_LIST_HEAD(&req->r_unsafe_item);
+	INIT_LIST_HEAD(&req->r_aux_item);
 
 	ktime_get_coarse_real_ts64(&req->r_stamp);
 
@@ -3532,20 +3533,23 @@ static int __prepare_send_request(struct ceph_mds_session *session,
 	 * Avoid infinite retrying after overflow. The client will
 	 * increase the retry count and if the MDS is old version,
 	 * so we limit to retry at most 256 times.
+	 *
+	 * r_attempts was already incremented under mdsc->mutex by the
+	 * caller (__do_request or replay_unsafe_requests), so the
+	 * actual retry count is r_attempts - 1 and we skip the check
+	 * on the first dispatch (r_attempts == 1).
 	 */
-	if (req->r_attempts) {
-	       old_max_retry = sizeof_field(struct ceph_mds_request_head,
-					    num_retry);
-	       old_max_retry = 1 << (old_max_retry * BITS_PER_BYTE);
-	       if ((old_version && req->r_attempts >= old_max_retry) ||
-		   ((uint32_t)req->r_attempts >= U32_MAX)) {
+	if (req->r_attempts > 1) {
+		old_max_retry = sizeof_field(struct ceph_mds_request_head,
+					     num_retry);
+		old_max_retry = 1 << (old_max_retry * BITS_PER_BYTE);
+		if ((old_version && (req->r_attempts - 1) >= old_max_retry) ||
+		    ((uint32_t)(req->r_attempts - 1) >= U32_MAX)) {
 			pr_warn_ratelimited_client(cl, "request tid %llu seq overflow\n",
 						   req->r_tid);
 			return -EMULTIHOP;
-	       }
+		}
 	}
-
-	req->r_attempts++;
 	if (req->r_inode) {
 		struct ceph_cap *cap =
 			ceph_get_cap_for_mds(ceph_inode(req->r_inode), mds);
@@ -3657,9 +3661,36 @@ static void __do_request(struct ceph_mds_client *mdsc,
 	int err = 0;
 	bool random;
 
+	mutex_lock(&mdsc->mutex);
+
+	/*
+	 * r_attempts is bumped under mdsc->mutex just before the mutex
+	 * is dropped to send the request, and it is only ever written
+	 * back to 0 by the forward handler or cleanup_session_requests()
+	 * (both under the mutex) before re-dispatching through a new
+	 * __do_request() call.  Consequently, r_attempts > 0 at this
+	 * point always means another __do_request() instance has already
+	 * passed the point of no return for this request, i.e. a racing
+	 * kick_requests() or __wake_requests() picked up the same
+	 * request from the xarray or a wait list and is about to send
+	 * it.  Bail out to prevent a double dispatch.
+	 *
+	 * Note: kick_requests() also filters on r_attempts > 0 during
+	 * its collection pass, but that is a one-time snapshot taken
+	 * under the mutex.  Between that snapshot and the actual
+	 * __do_request() call the mutex is dropped and re-acquired, so
+	 * the protection is not atomic — this per-request gate closes
+	 * the remaining window.
+	 */
+	if (req->r_attempts > 0) {
+		mutex_unlock(&mdsc->mutex);
+		return;
+	}
+
 	if (req->r_err || test_bit(CEPH_MDS_R_GOT_RESULT, &req->r_req_flags)) {
 		if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags))
 			__unregister_request(mdsc, req);
+		mutex_unlock(&mdsc->mutex);
 		return;
 	}
 
@@ -3692,6 +3723,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 			spin_lock(&mdsc->wait_list_lock);
 			list_add(&req->r_wait, &mdsc->waiting_for_map);
 			spin_unlock(&mdsc->wait_list_lock);
+			mutex_unlock(&mdsc->mutex);
 			return;
 		}
 		if (!(mdsc->fsc->mount_options->flags &
@@ -3717,6 +3749,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		spin_lock(&mdsc->wait_list_lock);
 		list_add(&req->r_wait, &mdsc->waiting_for_map);
 		spin_unlock(&mdsc->wait_list_lock);
+		mutex_unlock(&mdsc->mutex);
 		return;
 	}
 
@@ -3790,6 +3823,9 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		goto out_session;
 	}
 
+	req->r_attempts++;
+	mutex_unlock(&mdsc->mutex);
+
 	/* send request */
 	req->r_resend_mds = -1;   /* forget any previous mds hint */
 
@@ -3820,6 +3856,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 			err = wait_on_bit(&di->flags, CEPH_DENTRY_ASYNC_CREATE_BIT,
 					  TASK_KILLABLE);
 			if (err) {
+				mutex_lock(&mdsc->mutex);
 				mutex_lock(&req->r_fill_mutex);
 				set_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags);
 				mutex_unlock(&req->r_fill_mutex);
@@ -3857,6 +3894,8 @@ static void __do_request(struct ceph_mds_client *mdsc,
 
 	err = __send_request(session, req, false);
 
+	mutex_lock(&mdsc->mutex);
+
 out_session:
 	ceph_put_mds_session(session);
 finish:
@@ -3866,12 +3905,10 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		complete_request(mdsc, req);
 		__unregister_request(mdsc, req);
 	}
+	mutex_unlock(&mdsc->mutex);
 	return;
 }
 
-/*
- * called under mdsc->mutex
- */
 static void __wake_requests(struct ceph_mds_client *mdsc,
 			    struct list_head *head)
 {
@@ -3901,10 +3938,14 @@ static void __wake_requests(struct ceph_mds_client *mdsc,
 static void kick_requests(struct ceph_mds_client *mdsc, int mds)
 {
 	struct ceph_client *cl = mdsc->fsc->client;
-	struct ceph_mds_request *req;
+	struct ceph_mds_request *req, *nreq;
 	unsigned long idx;
+	LIST_HEAD(kick_list);
 
 	doutc(cl, "kick_requests mds%d\n", mds);
+
+	/* collect matching requests under the mutex */
+	mutex_lock(&mdsc->mutex);
 	idx = 0;
 	xa_for_each(&mdsc->request_tree, idx, req) {
 		if (test_bit(CEPH_MDS_R_GOT_UNSAFE, &req->r_req_flags))
@@ -3913,14 +3954,23 @@ static void kick_requests(struct ceph_mds_client *mdsc, int mds)
 			continue; /* only new requests */
 		if (req->r_session &&
 		    req->r_session->s_mds == mds) {
-			doutc(cl, " kicking tid %llu\n", req->r_tid);
+			ceph_mdsc_get_request(req);
 			spin_lock(&mdsc->wait_list_lock);
 			list_del_init(&req->r_wait);
 			spin_unlock(&mdsc->wait_list_lock);
-			trace_ceph_mdsc_resume_request(mdsc, req);
-			__do_request(mdsc, req);
+			list_add_tail(&req->r_aux_item, &kick_list);
 		}
 	}
+	mutex_unlock(&mdsc->mutex);
+
+	/* replay without the mutex */
+	list_for_each_entry_safe(req, nreq, &kick_list, r_aux_item) {
+		doutc(cl, " kicking tid %llu\n", req->r_tid);
+		trace_ceph_mdsc_resume_request(mdsc, req);
+		list_del_init(&req->r_aux_item);
+		__do_request(mdsc, req);
+		ceph_mdsc_put_request(req);
+	}
 }
 
 int ceph_mdsc_submit_request(struct ceph_mds_client *mdsc, struct inode *dir,
@@ -3980,10 +4030,11 @@ int ceph_mdsc_submit_request(struct ceph_mds_client *mdsc, struct inode *dir,
 	doutc(cl, "submit_request on %p for inode %p\n", req, dir);
 	mutex_lock(&mdsc->mutex);
 	__register_request(mdsc, req, dir);
+	mutex_unlock(&mdsc->mutex);
+
 	trace_ceph_mdsc_submit_request(mdsc, req);
 	__do_request(mdsc, req);
 	err = req->r_err;
-	mutex_unlock(&mdsc->mutex);
 	return err;
 }
 
@@ -4372,13 +4423,14 @@ static void handle_forward(struct ceph_mds_client *mdsc,
 		req->r_num_fwd = fwd_seq;
 		req->r_resend_mds = next_mds;
 		put_request_session(req);
-		__do_request(mdsc, req);
 	}
 	mutex_unlock(&mdsc->mutex);
 
 	/* kick calling process */
 	if (aborted)
 		complete_request(mdsc, req);
+	else if (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags))
+		__do_request(mdsc, req);
 	ceph_mdsc_put_request(req);
 	return;
 
@@ -4706,11 +4758,9 @@ static void handle_session(struct ceph_mds_session *session,
 
 	mutex_unlock(&session->s_mutex);
 	if (wake) {
-		mutex_lock(&mdsc->mutex);
 		__wake_requests(mdsc, &session->s_waiting);
 		if (wake == 2)
 			kick_requests(mdsc, mds);
-		mutex_unlock(&mdsc->mutex);
 	}
 	if (op == CEPH_SESSION_CLOSE)
 		ceph_put_mds_session(session);
@@ -4767,6 +4817,7 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 
 	mutex_lock(&mdsc->mutex);
 	list_for_each_entry_safe(req, nreq, &session->s_unsafe, r_unsafe_item)
+		req->r_attempts++;
 		__send_request(session, req, true);
 
 	/*
@@ -4786,6 +4837,7 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 
 		ceph_mdsc_release_dir_caps_async(req);
 
+		req->r_attempts++;
 		__send_request(session, req, true);
 	}
 	mutex_unlock(&mdsc->mutex);
@@ -5346,9 +5398,7 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
 
 	mutex_unlock(&session->s_mutex);
 
-	mutex_lock(&mdsc->mutex);
 	__wake_requests(mdsc, &session->s_waiting);
-	mutex_unlock(&mdsc->mutex);
 
 	up_read(&mdsc->snap_rwsem);
 	ceph_pagelist_release(recon_state.pagelist);
@@ -5773,8 +5823,8 @@ static void ceph_mdsc_reset_workfn(struct work_struct *work)
 		}
 		sessions[i]->s_state = CEPH_MDS_SESSION_CLOSED;
 		__unregister_session(mdsc, sessions[i]);
-		__wake_requests(mdsc, &sessions[i]->s_waiting);
 		mutex_unlock(&mdsc->mutex);
+		__wake_requests(mdsc, &sessions[i]->s_waiting);
 
 		mutex_lock(&sessions[i]->s_mutex);
 		cleanup_session_requests(mdsc, sessions[i]);
@@ -5785,9 +5835,7 @@ static void ceph_mdsc_reset_workfn(struct work_struct *work)
 
 		ceph_put_mds_session(sessions[i]);
 
-		mutex_lock(&mdsc->mutex);
 		kick_requests(mdsc, mds);
-		mutex_unlock(&mdsc->mutex);
 
 		torn_down++;
 		pr_info_client(cl, "mds%d session reset complete\n", mds);
@@ -5903,8 +5951,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 			/* force close session for stopped mds */
 			ceph_get_mds_session(s);
 			__unregister_session(mdsc, s);
-			__wake_requests(mdsc, &s->s_waiting);
 			mutex_unlock(&mdsc->mutex);
+			__wake_requests(mdsc, &s->s_waiting);
 
 			mutex_lock(&s->s_mutex);
 			cleanup_session_requests(mdsc, s);
@@ -5913,8 +5961,8 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 
 			ceph_put_mds_session(s);
 
-			mutex_lock(&mdsc->mutex);
 			kick_requests(mdsc, i);
+			mutex_lock(&mdsc->mutex);
 			continue;
 		}
 
@@ -5962,9 +6010,9 @@ static void check_new_map(struct ceph_mds_client *mdsc,
 			    oldstate != CEPH_MDS_STATE_STARTING)
 				pr_info_client(cl, "mds%d recovery completed\n",
 					       s->s_mds);
-			kick_requests(mdsc, i);
 			ceph_get_mds_session(s);
 			mutex_unlock(&mdsc->mutex);
+			kick_requests(mdsc, i);
 			mutex_lock(&s->s_mutex);
 			mutex_lock(&mdsc->mutex);
 			ceph_put_mds_session(s);
@@ -6877,8 +6925,8 @@ void ceph_mdsc_force_umount(struct ceph_mds_client *mdsc)
 
 		if (session->s_state == CEPH_MDS_SESSION_REJECTED)
 			__unregister_session(mdsc, session);
-		__wake_requests(mdsc, &session->s_waiting);
 		mutex_unlock(&mdsc->mutex);
+		__wake_requests(mdsc, &session->s_waiting);
 
 		mutex_lock(&session->s_mutex);
 		__close_session(mdsc, session);
@@ -6889,11 +6937,11 @@ void ceph_mdsc_force_umount(struct ceph_mds_client *mdsc)
 		mutex_unlock(&session->s_mutex);
 		ceph_put_mds_session(session);
 
-		mutex_lock(&mdsc->mutex);
 		kick_requests(mdsc, mds);
+		mutex_lock(&mdsc->mutex);
 	}
-	__wake_requests(mdsc, &mdsc->waiting_for_map);
 	mutex_unlock(&mdsc->mutex);
+	__wake_requests(mdsc, &mdsc->waiting_for_map);
 }
 
 static void ceph_mdsc_stop(struct ceph_mds_client *mdsc)
@@ -7035,8 +7083,8 @@ void ceph_mdsc_handle_fsmap(struct ceph_mds_client *mdsc, struct ceph_msg *msg)
 err_out:
 	mutex_lock(&mdsc->mutex);
 	mdsc->mdsmap_err = err;
-	__wake_requests(mdsc, &mdsc->waiting_for_map);
 	mutex_unlock(&mdsc->mutex);
+	__wake_requests(mdsc, &mdsc->waiting_for_map);
 }
 
 /*
@@ -7087,11 +7135,11 @@ void ceph_mdsc_handle_mdsmap(struct ceph_mds_client *mdsc, struct ceph_msg *msg)
 	mdsc->fsc->max_file_size = min((loff_t)mdsc->mdsmap->m_max_file_size,
 					MAX_LFS_FILESIZE);
 
+	mutex_unlock(&mdsc->mutex);
 	__wake_requests(mdsc, &mdsc->waiting_for_map);
 	ceph_monc_got_map(&mdsc->fsc->client->monc, CEPH_SUB_MDSMAP,
 			  mdsc->mdsmap->m_epoch);
 
-	mutex_unlock(&mdsc->mutex);
 	schedule_delayed(mdsc, 0);
 	return;
 
@@ -7189,8 +7237,8 @@ static void mds_peer_reset(struct ceph_connection *con)
 		ceph_get_mds_session(s);
 		s->s_state = CEPH_MDS_SESSION_CLOSED;
 		__unregister_session(mdsc, s);
-		__wake_requests(mdsc, &s->s_waiting);
 		mutex_unlock(&mdsc->mutex);
+		__wake_requests(mdsc, &s->s_waiting);
 
 		mutex_lock(&s->s_mutex);
 		cleanup_session_requests(mdsc, s);
@@ -7199,9 +7247,7 @@ static void mds_peer_reset(struct ceph_connection *con)
 
 		wake_up_all(&mdsc->session_close_wq);
 
-		mutex_lock(&mdsc->mutex);
 		kick_requests(mdsc, s->s_mds);
-		mutex_unlock(&mdsc->mutex);
 
 		ceph_put_mds_session(s);
 		break;
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 0ea144ba4c7d..11059678284d 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -428,6 +428,7 @@ struct ceph_mds_request {
 	struct completion r_safe_completion;
 	ceph_mds_request_callback_t r_callback;
 	struct list_head  r_unsafe_item;  /* per-session unsafe list item */
+	struct list_head  r_aux_item;     /* auxiliary local list item */
 
 	long long	  r_dir_release_cnt;
 	long long	  r_dir_ordered_cnt;

-- 
2.53.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v6 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests
  2026-08-29 11:35 [PATCH v6 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient Xiubo Li via B4 Relay
                   ` (3 preceding siblings ...)
  2026-08-29 11:35 ` [PATCH v6 4/5] ceph: move mdsc->mutex into __do_request() Xiubo Li via B4 Relay
@ 2026-08-29 11:35 ` Xiubo Li via B4 Relay
  4 siblings, 0 replies; 6+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-08-29 11:35 UTC (permalink / raw)
  To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
  Cc: ceph-devel, linux-kernel, Xiubo Li

From: Xiubo Li <xiubo.li@clyso.com>

Currently replay_unsafe_requests() holds mdsc->mutex across the
entire function, including the __send_request() calls.  Since
__send_request() is lockless and the async cap-release helper
schedules deferred work, neither needs the mutex.

Collect the unsafe-list entries and the matching old xarray
entries into local lists under mdsc->mutex, taking a reference on
each, then replay them outside the mutex.  Taking a reference
ensures a concurrent reply handler can complete and unregister a
request without invalidating the local list or the iterator.

Unsafe requests remain on session->s_unsafe; r_aux_item serves
only as a walk-list link.  This keeps them tracked as unsafe
until the MDS replies, so a later reconnect can replay them again
and cleanup_session_requests() can still abort them on session
teardown.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
 fs/ceph/mds_client.c | 311 ++++++++++++++++++++++++++++++++++++++++++++-------
 fs/ceph/mds_client.h |  30 ++++-
 2 files changed, 300 insertions(+), 41 deletions(-)

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 0b515df15081..aaf7e11b3d60 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1925,12 +1925,24 @@ static void cleanup_session_requests(struct ceph_mds_client *mdsc,
 			mapping_set_error(req->r_unsafe_dir->i_mapping, -EIO);
 		__unregister_request(mdsc, req);
 	}
-	/* zero r_attempts, so kick_requests() will re-send requests */
+	/*
+	 * Zero r_attempts so that the following kick_requests() will
+	 * re-send the request.  If a dispatch owner is currently in the
+	 * send window (CEPH_MDS_R_DISPATCHING set), a concurrent
+	 * kick_requests() could not pick the request up and the resend
+	 * request would be lost; set CEPH_MDS_R_RESEND instead and let
+	 * the owner re-evaluate the request when it releases dispatch
+	 * ownership.
+	 */
 	idx = 0;
 	xa_for_each(&mdsc->request_tree, idx, req) {
 		if (req->r_session &&
-		    req->r_session->s_mds == session->s_mds)
+		    req->r_session->s_mds == session->s_mds) {
 			req->r_attempts = 0;
+			if (test_bit(CEPH_MDS_R_DISPATCHING,
+				     &req->r_req_flags))
+				set_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
+		}
 	}
 	mutex_unlock(&mdsc->mutex);
 }
@@ -3661,37 +3673,58 @@ static void __do_request(struct ceph_mds_client *mdsc,
 	int err = 0;
 	bool random;
 
+restart:
+	/* re-entry from the resend loop below: reset per-pass state */
+	session = NULL;
+	err = 0;
 	mutex_lock(&mdsc->mutex);
 
 	/*
-	 * r_attempts is bumped under mdsc->mutex just before the mutex
-	 * is dropped to send the request, and it is only ever written
-	 * back to 0 by the forward handler or cleanup_session_requests()
-	 * (both under the mutex) before re-dispatching through a new
-	 * __do_request() call.  Consequently, r_attempts > 0 at this
-	 * point always means another __do_request() instance has already
-	 * passed the point of no return for this request, i.e. a racing
-	 * kick_requests() or __wake_requests() picked up the same
-	 * request from the xarray or a wait list and is about to send
-	 * it.  Bail out to prevent a double dispatch.
+	 * r_attempts only counts protocol send attempts now.  Dispatch
+	 * ownership is claimed separately via CEPH_MDS_R_DISPATCHING
+	 * under mdsc->mutex and held across the unlocked prepare/send
+	 * window: only one context may rebuild and send req->r_request
+	 * at a time.  A racing kick_requests() or __wake_requests()
+	 * that sees the claim set simply drops the request here.
 	 *
-	 * Note: kick_requests() also filters on r_attempts > 0 during
-	 * its collection pass, but that is a one-time snapshot taken
-	 * under the mutex.  Between that snapshot and the actual
-	 * __do_request() call the mutex is dropped and re-acquired, so
-	 * the protection is not atomic — this per-request gate closes
-	 * the remaining window.
+	 * cleanup_session_requests() and handle_forward() may zero
+	 * r_attempts while the owner is in flight; instead of racing,
+	 * they set CEPH_MDS_R_RESEND and the owner consumes it when
+	 * releasing ownership below.
 	 */
-	if (req->r_attempts > 0) {
+	if (test_and_set_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags)) {
 		mutex_unlock(&mdsc->mutex);
 		return;
 	}
+	/*
+	 * Claiming dispatch ownership supersedes any pending resend
+	 * request: the dispatch about to happen below is the
+	 * re-evaluation.  RESEND set later, while the send window is
+	 * open, is consumed by the release path at the bottom.
+	 */
+	clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
 
 	if (req->r_err || test_bit(CEPH_MDS_R_GOT_RESULT, &req->r_req_flags)) {
 		if (test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags))
 			__unregister_request(mdsc, req);
-		mutex_unlock(&mdsc->mutex);
-		return;
+		/*
+		 * The request is done (or dead) and must not be sent
+		 * again; the dispatch ownership claimed above is simply
+		 * released, consuming any pending resend request with it.
+		 */
+		clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
+		clear_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags);
+		goto no_dispatch;
+	}
+	if (test_bit(CEPH_MDS_R_GOT_UNSAFE, &req->r_req_flags)) {
+		/*
+		 * Unsafe requests are replayed only by
+		 * replay_unsafe_requests() during MDS reconnect, never
+		 * through the normal dispatch path.
+		 */
+		clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
+		clear_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags);
+		goto no_dispatch;
 	}
 
 	if (READ_ONCE(mdsc->fsc->mount_state) == CEPH_MOUNT_FENCE_IO) {
@@ -3721,10 +3754,10 @@ static void __do_request(struct ceph_mds_client *mdsc,
 			trace_ceph_mdsc_suspend_request(mdsc, session, req,
 							ceph_mdsc_suspend_reason_no_mdsmap);
 			spin_lock(&mdsc->wait_list_lock);
+			list_del_init(&req->r_wait);
 			list_add(&req->r_wait, &mdsc->waiting_for_map);
 			spin_unlock(&mdsc->wait_list_lock);
-			mutex_unlock(&mdsc->mutex);
-			return;
+			goto out_session;
 		}
 		if (!(mdsc->fsc->mount_options->flags &
 		      CEPH_MOUNT_OPT_MOUNTWAIT) &&
@@ -3747,10 +3780,10 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		trace_ceph_mdsc_suspend_request(mdsc, session, req,
 						ceph_mdsc_suspend_reason_no_active_mds);
 		spin_lock(&mdsc->wait_list_lock);
+		list_del_init(&req->r_wait);
 		list_add(&req->r_wait, &mdsc->waiting_for_map);
 		spin_unlock(&mdsc->wait_list_lock);
-		mutex_unlock(&mdsc->mutex);
-		return;
+		goto out_session;
 	}
 
 	/* get, open session */
@@ -3798,6 +3831,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 				trace_ceph_mdsc_suspend_request(mdsc, session, req,
 								ceph_mdsc_suspend_reason_rejected);
 				spin_lock(&mdsc->wait_list_lock);
+				list_del_init(&req->r_wait);
 				list_add(&req->r_wait, &mdsc->waiting_for_map);
 				spin_unlock(&mdsc->wait_list_lock);
 			} else {
@@ -3818,6 +3852,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		trace_ceph_mdsc_suspend_request(mdsc, session, req,
 						ceph_mdsc_suspend_reason_session);
 		spin_lock(&mdsc->wait_list_lock);
+		list_del_init(&req->r_wait);
 		list_add(&req->r_wait, &session->s_waiting);
 		spin_unlock(&mdsc->wait_list_lock);
 		goto out_session;
@@ -3905,6 +3940,20 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		complete_request(mdsc, req);
 		__unregister_request(mdsc, req);
 	}
+	/*
+	 * Release dispatch ownership.  If a session teardown or a
+	 * forward observed this request while the mutex was dropped
+	 * above, it set CEPH_MDS_R_RESEND under the mutex: consume it
+	 * here and re-evaluate, unless the request has already been
+	 * unregistered (error path above, or a racing reply).
+	 */
+	clear_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags);
+	if (test_and_clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags) &&
+	    xa_load(&mdsc->request_tree, req->r_tid) == req) {
+		mutex_unlock(&mdsc->mutex);
+		goto restart;
+	}
+no_dispatch:
 	mutex_unlock(&mdsc->mutex);
 	return;
 }
@@ -3913,22 +3962,68 @@ static void __wake_requests(struct ceph_mds_client *mdsc,
 			    struct list_head *head)
 {
 	struct ceph_client *cl = mdsc->fsc->client;
-	struct ceph_mds_request *req;
-	LIST_HEAD(tmp_list);
+	struct ceph_mds_request *req, *nreq;
+	LIST_HEAD(wake_list);
 
+	/*
+	 * Serialize the splice against the park decision in
+	 * __do_request(): both take mdsc->mutex, so a request cannot
+	 * be parked on @head after we have drained it, and everything
+	 * parked before we take the mutex is moved here.
+	 */
+	mutex_lock(&mdsc->mutex);
 	spin_lock(&mdsc->wait_list_lock);
-	list_splice_init(head, &tmp_list);
+	list_for_each_entry_safe(req, nreq, head, r_wait) {
+		/*
+		 * A non-empty r_aux_item means the request is already
+		 * owned by another dispatch collector (kick_requests()
+		 * or replay_unsafe_requests()); let it be dispatched by
+		 * that collector.  No stale waiter can result: a
+		 * kick-owned request was already delinked from r_wait
+		 * at claim time, and a replay-owned request is never on
+		 * a wait list (s_unsafe requests carry GOT_UNSAFE,
+		 * which __do_request() refuses to park, and replay's
+		 * old-request scan only takes r_attempts > 0 while
+		 * parked requests always have r_attempts == 0).
+		 */
+		if (!list_empty(&req->r_aux_item))
+			continue;
+		list_del_init(&req->r_wait);
+		/*
+		 * Pin the request in the same critical section that
+		 * delinks it from the wait list: once r_wait is
+		 * delinked, no list still owns it, so the reference
+		 * keeps the object alive while it is dispatched.
+		 */
+		ceph_mdsc_get_request(req);
+		list_add_tail(&req->r_aux_item, &wake_list);
+	}
 	spin_unlock(&mdsc->wait_list_lock);
+	mutex_unlock(&mdsc->mutex);
+
+	/*
+	 * Dispatch without holding the locks, but pop each node under
+	 * mdsc->mutex: r_aux_item doubles as the collector-ownership
+	 * predicate, so every access to it (collector list_empty()
+	 * checks and these pops) must be serialized by the mutex to
+	 * avoid a real data race on the node.
+	 */
+	mutex_lock(&mdsc->mutex);
+	while (!list_empty(&wake_list)) {
+		req = list_first_entry(&wake_list, struct ceph_mds_request,
+				       r_aux_item);
+		list_del_init(&req->r_aux_item);
+		mutex_unlock(&mdsc->mutex);
 
-	while (!list_empty(&tmp_list)) {
-		req = list_entry(tmp_list.next,
-				 struct ceph_mds_request, r_wait);
-		list_del_init(&req->r_wait);
 		doutc(cl, " wake request %p tid %llu\n", req,
 		      req->r_tid);
 		trace_ceph_mdsc_resume_request(mdsc, req);
 		__do_request(mdsc, req);
+		ceph_mdsc_put_request(req);
+
+		mutex_lock(&mdsc->mutex);
 	}
+	mutex_unlock(&mdsc->mutex);
 }
 
 /*
@@ -3938,7 +4033,7 @@ static void __wake_requests(struct ceph_mds_client *mdsc,
 static void kick_requests(struct ceph_mds_client *mdsc, int mds)
 {
 	struct ceph_client *cl = mdsc->fsc->client;
-	struct ceph_mds_request *req, *nreq;
+	struct ceph_mds_request *req;
 	unsigned long idx;
 	LIST_HEAD(kick_list);
 
@@ -3958,19 +4053,42 @@ static void kick_requests(struct ceph_mds_client *mdsc, int mds)
 			spin_lock(&mdsc->wait_list_lock);
 			list_del_init(&req->r_wait);
 			spin_unlock(&mdsc->wait_list_lock);
+			/*
+			 * A non-empty r_aux_item means the request is
+			 * already owned by another dispatch collector
+			 * (__wake_requests() or replay_unsafe_requests());
+			 * never queue the same node twice.
+			 */
+			if (!list_empty(&req->r_aux_item)) {
+				ceph_mdsc_put_request(req);
+				continue;
+			}
 			list_add_tail(&req->r_aux_item, &kick_list);
 		}
 	}
 	mutex_unlock(&mdsc->mutex);
 
 	/* replay without the mutex */
-	list_for_each_entry_safe(req, nreq, &kick_list, r_aux_item) {
+	/*
+	 * Same as __wake_requests(): r_aux_item is the collector-ownership
+	 * predicate, so pops are done under mdsc->mutex; the dispatch
+	 * itself runs outside the locks.
+	 */
+	mutex_lock(&mdsc->mutex);
+	while (!list_empty(&kick_list)) {
+		req = list_first_entry(&kick_list, struct ceph_mds_request,
+				       r_aux_item);
+		list_del_init(&req->r_aux_item);
+		mutex_unlock(&mdsc->mutex);
+
 		doutc(cl, " kicking tid %llu\n", req->r_tid);
 		trace_ceph_mdsc_resume_request(mdsc, req);
-		list_del_init(&req->r_aux_item);
 		__do_request(mdsc, req);
 		ceph_mdsc_put_request(req);
+
+		mutex_lock(&mdsc->mutex);
 	}
+	mutex_unlock(&mdsc->mutex);
 }
 
 int ceph_mdsc_submit_request(struct ceph_mds_client *mdsc, struct inode *dir,
@@ -4423,6 +4541,15 @@ static void handle_forward(struct ceph_mds_client *mdsc,
 		req->r_num_fwd = fwd_seq;
 		req->r_resend_mds = next_mds;
 		put_request_session(req);
+		/*
+		 * If a dispatch owner is in the send window, the
+		 * __do_request() below cannot claim the request and the
+		 * forward would be lost; set CEPH_MDS_R_RESEND so that
+		 * the owner re-evaluates the request when it releases
+		 * dispatch ownership.
+		 */
+		if (test_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags))
+			set_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
 	}
 	mutex_unlock(&mdsc->mutex);
 
@@ -4812,13 +4939,44 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 {
 	struct ceph_mds_request *req, *nreq;
 	unsigned long idx;
+	LIST_HEAD(unsafe_list);
+	LIST_HEAD(old_list);
 
 	doutc(mdsc->fsc->client, "mds%d\n", session->s_mds);
 
+	/*
+	 * Collect unsafe and old requests under mdsc->mutex, then
+	 * replay them without it: __send_request() is lockless and
+	 * ceph_mdsc_release_dir_caps_async() schedules work.
+	 */
 	mutex_lock(&mdsc->mutex);
-	list_for_each_entry_safe(req, nreq, &session->s_unsafe, r_unsafe_item)
+	list_for_each_entry_safe(req, nreq, &session->s_unsafe,
+				 r_unsafe_item) {
+		/*
+		 * A non-empty r_aux_item means the request is already
+		 * owned by another dispatch collector; never queue the
+		 * same node twice.  Replay sends __send_request()
+		 * directly, so it must also claim dispatch ownership
+		 * (CEPH_MDS_R_DISPATCHING) to exclude a concurrent
+		 * __do_request() from the send window.
+		 */
+		if (!list_empty(&req->r_aux_item))
+			continue;
+		if (test_and_set_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags))
+			continue;
+		/* the replay send below supersedes any pending resend */
+		clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
+		ceph_mdsc_get_request(req);
 		req->r_attempts++;
-		__send_request(session, req, true);
+		/*
+		 * Keep the request on s_unsafe: r_aux_item is only a
+		 * walk list.  The request must stay tracked as unsafe
+		 * until the MDS replies, so that a later reconnect can
+		 * replay it again and cleanup_session_requests() can
+		 * still abort it on session teardown.
+		 */
+		list_add_tail(&req->r_aux_item, &unsafe_list);
+	}
 
 	/*
 	 * also re-send old requests when MDS enters reconnect stage. So that MDS
@@ -4834,11 +4992,83 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 			continue;
 		if (req->r_session->s_mds != session->s_mds)
 			continue;
+		if (!list_empty(&req->r_aux_item))
+			continue;
+		if (test_and_set_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags))
+			continue;
+		/* the replay send below supersedes any pending resend */
+		clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
 
-		ceph_mdsc_release_dir_caps_async(req);
-
+		ceph_mdsc_get_request(req);
 		req->r_attempts++;
+		list_add_tail(&req->r_aux_item, &old_list);
+	}
+
+	mutex_unlock(&mdsc->mutex);
+
+	/*
+	 * Same as __wake_requests(): r_aux_item is the
+	 * collector-ownership predicate, so every access to it (the
+	 * list_empty() checks in the collectors above and these pops)
+	 * is serialized by mdsc->mutex.  The send itself runs outside
+	 * the locks.
+	 */
+
+	/* replay unsafe requests */
+	mutex_lock(&mdsc->mutex);
+	while (!list_empty(&unsafe_list)) {
+		bool resend;
+
+		req = list_first_entry(&unsafe_list, struct ceph_mds_request,
+				       r_aux_item);
+		list_del_init(&req->r_aux_item);
+		mutex_unlock(&mdsc->mutex);
+
 		__send_request(session, req, true);
+
+		mutex_lock(&mdsc->mutex);
+		clear_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags);
+		resend = test_and_clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
+		mutex_unlock(&mdsc->mutex);
+		/*
+		 * A teardown or forward may have requested a re-evaluation
+		 * while this replay send was in flight.  Re-dispatch
+		 * through the normal path, which claims dispatch ownership
+		 * itself: if the request has become unsafe meanwhile,
+		 * __do_request() sees GOT_UNSAFE and bails out, leaving
+		 * the request queued for the next replay; otherwise it is
+		 * sent again normally.
+		 */
+		if (resend)
+			__do_request(mdsc, req);
+		ceph_mdsc_put_request(req);
+
+		mutex_lock(&mdsc->mutex);
+	}
+	mutex_unlock(&mdsc->mutex);
+
+	/* replay old requests */
+	mutex_lock(&mdsc->mutex);
+	while (!list_empty(&old_list)) {
+		bool resend;
+
+		req = list_first_entry(&old_list, struct ceph_mds_request,
+				       r_aux_item);
+		list_del_init(&req->r_aux_item);
+		mutex_unlock(&mdsc->mutex);
+
+		ceph_mdsc_release_dir_caps_async(req);
+		__send_request(session, req, true);
+
+		mutex_lock(&mdsc->mutex);
+		clear_bit(CEPH_MDS_R_DISPATCHING, &req->r_req_flags);
+		resend = test_and_clear_bit(CEPH_MDS_R_RESEND, &req->r_req_flags);
+		mutex_unlock(&mdsc->mutex);
+		if (resend)
+			__do_request(mdsc, req);
+		ceph_mdsc_put_request(req);
+
+		mutex_lock(&mdsc->mutex);
 	}
 	mutex_unlock(&mdsc->mutex);
 }
@@ -5398,9 +5628,10 @@ static int send_mds_reconnect(struct ceph_mds_client *mdsc,
 
 	mutex_unlock(&session->s_mutex);
 
+	up_read(&mdsc->snap_rwsem);
+
 	__wake_requests(mdsc, &session->s_waiting);
 
-	up_read(&mdsc->snap_rwsem);
 	ceph_pagelist_release(recon_state.pagelist);
 	return 0;
 
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 11059678284d..4de4064ab727 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -359,6 +359,28 @@ struct ceph_mds_request {
 #define CEPH_MDS_R_PARENT_LOCKED	(7) /* is r_parent->i_rwsem wlocked? */
 #define CEPH_MDS_R_ASYNC		(8) /* async request */
 #define CEPH_MDS_R_FSCRYPT_FILE		(9) /* must marshal fscrypt_file field */
+/*
+ * A dispatch owner has claimed the right to rebuild and send
+ * req->r_request (__do_request() past the send gate, or
+ * replay_unsafe_requests()).  Set and cleared under mdsc->mutex;
+ * held across the unlocked send window, cleared by the owner when it
+ * re-acquires the mutex and releases ownership.
+ */
+#define CEPH_MDS_R_DISPATCHING		(10)
+/*
+ * Another context observed a session/request state change while a
+ * dispatch owner was in flight and wants a re-evaluation.  It is a
+ * request to re-evaluate, not a guarantee that a resend is still
+ * needed.
+ *
+ * If RESEND is set while DISPATCHING is held, the current dispatcher
+ * consumes it on release and redispatches.
+ * If RESEND is set after DISPATCHING has been cleared, the producer
+ * must itself trigger a redispatch: cleanup_session_requests() is
+ * followed by kick_requests(), while handle_forward() calls
+ * __do_request() directly.
+ */
+#define CEPH_MDS_R_RESEND		(11)
 	unsigned long	r_req_flags;
 
 	struct mutex r_fill_mutex;
@@ -428,7 +450,13 @@ struct ceph_mds_request {
 	struct completion r_safe_completion;
 	ceph_mds_request_callback_t r_callback;
 	struct list_head  r_unsafe_item;  /* per-session unsafe list item */
-	struct list_head  r_aux_item;     /* auxiliary local list item */
+	/*
+	 * Auxiliary walk list for the dispatch collectors; doubles as
+	 * the collector-ownership token.  INIT_LIST_HEAD() is done
+	 * before publication; afterwards, all accesses are serialized
+	 * by mdsc->mutex.
+	 */
+	struct list_head  r_aux_item;
 
 	long long	  r_dir_release_cnt;
 	long long	  r_dir_ordered_cnt;

-- 
2.53.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-29 11:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-29 11:35 [PATCH v6 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient Xiubo Li via B4 Relay
2026-08-29 11:35 ` [PATCH v6 1/5] ceph: use READ_ONCE/WRITE_ONCE for oldest_tid Xiubo Li via B4 Relay
2026-08-29 11:35 ` [PATCH v6 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid Xiubo Li via B4 Relay
2026-08-29 11:35 ` [PATCH v6 3/5] ceph: add wait_list_lock for wait-list serialization Xiubo Li via B4 Relay
2026-08-29 11:35 ` [PATCH v6 4/5] ceph: move mdsc->mutex into __do_request() Xiubo Li via B4 Relay
2026-08-29 11:35 ` [PATCH v6 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests Xiubo Li via B4 Relay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®