[PATCH 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient

[PATCH 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient

[Xiubo Li via B4 Relay]

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)

No functional changes intended.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
Xiubo Li (5):
      ceph: convert oldest_tid to atomic64_t
      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/debugfs.c    |   6 +-
 fs/ceph/mds_client.c | 207 +++++++++++++++++++++++++++------------------------
 fs/ceph/mds_client.h |   8 +-
 3 files changed, 117 insertions(+), 104 deletions(-)
---
base-commit: 7e1f9e2cd2d0e780c394a4402c40e125109fec72
change-id: 20260713-ceph-mdsc-mutex-optimization-7e74ab6bbc8b

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

[PATCH 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient

[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)

No functional changes intended.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
Xiubo Li (5):
      ceph: convert oldest_tid to atomic64_t
      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/debugfs.c    |   6 +-
 fs/ceph/mds_client.c | 207 +++++++++++++++++++++++++++------------------------
 fs/ceph/mds_client.h |   8 +-
 3 files changed, 117 insertions(+), 104 deletions(-)
---
base-commit: 7e1f9e2cd2d0e780c394a4402c40e125109fec72
change-id: 20260713-ceph-mdsc-mutex-optimization-7e74ab6bbc8b

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

[PATCH 1/5] ceph: convert oldest_tid to atomic64_t

[Xiubo Li via B4 Relay]

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
we already have, or skip one we still need (which will just be
retried).  With the plain u64 read, however, the compiler is free
to split or cache the load, which is undefined behaviour when the
write side runs under mdsc->mutex on a different CPU.

Convert mdsc->oldest_tid from u64 to atomic64_t so that reads
are guaranteed to be single-copy atomic on all architectures.
This removes the one remaining reason __prepare_send_request()
and __send_request() needed to be called under mdsc->mutex, so
drop those comments as well.

All write sites (__register_request, __unregister_request) still
run under mdsc->mutex, so use atomic64_set() for clarity.

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

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 9f84ef2ac6e4..98d0a5baff70 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1235,8 +1235,9 @@ 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 (atomic64_read(&mdsc->oldest_tid) == 0 &&
+	    req->r_op != CEPH_MDS_OP_SETFILELOCK)
+		atomic64_set(&mdsc->oldest_tid, req->r_tid);
 
 	if (dir) {
 		struct ceph_inode_info *ci = ceph_inode(dir);
@@ -1257,14 +1258,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 == atomic64_read(&mdsc->oldest_tid)) {
 		struct rb_node *p = rb_next(&req->r_node);
-		mdsc->oldest_tid = 0;
+		atomic64_set(&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;
+				atomic64_set(&mdsc->oldest_tid, next_req->r_tid);
 				break;
 			}
 			p = rb_next(p);
@@ -1693,7 +1694,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, atomic64_read(&mdsc->oldest_tid));
 
 	msg->front.iov_len = p - msg->front.iov_base;
 	msg->hdr.front_len = cpu_to_le32(msg->front.iov_len);
@@ -2759,7 +2760,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 atomic64_read(&mdsc->oldest_tid);
 }
 
 #if IS_ENABLED(CONFIG_FS_ENCRYPTION)
@@ -3438,9 +3439,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)
@@ -3555,9 +3553,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)
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 731d6ad04956..3b614b5df18c 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -532,7 +532,7 @@ struct ceph_mds_client {
 	spinlock_t              snap_empty_lock;  /* protect snap_empty */
 
 	u64                    last_tid;      /* most recent mds request */
-	u64                    oldest_tid;    /* oldest incomplete mds request,
+	atomic64_t             oldest_tid;    /* oldest incomplete mds request,
 						 excluding setfilelock requests */
 	struct rb_root         request_tree;  /* pending mds requests */
 	struct delayed_work    delayed_work;  /* delayed work */

-- 
2.53.0

[PATCH 1/5] ceph: convert oldest_tid to atomic64_t

[Xiubo Li]

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
we already have, or skip one we still need (which will just be
retried).  With the plain u64 read, however, the compiler is free
to split or cache the load, which is undefined behaviour when the
write side runs under mdsc->mutex on a different CPU.

Convert mdsc->oldest_tid from u64 to atomic64_t so that reads
are guaranteed to be single-copy atomic on all architectures.
This removes the one remaining reason __prepare_send_request()
and __send_request() needed to be called under mdsc->mutex, so
drop those comments as well.

All write sites (__register_request, __unregister_request) still
run under mdsc->mutex, so use atomic64_set() for clarity.

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

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 9f84ef2ac6e4..98d0a5baff70 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1235,8 +1235,9 @@ 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 (atomic64_read(&mdsc->oldest_tid) == 0 &&
+	    req->r_op != CEPH_MDS_OP_SETFILELOCK)
+		atomic64_set(&mdsc->oldest_tid, req->r_tid);
 
 	if (dir) {
 		struct ceph_inode_info *ci = ceph_inode(dir);
@@ -1257,14 +1258,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 == atomic64_read(&mdsc->oldest_tid)) {
 		struct rb_node *p = rb_next(&req->r_node);
-		mdsc->oldest_tid = 0;
+		atomic64_set(&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;
+				atomic64_set(&mdsc->oldest_tid, next_req->r_tid);
 				break;
 			}
 			p = rb_next(p);
@@ -1693,7 +1694,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, atomic64_read(&mdsc->oldest_tid));
 
 	msg->front.iov_len = p - msg->front.iov_base;
 	msg->hdr.front_len = cpu_to_le32(msg->front.iov_len);
@@ -2759,7 +2760,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 atomic64_read(&mdsc->oldest_tid);
 }
 
 #if IS_ENABLED(CONFIG_FS_ENCRYPTION)
@@ -3438,9 +3439,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)
@@ -3555,9 +3553,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)
diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
index 731d6ad04956..3b614b5df18c 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -532,7 +532,7 @@ struct ceph_mds_client {
 	spinlock_t              snap_empty_lock;  /* protect snap_empty */
 
 	u64                    last_tid;      /* most recent mds request */
-	u64                    oldest_tid;    /* oldest incomplete mds request,
+	atomic64_t             oldest_tid;    /* oldest incomplete mds request,
 						 excluding setfilelock requests */
 	struct rb_root         request_tree;  /* pending mds requests */
 	struct delayed_work    delayed_work;  /* delayed work */

-- 
2.53.0

[PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.

[Xiubo Li via B4 Relay]

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

The xarray gives O(1) lookups by tid (vs O(log N) rbtree) and
uses internal RCU-based locking, eliminating mdsc->mutex as a
precondition for tree access and paving the way for lockless
xa_load() in the future. Iteration is also cleaner: xa_for_each()
and xa_find() replace the open-coded rb_first()/rb_next() walks.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
 fs/ceph/debugfs.c    |   6 +--
 fs/ceph/mds_client.c | 109 ++++++++++++++++++++++-----------------------------
 fs/ceph/mds_client.h |   3 +-
 3 files changed, 50 insertions(+), 68 deletions(-)

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 98d0a5baff70..909f909b64ed 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1183,7 +1183,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.
@@ -1195,7 +1194,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);
 
@@ -1229,7 +1228,7 @@ 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);
+	xa_store(&mdsc->request_tree, req->r_tid, req, GFP_NOFS);
 
 	req->r_cred = get_current_cred();
 	if (!req->r_mnt_idmap)
@@ -1259,20 +1258,20 @@ static void __unregister_request(struct ceph_mds_client *mdsc,
 	list_del_init(&req->r_unsafe_item);
 
 	if (req->r_tid == atomic64_read(&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;
+
 		atomic64_set(&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) {
-				atomic64_set(&mdsc->oldest_tid, next_req->r_tid);
+				atomic64_set(&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);
@@ -1829,7 +1828,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);
@@ -1845,10 +1844,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;
@@ -2732,7 +2729,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);
@@ -2752,10 +2748,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)
@@ -3816,12 +3811,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)
@@ -4654,7 +4648,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);
 
@@ -4666,10 +4660,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)
@@ -5514,7 +5506,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);
@@ -5522,14 +5514,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);
@@ -5542,7 +5532,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);
 
@@ -6268,7 +6258,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);
@@ -6590,34 +6580,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);
 
@@ -6635,16 +6624,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);
@@ -6803,6 +6785,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 3b614b5df18c..00ca993ed583 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -329,7 +329,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;
@@ -534,7 +533,7 @@ struct ceph_mds_client {
 	u64                    last_tid;      /* most recent mds request */
 	atomic64_t             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

[PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.

[Xiubo Li]

The xarray gives O(1) lookups by tid (vs O(log N) rbtree) and
uses internal RCU-based locking, eliminating mdsc->mutex as a
precondition for tree access and paving the way for lockless
xa_load() in the future. Iteration is also cleaner: xa_for_each()
and xa_find() replace the open-coded rb_first()/rb_next() walks.

Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
---
 fs/ceph/debugfs.c    |   6 +--
 fs/ceph/mds_client.c | 109 ++++++++++++++++++++++-----------------------------
 fs/ceph/mds_client.h |   3 +-
 3 files changed, 50 insertions(+), 68 deletions(-)

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 98d0a5baff70..909f909b64ed 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -1183,7 +1183,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.
@@ -1195,7 +1194,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);
 
@@ -1229,7 +1228,7 @@ 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);
+	xa_store(&mdsc->request_tree, req->r_tid, req, GFP_NOFS);
 
 	req->r_cred = get_current_cred();
 	if (!req->r_mnt_idmap)
@@ -1259,20 +1258,20 @@ static void __unregister_request(struct ceph_mds_client *mdsc,
 	list_del_init(&req->r_unsafe_item);
 
 	if (req->r_tid == atomic64_read(&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;
+
 		atomic64_set(&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) {
-				atomic64_set(&mdsc->oldest_tid, next_req->r_tid);
+				atomic64_set(&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);
@@ -1829,7 +1828,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);
@@ -1845,10 +1844,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;
@@ -2732,7 +2729,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);
@@ -2752,10 +2748,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)
@@ -3816,12 +3811,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)
@@ -4654,7 +4648,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);
 
@@ -4666,10 +4660,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)
@@ -5514,7 +5506,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);
@@ -5522,14 +5514,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);
@@ -5542,7 +5532,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);
 
@@ -6268,7 +6258,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);
@@ -6590,34 +6580,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);
 
@@ -6635,16 +6624,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);
@@ -6803,6 +6785,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 3b614b5df18c..00ca993ed583 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -329,7 +329,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;
@@ -534,7 +533,7 @@ struct ceph_mds_client {
 	u64                    last_tid;      /* most recent mds request */
 	atomic64_t             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

[PATCH 3/5] ceph: add wait_list_lock for wait-list serialization

[Xiubo Li via B4 Relay]

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

Add a dedicated spinlock to protect the waiting_for_map and
s_waiting lists, which are currently serialized by mdsc->mutex.
This is a preparatory step for removing mdsc->mutex from the
kick_requests() path: xa_for_each() is internally locked and the
per-request filter checks are lockless, so only list_del_init()
needs external protection.

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 909f909b64ed..6f9049148721 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3609,7 +3609,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 &
@@ -3632,7 +3634,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;
 	}
 
@@ -3680,9 +3684,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;
 		}
 
@@ -3697,7 +3704,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;
 	}
 
@@ -3790,7 +3799,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,
@@ -3823,7 +3834,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);
 		}
@@ -6258,6 +6271,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;
@@ -6340,7 +6354,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 00ca993ed583..cd5dacfb42bd 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -529,6 +529,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 */
 	atomic64_t             oldest_tid;    /* oldest incomplete mds request,

-- 
2.53.0

[PATCH 3/5] ceph: add wait_list_lock for wait-list serialization

[Xiubo Li]

Add a dedicated spinlock to protect the waiting_for_map and
s_waiting lists, which are currently serialized by mdsc->mutex.
This is a preparatory step for removing mdsc->mutex from the
kick_requests() path: xa_for_each() is internally locked and the
per-request filter checks are lockless, so only list_del_init()
needs external protection.

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 909f909b64ed..6f9049148721 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3609,7 +3609,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 &
@@ -3632,7 +3634,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;
 	}
 
@@ -3680,9 +3684,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;
 		}
 
@@ -3697,7 +3704,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;
 	}
 
@@ -3790,7 +3799,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,
@@ -3823,7 +3834,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);
 		}
@@ -6258,6 +6271,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;
@@ -6340,7 +6354,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 00ca993ed583..cd5dacfb42bd 100644
--- a/fs/ceph/mds_client.h
+++ b/fs/ceph/mds_client.h
@@ -529,6 +529,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 */
 	atomic64_t             oldest_tid;    /* oldest incomplete mds request,

-- 
2.53.0

[PATCH 4/5] ceph: move mdsc->mutex into __do_request()

[Xiubo Li via B4 Relay]

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

__do_request() now acquires mdsc->mutex on entry and unlocks at
every exit point, so callers no longer need to hold it.  As a
result kick_requests() is now completely free of mdsc->mutex:
xa_for_each() is internally locked, list_del_init() is guarded by
wait_list_lock, and __do_request() manages its own serialization.

Remove the no-longer-needed mutex_lock/unlock around __do_request()
in ceph_mdsc_submit_request() and handle_forward().  The do_request()
wrapper introduced in the previous commit is no longer necessary
and is dropped.

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

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 6f9049148721..ef54d19ace2a 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3577,9 +3577,12 @@ static void __do_request(struct ceph_mds_client *mdsc,
 	int err = 0;
 	bool random;
 
+	mutex_lock(&mdsc->mutex);
+
 	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;
 	}
 
@@ -3612,6 +3615,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 &
@@ -3637,6 +3641,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;
 	}
 
@@ -3710,6 +3715,8 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		goto out_session;
 	}
 
+	mutex_unlock(&mdsc->mutex);
+
 	/* send request */
 	req->r_resend_mds = -1;   /* forget any previous mds hint */
 
@@ -3740,6 +3747,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);
@@ -3777,6 +3785,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:
@@ -3786,6 +3796,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		complete_request(mdsc, req);
 		__unregister_request(mdsc, req);
 	}
+	mutex_unlock(&mdsc->mutex);
 	return;
 }
 
@@ -3900,10 +3911,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;
 }
 
@@ -4276,13 +4288,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;
 
@@ -4606,11 +4619,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);
@@ -5245,9 +5256,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);
@@ -5672,8 +5681,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]);
@@ -5684,9 +5693,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);
@@ -5802,8 +5809,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);
@@ -5812,8 +5819,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;
 		}
 
@@ -5857,8 +5864,8 @@ 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);
 			mutex_unlock(&mdsc->mutex);
+			kick_requests(mdsc, i);
 			mutex_lock(&s->s_mutex);
 			mutex_lock(&mdsc->mutex);
 			ceph_kick_flushing_caps(mdsc, s);
@@ -6766,8 +6773,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);
@@ -6778,11 +6785,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)
@@ -6924,8 +6931,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);
 }
 
 /*
@@ -6976,11 +6983,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;
 
@@ -7078,8 +7085,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);
@@ -7088,9 +7095,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;

-- 
2.53.0

[PATCH 4/5] ceph: move mdsc->mutex into __do_request()

[Xiubo Li]

__do_request() now acquires mdsc->mutex on entry and unlocks at
every exit point, so callers no longer need to hold it.  As a
result kick_requests() is now completely free of mdsc->mutex:
xa_for_each() is internally locked, list_del_init() is guarded by
wait_list_lock, and __do_request() manages its own serialization.

Remove the no-longer-needed mutex_lock/unlock around __do_request()
in ceph_mdsc_submit_request() and handle_forward().  The do_request()
wrapper introduced in the previous commit is no longer necessary
and is dropped.

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

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 6f9049148721..ef54d19ace2a 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3577,9 +3577,12 @@ static void __do_request(struct ceph_mds_client *mdsc,
 	int err = 0;
 	bool random;
 
+	mutex_lock(&mdsc->mutex);
+
 	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;
 	}
 
@@ -3612,6 +3615,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 &
@@ -3637,6 +3641,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;
 	}
 
@@ -3710,6 +3715,8 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		goto out_session;
 	}
 
+	mutex_unlock(&mdsc->mutex);
+
 	/* send request */
 	req->r_resend_mds = -1;   /* forget any previous mds hint */
 
@@ -3740,6 +3747,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);
@@ -3777,6 +3785,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:
@@ -3786,6 +3796,7 @@ static void __do_request(struct ceph_mds_client *mdsc,
 		complete_request(mdsc, req);
 		__unregister_request(mdsc, req);
 	}
+	mutex_unlock(&mdsc->mutex);
 	return;
 }
 
@@ -3900,10 +3911,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;
 }
 
@@ -4276,13 +4288,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;
 
@@ -4606,11 +4619,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);
@@ -5245,9 +5256,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);
@@ -5672,8 +5681,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]);
@@ -5684,9 +5693,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);
@@ -5802,8 +5809,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);
@@ -5812,8 +5819,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;
 		}
 
@@ -5857,8 +5864,8 @@ 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);
 			mutex_unlock(&mdsc->mutex);
+			kick_requests(mdsc, i);
 			mutex_lock(&s->s_mutex);
 			mutex_lock(&mdsc->mutex);
 			ceph_kick_flushing_caps(mdsc, s);
@@ -6766,8 +6773,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);
@@ -6778,11 +6785,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)
@@ -6924,8 +6931,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);
 }
 
 /*
@@ -6976,11 +6983,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;
 
@@ -7078,8 +7085,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);
@@ -7088,9 +7095,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;

-- 
2.53.0

[PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests

[Xiubo Li via B4 Relay]

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

__send_request() is lockless so holding mdsc->mutex across the
replay loop only serializes the list iteration itself.  Collect
the unsafe list entries under the mutex, then replay them without
it.  The old-request tree walk uses xa_for_each() which is
internally locked.

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

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index ef54d19ace2a..6c5a8c651d3e 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4673,12 +4673,25 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 {
 	struct ceph_mds_request *req, *nreq;
 	unsigned long idx;
+	LIST_HEAD(replay_list);
 
 	doutc(mdsc->fsc->client, "mds%d\n", session->s_mds);
 
+	/* collect unsafe requests under the mutex */
 	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) {
+		ceph_mdsc_get_request(req);
+		list_move(&req->r_unsafe_item, &replay_list);
+	}
+	mutex_unlock(&mdsc->mutex);
+
+	/* replay unsafe requests — __send_request is lockless */
+	list_for_each_entry_safe(req, nreq, &replay_list, r_unsafe_item) {
 		__send_request(session, req, true);
+		list_del_init(&req->r_unsafe_item);
+		ceph_mdsc_put_request(req);
+	}
 
 	/*
 	 * also re-send old requests when MDS enters reconnect stage. So that MDS
@@ -4699,7 +4712,6 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 
 		__send_request(session, req, true);
 	}
-	mutex_unlock(&mdsc->mutex);
 }
 
 static int send_reconnect_partial(struct ceph_reconnect_state *recon_state)

-- 
2.53.0

[PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests

[Xiubo Li]

__send_request() is lockless so holding mdsc->mutex across the
replay loop only serializes the list iteration itself.  Collect
the unsafe list entries under the mutex, then replay them without
it.  The old-request tree walk uses xa_for_each() which is
internally locked.

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

diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index ef54d19ace2a..6c5a8c651d3e 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -4673,12 +4673,25 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 {
 	struct ceph_mds_request *req, *nreq;
 	unsigned long idx;
+	LIST_HEAD(replay_list);
 
 	doutc(mdsc->fsc->client, "mds%d\n", session->s_mds);
 
+	/* collect unsafe requests under the mutex */
 	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) {
+		ceph_mdsc_get_request(req);
+		list_move(&req->r_unsafe_item, &replay_list);
+	}
+	mutex_unlock(&mdsc->mutex);
+
+	/* replay unsafe requests — __send_request is lockless */
+	list_for_each_entry_safe(req, nreq, &replay_list, r_unsafe_item) {
 		__send_request(session, req, true);
+		list_del_init(&req->r_unsafe_item);
+		ceph_mdsc_put_request(req);
+	}
 
 	/*
 	 * also re-send old requests when MDS enters reconnect stage. So that MDS
@@ -4699,7 +4712,6 @@ static void replay_unsafe_requests(struct ceph_mds_client *mdsc,
 
 		__send_request(session, req, true);
 	}
-	mutex_unlock(&mdsc->mutex);
 }
 
 static int send_reconnect_partial(struct ceph_reconnect_state *recon_state)

-- 
2.53.0

Re: [PATCH 1/5] ceph: convert oldest_tid to atomic64_t

[Viacheslav Dubeyko]

On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> 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
> we already have, or skip one we still need (which will just be
> retried).  With the plain u64 read, however, the compiler is free
> to split or cache the load, which is undefined behaviour when the
> write side runs under mdsc->mutex on a different CPU.
> 
> Convert mdsc->oldest_tid from u64 to atomic64_t so that reads
> are guaranteed to be single-copy atomic on all architectures.
> This removes the one remaining reason __prepare_send_request()
> and __send_request() needed to be called under mdsc->mutex, so
> drop those comments as well.
> 
> All write sites (__register_request, __unregister_request) still
> run under mdsc->mutex, so use atomic64_set() for clarity.
> 
> Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
> ---
>  fs/ceph/mds_client.c | 21 ++++++++-------------
>  fs/ceph/mds_client.h |  2 +-
>  2 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 9f84ef2ac6e4..98d0a5baff70 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1235,8 +1235,9 @@ 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 (atomic64_read(&mdsc->oldest_tid) == 0 &&
> +	    req->r_op != CEPH_MDS_OP_SETFILELOCK)
> +		atomic64_set(&mdsc->oldest_tid, req->r_tid);
>  
>  	if (dir) {
>  		struct ceph_inode_info *ci = ceph_inode(dir);
> @@ -1257,14 +1258,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 == atomic64_read(&mdsc->oldest_tid)) {
>  		struct rb_node *p = rb_next(&req->r_node);
> -		mdsc->oldest_tid = 0;
> +		atomic64_set(&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;
> +				atomic64_set(&mdsc->oldest_tid,
> next_req->r_tid);
>  				break;
>  			}
>  			p = rb_next(p);
> @@ -1693,7 +1694,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, atomic64_read(&mdsc->oldest_tid));
>  
>  	msg->front.iov_len = p - msg->front.iov_base;
>  	msg->hdr.front_len = cpu_to_le32(msg->front.iov_len);
> @@ -2759,7 +2760,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 atomic64_read(&mdsc->oldest_tid);
>  }
>  
>  #if IS_ENABLED(CONFIG_FS_ENCRYPTION)
> @@ -3438,9 +3439,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)
> @@ -3555,9 +3553,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)
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index 731d6ad04956..3b614b5df18c 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -532,7 +532,7 @@ struct ceph_mds_client {
>  	spinlock_t              snap_empty_lock;  /* protect
> snap_empty */
>  
>  	u64                    last_tid;      /* most recent mds
> request */
> -	u64                    oldest_tid;    /* oldest incomplete
> mds request,
> +	atomic64_t             oldest_tid;    /* oldest incomplete
> mds request,
>  						 excluding
> setfilelock requests */
>  	struct rb_root         request_tree;  /* pending mds
> requests */
>  	struct delayed_work    delayed_work;  /* delayed work */

Looks good.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.

Re: [PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.

[Viacheslav Dubeyko]

On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> From: Xiubo Li <xiubo.li@clyso.com>
> 
> The xarray gives O(1) lookups by tid (vs O(log N) rbtree) and
> uses internal RCU-based locking, eliminating mdsc->mutex as a
> precondition for tree access and paving the way for lockless
> xa_load() in the future. Iteration is also cleaner: xa_for_each()
> and xa_find() replace the open-coded rb_first()/rb_next() walks.
> 
> Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
> ---
>  fs/ceph/debugfs.c    |   6 +--
>  fs/ceph/mds_client.c | 109 ++++++++++++++++++++++-------------------
> ----------
>  fs/ceph/mds_client.h |   3 +-
>  3 files changed, 50 insertions(+), 68 deletions(-)
> 
> 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 98d0a5baff70..909f909b64ed 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1183,7 +1183,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.
> @@ -1195,7 +1194,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);
>  
> @@ -1229,7 +1228,7 @@ 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);
> +	xa_store(&mdsc->request_tree, req->r_tid, req, GFP_NOFS);

Should we check the return value of xa_store()? What about this?

        err = xa_err(xa_store(&mdsc->request_tree, req->r_tid, req,
GFP_NOFS));
        if (err) {
                ceph_mdsc_put_request(req);
                return err;
        } 

>  
>  	req->r_cred = get_current_cred();
>  	if (!req->r_mnt_idmap)
> @@ -1259,20 +1258,20 @@ static void __unregister_request(struct
> ceph_mds_client *mdsc,
>  	list_del_init(&req->r_unsafe_item);
>  
>  	if (req->r_tid == atomic64_read(&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;
> +
>  		atomic64_set(&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) {
> -				atomic64_set(&mdsc->oldest_tid,
> next_req->r_tid);
> +				atomic64_set(&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);
> @@ -1829,7 +1828,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);
> @@ -1845,10 +1844,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;
> @@ -2732,7 +2729,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);
> @@ -2752,10 +2748,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)
> @@ -3816,12 +3811,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)
> @@ -4654,7 +4648,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);
>  
> @@ -4666,10 +4660,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)
> @@ -5514,7 +5506,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);
> @@ -5522,14 +5514,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);
> @@ -5542,7 +5532,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);
>  
> @@ -6268,7 +6258,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);
> @@ -6590,34 +6580,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);
>  
> @@ -6635,16 +6624,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);
> @@ -6803,6 +6785,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 3b614b5df18c..00ca993ed583 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -329,7 +329,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;
> @@ -534,7 +533,7 @@ struct ceph_mds_client {
>  	u64                    last_tid;      /* most recent mds
> request */
>  	atomic64_t             oldest_tid;    /* oldest incomplete
> mds request,
>  						 excluding
> setfilelock requests */
> -	struct rb_root         request_tree;  /* pending mds
> requests */
> +	struct xarray           request_tree;  /* pending mds
> requests */

Transaction IDs are u64; xarray indices are unsigned long. req->r_tid,
mdsc->last_tid, and want_tid/last_tid in the drain paths are all u64,
but xa_store(), xa_load(), xa_erase(), and xa_find()'s max parameter
all take unsigned long. CEPH_FS in fs/ceph/Kconfig only depends on INET
— it's not gated on 64BIT — so this driver still builds for 32-bit
targets, where unsigned long truncates r_tid to 32 bits. Are we safe
here?

Thanks,
Slava.

>  	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 */

Re: [PATCH 3/5] ceph: add wait_list_lock for wait-list serialization

[Viacheslav Dubeyko]

On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> From: Xiubo Li <xiubo.li@clyso.com>
> 
> Add a dedicated spinlock to protect the waiting_for_map and
> s_waiting lists, which are currently serialized by mdsc->mutex.
> This is a preparatory step for removing mdsc->mutex from the
> kick_requests() path: xa_for_each() is internally locked and the
> per-request filter checks are lockless, so only list_del_init()
> needs external protection.
> 
> 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 909f909b64ed..6f9049148721 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3609,7 +3609,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_su
> spend_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 &
> @@ -3632,7 +3634,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_re
> ason_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;
>  	}
>  
> @@ -3680,9 +3684,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;
>  		}
>  
> @@ -3697,7 +3704,9 @@ static void __do_request(struct ceph_mds_client
> *mdsc,
>  		}
>  		trace_ceph_mdsc_suspend_request(mdsc, session, req,
>  						ceph_mdsc_suspend_re
> ason_session);
> +		spin_lock(&mdsc->wait_list_lock);
>  		list_add(&req->r_wait, &session->s_waiting);
> +		spin_unlock(&mdsc->wait_list_lock);
>  		goto out_session;
>  	}
>  
> @@ -3790,7 +3799,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,
> @@ -3823,7 +3834,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);
>  		}
> @@ -6258,6 +6271,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;
> @@ -6340,7 +6354,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 00ca993ed583..cd5dacfb42bd 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -529,6 +529,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 */
>  	atomic64_t             oldest_tid;    /* oldest incomplete
> mds request,

I don't see any issues in the patch. Looks good.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.

Re: [PATCH 4/5] ceph: move mdsc->mutex into __do_request()

[Viacheslav Dubeyko]

On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> From: Xiubo Li <xiubo.li@clyso.com>
> 
> __do_request() now acquires mdsc->mutex on entry and unlocks at
> every exit point, so callers no longer need to hold it.  As a
> result kick_requests() is now completely free of mdsc->mutex:
> xa_for_each() is internally locked, list_del_init() is guarded by
> wait_list_lock, and __do_request() manages its own serialization.
> 
> Remove the no-longer-needed mutex_lock/unlock around __do_request()
> in ceph_mdsc_submit_request() and handle_forward().  The do_request()
> wrapper introduced in the previous commit is no longer necessary
> and is dropped.
> 
> Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
> ---
>  fs/ceph/mds_client.c | 45 +++++++++++++++++++++++++-----------------
> ---
>  1 file changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 6f9049148721..ef54d19ace2a 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3577,9 +3577,12 @@ static void __do_request(struct
> ceph_mds_client *mdsc,
>  	int err = 0;
>  	bool random;
>  
> +	mutex_lock(&mdsc->mutex);
> +
>  	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;
>  	}
>  
> @@ -3612,6 +3615,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 &
> @@ -3637,6 +3641,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;
>  	}
>  
> @@ -3710,6 +3715,8 @@ static void __do_request(struct ceph_mds_client
> *mdsc,
>  		goto out_session;
>  	}
>  
> +	mutex_unlock(&mdsc->mutex);
> +
>  	/* send request */
>  	req->r_resend_mds = -1;   /* forget any previous mds hint */
>  
> @@ -3740,6 +3747,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);
> @@ -3777,6 +3785,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:
> @@ -3786,6 +3796,7 @@ static void __do_request(struct ceph_mds_client
> *mdsc,
>  		complete_request(mdsc, req);
>  		__unregister_request(mdsc, req);
>  	}
> +	mutex_unlock(&mdsc->mutex);
>  	return;
>  }
>  
> @@ -3900,10 +3911,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;
>  }
>  
> @@ -4276,13 +4288,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;
>  
> @@ -4606,11 +4619,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);
> @@ -5245,9 +5256,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);
> @@ -5672,8 +5681,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]);
> @@ -5684,9 +5693,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);
> @@ -5802,8 +5809,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);
> @@ -5812,8 +5819,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;
>  		}
>  
> @@ -5857,8 +5864,8 @@ 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);
>  			mutex_unlock(&mdsc->mutex);
> +			kick_requests(mdsc, i);
>  			mutex_lock(&s->s_mutex);
>  			mutex_lock(&mdsc->mutex);
>  			ceph_kick_flushing_caps(mdsc, s);
> @@ -6766,8 +6773,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);
> @@ -6778,11 +6785,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)
> @@ -6924,8 +6931,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);
>  }
>  
>  /*
> @@ -6976,11 +6983,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;
>  
> @@ -7078,8 +7085,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);
> @@ -7088,9 +7095,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;


If I am not wrong, we still have comment [1]:

/*
 * called under mdsc->mutex
 */
static void __wake_requests(struct ceph_mds_client *mdsc,
			    struct list_head *head)

Thanks,
Slava.

[1]
https://elixir.bootlin.com/linux/v7.2-rc3/source/fs/ceph/mds_client.c#L3793

Re: [PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests

[Viacheslav Dubeyko]

On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> From: Xiubo Li <xiubo.li@clyso.com>
> 
> __send_request() is lockless so holding mdsc->mutex across the
> replay loop only serializes the list iteration itself.  Collect
> the unsafe list entries under the mutex, then replay them without
> it.  The old-request tree walk uses xa_for_each() which is
> internally locked.
> 
> Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
> ---
>  fs/ceph/mds_client.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index ef54d19ace2a..6c5a8c651d3e 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -4673,12 +4673,25 @@ static void replay_unsafe_requests(struct
> ceph_mds_client *mdsc,
>  {
>  	struct ceph_mds_request *req, *nreq;
>  	unsigned long idx;
> +	LIST_HEAD(replay_list);
>  
>  	doutc(mdsc->fsc->client, "mds%d\n", session->s_mds);
>  
> +	/* collect unsafe requests under the mutex */
>  	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) {
> +		ceph_mdsc_get_request(req);
> +		list_move(&req->r_unsafe_item, &replay_list);
> +	}
> +	mutex_unlock(&mdsc->mutex);

This loop is done right.

> +
> +	/* replay unsafe requests — __send_request is lockless */
> +	list_for_each_entry_safe(req, nreq, &replay_list,
> r_unsafe_item) {
>  		__send_request(session, req, true);
> +		list_del_init(&req->r_unsafe_item);
> +		ceph_mdsc_put_request(req);
> +	}

However, this one has issue, if I am right. No ceph_mdsc_get_request()
anywhere in it. It has always relied entirely on the caller holding
mdsc->mutex for the duration of the loop.

While this loop is inside __send_request() or
ceph_mdsc_release_dir_caps_async() for one req, nothing stops another
thread from acquiring mdsc->mutex and completing/timing-out/aborting
that same request through the now-fully-unlocked
__do_request()/wait_requests() path, dropping the last reference and
freeing it — at which point the next loop iteration's xa_find() touches
freed memory. Am I right here?

Thanks,
Slava.

>  
>  	/*
>  	 * also re-send old requests when MDS enters reconnect
> stage. So that MDS
> @@ -4699,7 +4712,6 @@ static void replay_unsafe_requests(struct
> ceph_mds_client *mdsc,
>  
>  		__send_request(session, req, true);
>  	}
> -	mutex_unlock(&mdsc->mutex);
>  }
>  
>  static int send_reconnect_partial(struct ceph_reconnect_state
> *recon_state)

Re: [PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.

[Xiubo Li]

Hi Slava,

On Wed, 15 Jul 2026 at 02:35, Viacheslav Dubeyko <slava@dubeyko.com> wrote:
>
> On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> > From: Xiubo Li <xiubo.li@clyso.com>
> >
[......]
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -1183,7 +1183,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.
> > @@ -1195,7 +1194,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);
> >
> > @@ -1229,7 +1228,7 @@ 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);
> > +     xa_store(&mdsc->request_tree, req->r_tid, req, GFP_NOFS);
>
> Should we check the return value of xa_store()? What about this?
>
>         err = xa_err(xa_store(&mdsc->request_tree, req->r_tid, req,
> GFP_NOFS));
>         if (err) {
>                 ceph_mdsc_put_request(req);
>                 return err;
>         }
>

Good Catch. Let me fix it.

> >
> >       req->r_cred = get_current_cred();
> >       if (!req->r_mnt_idmap)
> > @@ -1259,20 +1258,20 @@ static void __unregister_request(struct
> > ceph_mds_client *mdsc,
> >       list_del_init(&req->r_unsafe_item);
> >

[......]

> > @@ -534,7 +533,7 @@ struct ceph_mds_client {
> >       u64                    last_tid;      /* most recent mds
> > request */
> >       atomic64_t             oldest_tid;    /* oldest incomplete
> > mds request,
> >                                                excluding
> > setfilelock requests */
> > -     struct rb_root         request_tree;  /* pending mds
> > requests */
> > +     struct xarray           request_tree;  /* pending mds
> > requests */
>
> Transaction IDs are u64; xarray indices are unsigned long. req->r_tid,
> mdsc->last_tid, and want_tid/last_tid in the drain paths are all u64,
> but xa_store(), xa_load(), xa_erase(), and xa_find()'s max parameter
> all take unsigned long. CEPH_FS in fs/ceph/Kconfig only depends on INET
> — it's not gated on 64BIT — so this driver still builds for 32-bit
> targets, where unsigned long truncates r_tid to 32 bits. Are we safe
> here?
>

Yeah, you are right. We have two options to fix it. Just add
"depends on 64BIT" in Kconfig or fall back to the rbtree code on
32-bit with an #if BITS_PER_LONG == 64 guard, similar to what we
already do for s_delegated_inos. I went with the #if guard in v2
because it keeps the driver buildable on 32-bit without introducing
a regression — the rbtree handles u64 keys correctly there. The
downside is a few extra #if/#else/#endif blocks, but they are
mostly confined to the iteration headers; the loop bodies are
shared. Only flush_mdlog_and_wait_mdsc_unsafe_requests() needs a
full alternate implementation because the two control flows are
fundamentally different.

What do you prefer?

Thanks
Xiubo Li

Re: [PATCH 4/5] ceph: move mdsc->mutex into __do_request()

[Xiubo Li]

On Wed, 15 Jul 2026 at 02:52, Viacheslav Dubeyko <slava@dubeyko.com> wrote:
>
> On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> > From: Xiubo Li <xiubo.li@clyso.com>
> >
> > __do_request() now acquires mdsc->mutex on entry and unlocks at
> > every exit point, so callers no longer need to hold it.  As a
> > result kick_requests() is now completely free of mdsc->mutex:
> > xa_for_each() is internally locked, list_del_init() is guarded by
> > wait_list_lock, and __do_request() manages its own serialization.
> >
> > Remove the no-longer-needed mutex_lock/unlock around __do_request()
> > in ceph_mdsc_submit_request() and handle_forward().  The do_request()

[......]

> > @@ -7088,9 +7095,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;
>
>
> If I am not wrong, we still have comment [1]:
>
> /*
>  * called under mdsc->mutex
>  */
> static void __wake_requests(struct ceph_mds_client *mdsc,
>                             struct list_head *head)
>

Good catch. I will remove that comment. Thanks Slava.

Xiubo Li

> Thanks,
> Slava.
>
> [1]
> https://elixir.bootlin.com/linux/v7.2-rc3/source/fs/ceph/mds_client.c#L3793

Re: [PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests

[Xiubo Li]

On Wed, 15 Jul 2026 at 03:07, Viacheslav Dubeyko <slava@dubeyko.com> wrote:
>
> On Mon, 2026-07-13 at 17:46 +0800, Xiubo Li via B4 Relay wrote:
> > From: Xiubo Li <xiubo.li@clyso.com>
[......]
> > @@ -4673,12 +4673,25 @@ static void replay_unsafe_requests(struct
> > ceph_mds_client *mdsc,
> >  {
> >       struct ceph_mds_request *req, *nreq;
> >       unsigned long idx;
> > +     LIST_HEAD(replay_list);
> >
> >       doutc(mdsc->fsc->client, "mds%d\n", session->s_mds);
> >
> > +     /* collect unsafe requests under the mutex */
> >       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) {
> > +             ceph_mdsc_get_request(req);
> > +             list_move(&req->r_unsafe_item, &replay_list);
> > +     }
> > +     mutex_unlock(&mdsc->mutex);
>
> This loop is done right.
>
> > +
> > +     /* replay unsafe requests — __send_request is lockless */
> > +     list_for_each_entry_safe(req, nreq, &replay_list,
> > r_unsafe_item) {
> >               __send_request(session, req, true);
> > +             list_del_init(&req->r_unsafe_item);
> > +             ceph_mdsc_put_request(req);
> > +     }
>
> However, this one has issue, if I am right. No ceph_mdsc_get_request()
> anywhere in it. It has always relied entirely on the caller holding
> mdsc->mutex for the duration of the loop.
>
> While this loop is inside __send_request() or
> ceph_mdsc_release_dir_caps_async() for one req, nothing stops another
> thread from acquiring mdsc->mutex and completing/timing-out/aborting
> that same request through the now-fully-unlocked
> __do_request()/wait_requests() path, dropping the last reference and
> freeing it — at which point the next loop iteration's xa_find() touches
> freed memory. Am I right here?
>

Good catch. The old-request loop in phase 3 iterates raw pointers from
xa_for_each() / rb_next() without holding a reference. If another
thread completes the request while __send_request() runs without
mdsc->mutex, the request could be freed, leading to UAF.

I will fix it.

Thanks
Xiubo LI

Re: [PATCH 1/5] ceph: convert oldest_tid to atomic64_t

[David Laight]

On Mon, 13 Jul 2026 17:46:07 +0800
Xiubo Li via B4 Relay <devnull+xiubo.li.clyso.com@kernel.org> wrote:

> 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
> we already have, or skip one we still need (which will just be
> retried).  With the plain u64 read, however, the compiler is free
> to split or cache the load, which is undefined behaviour when the
> write side runs under mdsc->mutex on a different CPU.
> 
> Convert mdsc->oldest_tid from u64 to atomic64_t so that reads
> are guaranteed to be single-copy atomic on all architectures.
> This removes the one remaining reason __prepare_send_request()
> and __send_request() needed to be called under mdsc->mutex, so
> drop those comments as well.
> 
> All write sites (__register_request, __unregister_request) still
> run under mdsc->mutex, so use atomic64_set() for clarity.

If the read side doesn't hold the lock then it makes little difference
whether the write side holds it.
It the write is split (eg on 32bit) then even an atomic read could
get an invalid value.

On 64bit this change probably has no effect.
But some 32bit architectures cannot do atomic 64bit read/write so
they are expensive operations.
x86-32 now requires the 64bit 'compare exchange' that can be used
for 64bit load/store so they are only moderately expensive not
stupidly expensive.

	David

> 
> Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
> ---
>  fs/ceph/mds_client.c | 21 ++++++++-------------
>  fs/ceph/mds_client.h |  2 +-
>  2 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 9f84ef2ac6e4..98d0a5baff70 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -1235,8 +1235,9 @@ 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 (atomic64_read(&mdsc->oldest_tid) == 0 &&
> +	    req->r_op != CEPH_MDS_OP_SETFILELOCK)
> +		atomic64_set(&mdsc->oldest_tid, req->r_tid);
>  
>  	if (dir) {
>  		struct ceph_inode_info *ci = ceph_inode(dir);
> @@ -1257,14 +1258,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 == atomic64_read(&mdsc->oldest_tid)) {
>  		struct rb_node *p = rb_next(&req->r_node);
> -		mdsc->oldest_tid = 0;
> +		atomic64_set(&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;
> +				atomic64_set(&mdsc->oldest_tid, next_req->r_tid);
>  				break;
>  			}
>  			p = rb_next(p);
> @@ -1693,7 +1694,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, atomic64_read(&mdsc->oldest_tid));
>  
>  	msg->front.iov_len = p - msg->front.iov_base;
>  	msg->hdr.front_len = cpu_to_le32(msg->front.iov_len);
> @@ -2759,7 +2760,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 atomic64_read(&mdsc->oldest_tid);
>  }
>  
>  #if IS_ENABLED(CONFIG_FS_ENCRYPTION)
> @@ -3438,9 +3439,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)
> @@ -3555,9 +3553,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)
> diff --git a/fs/ceph/mds_client.h b/fs/ceph/mds_client.h
> index 731d6ad04956..3b614b5df18c 100644
> --- a/fs/ceph/mds_client.h
> +++ b/fs/ceph/mds_client.h
> @@ -532,7 +532,7 @@ struct ceph_mds_client {
>  	spinlock_t              snap_empty_lock;  /* protect snap_empty */
>  
>  	u64                    last_tid;      /* most recent mds request */
> -	u64                    oldest_tid;    /* oldest incomplete mds request,
> +	atomic64_t             oldest_tid;    /* oldest incomplete mds request,
>  						 excluding setfilelock requests */
>  	struct rb_root         request_tree;  /* pending mds requests */
>  	struct delayed_work    delayed_work;  /* delayed work */
>