* [PATCH 0/5] ceph: reduce mdsc->mutex contention in the cephfs kclient
@ 2026-07-13 9:46 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, Xiubo Li
This series reduces mdsc->mutex hold times from hundreds of
microseconds to tens of microseconds on the hot request-submit and
reply-handling paths.
The approach is incremental:
1. Convert oldest_tid to atomic64_t so that __prepare_send_request()
and __send_request() no longer need the mutex.
2. Replace the request_tree rbtree with an xarray for O(1) lookups
and internally-locked iteration.
3. Add a dedicated wait_list_lock spinlock so wait-list operations
no longer depend on the global mutex.
4. Move mdsc->mutex acquisition inside __do_request(), then release
it during the send phase (message construction and path walking),
leaving only the brief setup/teardown under the lock.
5. Narrow the mutex scope in replay_unsafe_requests() similarly.
Tested with concurrent readdir + stat on a 5000-file directory
(32 threads). bpftrace measurements show:
before after
__do_request 354-2327 us 10-68 us (34x)
handle_reply 51-416 us 10-32 us (13x)
submit_request 89-211 us 10-41 us (5x)
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>
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/5] ceph: convert oldest_tid to atomic64_t
2026-07-13 9:46 ` Xiubo Li
@ 2026-07-13 9:46 ` Xiubo Li
-1 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, Xiubo Li
From: Xiubo Li <xiubo.li@clyso.com>
The oldest_client_tid sent in the MDS request header is advisory:
a stale value is harmless -- at worst the MDS may resend a reply
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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 1/5] ceph: convert oldest_tid to atomic64_t
@ 2026-07-13 9:46 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 1/5] ceph: convert oldest_tid to atomic64_t
2026-07-13 9:46 ` Xiubo Li
(?)
@ 2026-07-14 18:21 ` Viacheslav Dubeyko
-1 siblings, 0 replies; 22+ messages in thread
From: Viacheslav Dubeyko @ 2026-07-14 18:21 UTC (permalink / raw)
To: xiubo.li, Ilya Dryomov, Alex Markuze; +Cc: ceph-devel, linux-kernel
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.
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 1/5] ceph: convert oldest_tid to atomic64_t
2026-07-13 9:46 ` Xiubo Li
(?)
(?)
@ 2026-07-15 9:55 ` David Laight
-1 siblings, 0 replies; 22+ messages in thread
From: David Laight @ 2026-07-15 9:55 UTC (permalink / raw)
To: Xiubo Li via B4 Relay
Cc: xiubo.li, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
ceph-devel, linux-kernel
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 */
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.
2026-07-13 9:46 ` Xiubo Li
@ 2026-07-13 9:46 ` Xiubo Li
-1 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, Xiubo Li
From: Xiubo Li <xiubo.li@clyso.com>
The 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.
@ 2026-07-13 9:46 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.
2026-07-13 9:46 ` Xiubo Li
(?)
@ 2026-07-14 18:35 ` Viacheslav Dubeyko
2026-07-15 2:21 ` Xiubo Li
-1 siblings, 1 reply; 22+ messages in thread
From: Viacheslav Dubeyko @ 2026-07-14 18:35 UTC (permalink / raw)
To: xiubo.li, Ilya Dryomov, Alex Markuze; +Cc: ceph-devel, linux-kernel
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 */
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.
2026-07-14 18:35 ` Viacheslav Dubeyko
@ 2026-07-15 2:21 ` Xiubo Li
2026-07-15 17:56 ` Viacheslav Dubeyko
0 siblings, 1 reply; 22+ messages in thread
From: Xiubo Li @ 2026-07-15 2:21 UTC (permalink / raw)
To: Viacheslav Dubeyko; +Cc: Ilya Dryomov, Alex Markuze, ceph-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 2/5] ceph: replace the request_tree rbtree with an xarray keyed by r_tid.
2026-07-15 2:21 ` Xiubo Li
@ 2026-07-15 17:56 ` Viacheslav Dubeyko
0 siblings, 0 replies; 22+ messages in thread
From: Viacheslav Dubeyko @ 2026-07-15 17:56 UTC (permalink / raw)
To: Xiubo Li; +Cc: Ilya Dryomov, Alex Markuze, ceph-devel, linux-kernel
On Wed, 2026-07-15 at 10:21 +0800, Xiubo Li wrote:
> 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?
>
>
Do we know any customer that uses 32-bit builds? If there is no such
users, then we can restrict the builds by "depends on 64BIT" in
Kconfig. Otherwise, we have to provide opportunity to execute 32-bit
builds by proper supporting rbtree for this case. But, frankly
speaking, I would like to prefer simple solution like "depends on
64BIT".
Thanks,
Slava.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/5] ceph: add wait_list_lock for wait-list serialization
2026-07-13 9:46 ` Xiubo Li
@ 2026-07-13 9:46 ` Xiubo Li
-1 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, Xiubo Li
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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 3/5] ceph: add wait_list_lock for wait-list serialization
@ 2026-07-13 9:46 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 3/5] ceph: add wait_list_lock for wait-list serialization
2026-07-13 9:46 ` Xiubo Li
(?)
@ 2026-07-14 18:42 ` Viacheslav Dubeyko
-1 siblings, 0 replies; 22+ messages in thread
From: Viacheslav Dubeyko @ 2026-07-14 18:42 UTC (permalink / raw)
To: xiubo.li, Ilya Dryomov, Alex Markuze; +Cc: ceph-devel, linux-kernel
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/5] ceph: move mdsc->mutex into __do_request()
2026-07-13 9:46 ` Xiubo Li
@ 2026-07-13 9:46 ` Xiubo Li
-1 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, Xiubo Li
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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 4/5] ceph: move mdsc->mutex into __do_request()
@ 2026-07-13 9:46 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 4/5] ceph: move mdsc->mutex into __do_request()
2026-07-13 9:46 ` Xiubo Li
(?)
@ 2026-07-14 18:52 ` Viacheslav Dubeyko
2026-07-15 3:02 ` Xiubo Li
-1 siblings, 1 reply; 22+ messages in thread
From: Viacheslav Dubeyko @ 2026-07-14 18:52 UTC (permalink / raw)
To: xiubo.li, Ilya Dryomov, Alex Markuze; +Cc: ceph-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 4/5] ceph: move mdsc->mutex into __do_request()
2026-07-14 18:52 ` Viacheslav Dubeyko
@ 2026-07-15 3:02 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-15 3:02 UTC (permalink / raw)
To: Viacheslav Dubeyko; +Cc: Ilya Dryomov, Alex Markuze, ceph-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests
2026-07-13 9:46 ` Xiubo Li
@ 2026-07-13 9:46 ` Xiubo Li
-1 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li via B4 Relay @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, Xiubo Li
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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests
@ 2026-07-13 9:46 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-13 9:46 UTC (permalink / raw)
To: Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko
Cc: ceph-devel, linux-kernel, 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests
2026-07-13 9:46 ` Xiubo Li
(?)
@ 2026-07-14 19:07 ` Viacheslav Dubeyko
2026-07-15 3:14 ` Xiubo Li
-1 siblings, 1 reply; 22+ messages in thread
From: Viacheslav Dubeyko @ 2026-07-14 19:07 UTC (permalink / raw)
To: xiubo.li, Ilya Dryomov, Alex Markuze; +Cc: ceph-devel, linux-kernel
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)
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 5/5] ceph: narrow mdsc->mutex scope in replay_unsafe_requests
2026-07-14 19:07 ` Viacheslav Dubeyko
@ 2026-07-15 3:14 ` Xiubo Li
0 siblings, 0 replies; 22+ messages in thread
From: Xiubo Li @ 2026-07-15 3:14 UTC (permalink / raw)
To: Viacheslav Dubeyko; +Cc: Ilya Dryomov, Alex Markuze, ceph-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 22+ messages in thread