* [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
@ 2025-08-08 7:08 Zhao Sun
2025-08-11 19:43 ` Viacheslav Dubeyko
0 siblings, 1 reply; 9+ messages in thread
From: Zhao Sun @ 2025-08-08 7:08 UTC (permalink / raw)
To: xiubli, idryomov, amarkuze; +Cc: ceph-devel, linux-kernel, Zhao Sun
A deadlock can occur when ceph_get_inode is called outside of locks:
1) handle_reply calls ceph_get_inode, gets a new inode with I_NEW,
and blocks on mdsc->snap_rwsem for write.
2) At the same time, ceph_readdir_prepopulate calls ceph_get_inode
for the same inode while holding mdsc->snap_rwsem for read,
and blocks on I_NEW.
This causes an ABBA deadlock between mdsc->snap_rwsem and the I_NEW bit.
The issue was introduced by commit bca9fc14c70f
("ceph: when filling trace, call ceph_get_inode outside of mutexes")
which attempted to avoid a deadlock involving ceph_check_caps.
That concern is now obsolete since commit 6a92b08fdad2
("ceph: don't take s_mutex or snap_rwsem in ceph_check_caps")
which made ceph_check_caps fully lock-free.
This patch primarily reverts bca9fc14c70f to resolve the new deadlock,
with a few minor adjustments to fit the current codebase.
Link: https://tracker.ceph.com/issues/72307
Signed-off-by: Zhao Sun <sunzhao03@kuaishou.com>
---
fs/ceph/inode.c | 26 ++++++++++++++++++++++----
fs/ceph/mds_client.c | 29 -----------------------------
2 files changed, 22 insertions(+), 33 deletions(-)
diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 06cd2963e41e..d0f0035ee117 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1623,10 +1623,28 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
}
if (rinfo->head->is_target) {
- /* Should be filled in by handle_reply */
- BUG_ON(!req->r_target_inode);
+ in = xchg(&req->r_new_inode, NULL);
+ tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
+ tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
+
+ /*
+ * If we ended up opening an existing inode, discard
+ * r_new_inode
+ */
+ if (req->r_op == CEPH_MDS_OP_CREATE &&
+ !req->r_reply_info.has_create_ino) {
+ /* This should never happen on an async create */
+ WARN_ON_ONCE(req->r_deleg_ino);
+ iput(in);
+ in = NULL;
+ }
+
+ in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
+ if (IS_ERR(in)) {
+ err = PTR_ERR(in);
+ goto done;
+ }
- in = req->r_target_inode;
err = ceph_fill_inode(in, req->r_locked_page, &rinfo->targeti,
NULL, session,
(!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) &&
@@ -1636,13 +1654,13 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
if (err < 0) {
pr_err_client(cl, "badness %p %llx.%llx\n", in,
ceph_vinop(in));
- req->r_target_inode = NULL;
if (in->i_state & I_NEW)
discard_new_inode(in);
else
iput(in);
goto done;
}
+ req->r_target_inode = in;
if (in->i_state & I_NEW)
unlock_new_inode(in);
}
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 230e0c3f341f..8b70f2b96f46 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3874,36 +3874,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
session->s_con.peer_features);
mutex_unlock(&mdsc->mutex);
- /* Must find target inode outside of mutexes to avoid deadlocks */
rinfo = &req->r_reply_info;
- if ((err >= 0) && rinfo->head->is_target) {
- struct inode *in = xchg(&req->r_new_inode, NULL);
- struct ceph_vino tvino = {
- .ino = le64_to_cpu(rinfo->targeti.in->ino),
- .snap = le64_to_cpu(rinfo->targeti.in->snapid)
- };
-
- /*
- * If we ended up opening an existing inode, discard
- * r_new_inode
- */
- if (req->r_op == CEPH_MDS_OP_CREATE &&
- !req->r_reply_info.has_create_ino) {
- /* This should never happen on an async create */
- WARN_ON_ONCE(req->r_deleg_ino);
- iput(in);
- in = NULL;
- }
-
- in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
- if (IS_ERR(in)) {
- err = PTR_ERR(in);
- mutex_lock(&session->s_mutex);
- goto out_err;
- }
- req->r_target_inode = in;
- }
-
mutex_lock(&session->s_mutex);
if (err < 0) {
pr_err_client(cl, "got corrupt reply mds%d(tid:%lld)\n",
--
2.39.2 (Apple Git-143)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
2025-08-08 7:08 [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate Zhao Sun
@ 2025-08-11 19:43 ` Viacheslav Dubeyko
[not found] ` <409ca87f8add4a80a7927981059b3c5f@kuaishou.com>
0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2025-08-11 19:43 UTC (permalink / raw)
To: Alex Markuze, sunzhao03, Xiubo Li, idryomov; +Cc: ceph-devel, linux-kernel
On Fri, 2025-08-08 at 15:08 +0800, Zhao Sun wrote:
> A deadlock can occur when ceph_get_inode is called outside of locks:
>
> 1) handle_reply calls ceph_get_inode, gets a new inode with I_NEW,
> and blocks on mdsc->snap_rwsem for write.
>
Frankly speaking, it's hard to follow to your logic. Which particular mdsc-
>snap_rwsem lock do you mean in handle_reply()?
> 2) At the same time, ceph_readdir_prepopulate calls ceph_get_inode
> for the same inode while holding mdsc->snap_rwsem for read,
> and blocks on I_NEW.
>
The same here. Which particular mdsc->snap_rwsem lock do you mean in
ceph_readdir_prepopulate()?
> This causes an ABBA deadlock between mdsc->snap_rwsem and the I_NEW bit.
>
> The issue was introduced by commit bca9fc14c70f
> ("ceph: when filling trace, call ceph_get_inode outside of mutexes")
> which attempted to avoid a deadlock involving ceph_check_caps.
>
> That concern is now obsolete since commit 6a92b08fdad2
> ("ceph: don't take s_mutex or snap_rwsem in ceph_check_caps")
> which made ceph_check_caps fully lock-free.
>
> This patch primarily reverts bca9fc14c70f to resolve the new deadlock,
> with a few minor adjustments to fit the current codebase.
>
I assume that you hit the issue. I believe it will be good to have the
explanation which use-case/workload trigger the issue and which symptoms do you
see (system log's content, for example).
Thanks,
Slava.
> Link: https://tracker.ceph.com/issues/72307
> Signed-off-by: Zhao Sun <sunzhao03@kuaishou.com>
> ---
> fs/ceph/inode.c | 26 ++++++++++++++++++++++----
> fs/ceph/mds_client.c | 29 -----------------------------
> 2 files changed, 22 insertions(+), 33 deletions(-)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 06cd2963e41e..d0f0035ee117 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1623,10 +1623,28 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> }
>
> if (rinfo->head->is_target) {
> - /* Should be filled in by handle_reply */
> - BUG_ON(!req->r_target_inode);
> + in = xchg(&req->r_new_inode, NULL);
> + tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
> + tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
> +
> + /*
> + * If we ended up opening an existing inode, discard
> + * r_new_inode
> + */
> + if (req->r_op == CEPH_MDS_OP_CREATE &&
> + !req->r_reply_info.has_create_ino) {
> + /* This should never happen on an async create */
> + WARN_ON_ONCE(req->r_deleg_ino);
> + iput(in);
> + in = NULL;
> + }
> +
> + in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> + if (IS_ERR(in)) {
> + err = PTR_ERR(in);
> + goto done;
> + }
>
> - in = req->r_target_inode;
> err = ceph_fill_inode(in, req->r_locked_page, &rinfo->targeti,
> NULL, session,
> (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) &&
> @@ -1636,13 +1654,13 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> if (err < 0) {
> pr_err_client(cl, "badness %p %llx.%llx\n", in,
> ceph_vinop(in));
> - req->r_target_inode = NULL;
> if (in->i_state & I_NEW)
> discard_new_inode(in);
> else
> iput(in);
> goto done;
> }
> + req->r_target_inode = in;
> if (in->i_state & I_NEW)
> unlock_new_inode(in);
> }
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 230e0c3f341f..8b70f2b96f46 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3874,36 +3874,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
> session->s_con.peer_features);
> mutex_unlock(&mdsc->mutex);
>
> - /* Must find target inode outside of mutexes to avoid deadlocks */
> rinfo = &req->r_reply_info;
> - if ((err >= 0) && rinfo->head->is_target) {
> - struct inode *in = xchg(&req->r_new_inode, NULL);
> - struct ceph_vino tvino = {
> - .ino = le64_to_cpu(rinfo->targeti.in->ino),
> - .snap = le64_to_cpu(rinfo->targeti.in->snapid)
> - };
> -
> - /*
> - * If we ended up opening an existing inode, discard
> - * r_new_inode
> - */
> - if (req->r_op == CEPH_MDS_OP_CREATE &&
> - !req->r_reply_info.has_create_ino) {
> - /* This should never happen on an async create */
> - WARN_ON_ONCE(req->r_deleg_ino);
> - iput(in);
> - in = NULL;
> - }
> -
> - in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> - if (IS_ERR(in)) {
> - err = PTR_ERR(in);
> - mutex_lock(&session->s_mutex);
> - goto out_err;
> - }
> - req->r_target_inode = in;
> - }
> -
> mutex_lock(&session->s_mutex);
> if (err < 0) {
> pr_err_client(cl, "got corrupt reply mds%d(tid:%lld)\n",
^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
[not found] ` <409ca87f8add4a80a7927981059b3c5f@kuaishou.com>
@ 2026-01-04 9:19 ` 李磊
2026-01-05 19:55 ` Viacheslav Dubeyko
0 siblings, 1 reply; 9+ messages in thread
From: 李磊 @ 2026-01-04 9:19 UTC (permalink / raw)
To: Slava.Dubeyko
Cc: 孙朝, ceph-devel, linux-kernel, amarkuze, idryomov
Hi Slava,
I guess here is the deadlock scenario described by Zhao.
1. DIR A has a snapshot and FILE X already in DIR A.
2. client submits create FILE X and readdir DIR A requests almost simultaneously.
3. The 2 requests are handled by 2 different kworkers.
kworker1:
1. since FILE X already exists, A inode with I_NEW flag is set by ceph_get_inode().
2. rinfo->snapblob_len != 0; So, kworker1 is blocked on down_write(&mdsc->snap_rwsem);
kworker2:
1. enter handle_reply
2. hold mdsc->snap_rwsem
3. call ceph_readdir_prepopulate()
4. in ceph_readdir_prepopulate(), it iterates all the ceph_mds_reply_dir_entry.
5. the ino of FILE X is found, and call ceph_get_inode().
6. However, the inode is set with I_NEW, kworker has to wait I_NEW cleared.
Please correct me if I missed something. Here is the timeline of the 2 kworkers
kworker1 [handle creating] kworker2 [handle readdir]
| |
v v
handle_reply handle_reply
| |
| v
| down_read(&mdsc->snap_rwrem);
| |
| |
v |
/* opening an existing inode */ |
ceph_get_inode (I_NEW) |
| |
v |
rinfo->snapblob_len != 0; |
down_write(&mdsc->snap_rwsem); |
|
|
v
ceph_fill_trace
|
v
ceph_readdir_prepoulate
|
v
ceph_get_inode
|
v
iget5_locked
|
v
ilookup5
|
v
/* waiting I_NEW cleared */
wait_on_inode
Here comes the callstack:
task:kworker/21:2 state:D stack: 0 pid:23053 ppid: 2 flags:0x00004000
Workqueue: events delayed_work [ceph]
Call Trace:
__schedule+0x3a9/0x8d0
schedule+0x49/0xb0
schedule_preempt_disabled+0xa/0x10
__mutex_lock.isra.11+0x354/0x430
delayed_work+0x13b/0x210 [ceph]
process_one_work+0x1cb/0x370
worker_thread+0x30/0x390
? process_one_work+0x370/0x370
kthread+0x13e/0x160
? set_kthread_struct+0x50/0x50
ret_from_fork+0x1f/0x30
task:kworker/u113:1 state:D stack: 0 pid:34454 ppid: 2 flags:0x00004000
Workqueue: ceph-msgr ceph_con_workfn [libceph]
Call Trace:
__schedule+0x3a9/0x8d0
schedule+0x49/0xb0
rwsem_down_write_slowpath+0x30a/0x5e0
handle_reply+0x4d7/0x7f0 [ceph]
? ceph_tcp_recvmsg+0x6f/0xa0 [libceph]
mds_dispatch+0x10a/0x690 [ceph]
? calc_signature+0xdf/0x110 [libceph]
? ceph_x_check_message_signature+0x58/0xc0 [libceph]
ceph_con_process_message+0x73/0x140 [libceph]
ceph_con_v1_try_read+0x2f2/0x860 [libceph]
ceph_con_workfn+0x31e/0x660 [libceph]
process_one_work+0x1cb/0x370
worker_thread+0x30/0x390
? process_one_work+0x370/0x370
kthread+0x13e/0x160
? set_kthread_struct+0x50/0x50
ret_from_fork+0x1f/0x30
task:kworker/u113:2 state:D stack: 0 pid:54267 ppid: 2 flags:0x00004000
Workqueue: ceph-msgr ceph_con_workfn [libceph]
Call Trace:
__schedule+0x3a9/0x8d0
? bit_wait_io+0x60/0x60
? bit_wait_io+0x60/0x60
schedule+0x49/0xb0
bit_wait+0xd/0x60
__wait_on_bit+0x2a/0x90
? ceph_force_reconnect+0x90/0x90 [ceph]
out_of_line_wait_on_bit+0x91/0xb0
? bitmap_empty+0x20/0x20
ilookup5.part.29+0x69/0x90
? ceph_force_reconnect+0x90/0x90 [ceph]
? ceph_ino_compare+0x30/0x30 [ceph]
iget5_locked+0x26/0x90
ceph_get_inode+0x45/0x130 [ceph]
ceph_readdir_prepopulate+0x59f/0xca0 [ceph]
handle_reply+0x78d/0x7f0 [ceph]
? ceph_tcp_recvmsg+0x6f/0xa0 [libceph]
mds_dispatch+0x10a/0x690 [ceph]
? calc_signature+0xdf/0x110 [libceph]
? ceph_x_check_message_signature+0x58/0xc0 [libceph]
ceph_con_process_message+0x73/0x140 [libceph]
ceph_con_v1_try_read+0x2f2/0x860 [libceph]
ceph_con_workfn+0x31e/0x660 [libceph]
process_one_work+0x1cb/0x370
worker_thread+0x30/0x390
? process_one_work+0x370/0x370
kthread+0x13e/0x160
? set_kthread_struct+0x50/0x50
ret_from_fork+0x1f/0x30
On Fri, 2025-08-08 at 15:08 +0800, Zhao Sun wrote:
> A deadlock can occur when ceph_get_inode is called outside of locks:
>
> 1) handle_reply calls ceph_get_inode, gets a new inode with I_NEW,
> and blocks on mdsc->snap_rwsem for write.
>
Frankly speaking, it's hard to follow to your logic. Which particular mdsc-
>snap_rwsem lock do you mean in handle_reply()?
> 2) At the same time, ceph_readdir_prepopulate calls ceph_get_inode
> for the same inode while holding mdsc->snap_rwsem for read,
> and blocks on I_NEW.
>
The same here. Which particular mdsc->snap_rwsem lock do you mean in
ceph_readdir_prepopulate()?
> This causes an ABBA deadlock between mdsc->snap_rwsem and the I_NEW bit.
>
> The issue was introduced by commit bca9fc14c70f
> ("ceph: when filling trace, call ceph_get_inode outside of mutexes")
> which attempted to avoid a deadlock involving ceph_check_caps.
>
> That concern is now obsolete since commit 6a92b08fdad2
> ("ceph: don't take s_mutex or snap_rwsem in ceph_check_caps")
> which made ceph_check_caps fully lock-free.
>
> This patch primarily reverts bca9fc14c70f to resolve the new deadlock,
> with a few minor adjustments to fit the current codebase.
>
I assume that you hit the issue. I believe it will be good to have the
explanation which use-case/workload trigger the issue and which symptoms do you
see (system log's content, for example).
Thanks,
Slava.
> Link: https://tracker.ceph.com/issues/72307
> Signed-off-by: Zhao Sun <sunzhao03@kuaishou.com>
> ---
> fs/ceph/inode.c | 26 ++++++++++++++++++++++----
> fs/ceph/mds_client.c | 29 -----------------------------
> 2 files changed, 22 insertions(+), 33 deletions(-)
>
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 06cd2963e41e..d0f0035ee117 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1623,10 +1623,28 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> }
>
> if (rinfo->head->is_target) {
> - /* Should be filled in by handle_reply */
> - BUG_ON(!req->r_target_inode);
> + in = xchg(&req->r_new_inode, NULL);
> + tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
> + tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
> +
> + /*
> + * If we ended up opening an existing inode, discard
> + * r_new_inode
> + */
> + if (req->r_op == CEPH_MDS_OP_CREATE &&
> + !req->r_reply_info.has_create_ino) {
> + /* This should never happen on an async create */
> + WARN_ON_ONCE(req->r_deleg_ino);
> + iput(in);
> + in = NULL;
> + }
> +
> + in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> + if (IS_ERR(in)) {
> + err = PTR_ERR(in);
> + goto done;
> + }
>
> - in = req->r_target_inode;
> err = ceph_fill_inode(in, req->r_locked_page, &rinfo->targeti,
> NULL, session,
> (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) &&
> @@ -1636,13 +1654,13 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> if (err < 0) {
> pr_err_client(cl, "badness %p %llx.%llx\n", in,
> ceph_vinop(in));
> - req->r_target_inode = NULL;
> if (in->i_state & I_NEW)
> discard_new_inode(in);
> else
> iput(in);
> goto done;
> }
> + req->r_target_inode = in;
> if (in->i_state & I_NEW)
> unlock_new_inode(in);
> }
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 230e0c3f341f..8b70f2b96f46 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3874,36 +3874,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
> session->s_con.peer_features);
> mutex_unlock(&mdsc->mutex);
>
> - /* Must find target inode outside of mutexes to avoid deadlocks */
> rinfo = &req->r_reply_info;
> - if ((err >= 0) && rinfo->head->is_target) {
> - struct inode *in = xchg(&req->r_new_inode, NULL);
> - struct ceph_vino tvino = {
> - .ino = le64_to_cpu(rinfo->targeti.in->ino),
> - .snap = le64_to_cpu(rinfo->targeti.in->snapid)
> - };
> -
> - /*
> - * If we ended up opening an existing inode, discard
> - * r_new_inode
> - */
> - if (req->r_op == CEPH_MDS_OP_CREATE &&
> - !req->r_reply_info.has_create_ino) {
> - /* This should never happen on an async create */
> - WARN_ON_ONCE(req->r_deleg_ino);
> - iput(in);
> - in = NULL;
> - }
> -
> - in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> - if (IS_ERR(in)) {
> - err = PTR_ERR(in);
> - mutex_lock(&session->s_mutex);
> - goto out_err;
> - }
> - req->r_target_inode = in;
> - }
> -
> mutex_lock(&session->s_mutex);
> if (err < 0) {
> pr_err_client(cl, "got corrupt reply mds%d(tid:%lld)\n",
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
2026-01-04 9:19 ` 答复: 【外部邮件!】Re: " 李磊
@ 2026-01-05 19:55 ` Viacheslav Dubeyko
2026-01-07 16:01 ` 答复: " 李磊
0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2026-01-05 19:55 UTC (permalink / raw)
To: lilei24; +Cc: idryomov, sunzhao03, ceph-devel, linux-kernel, Alex Markuze
On Sun, 2026-01-04 at 09:19 +0000, 李磊 wrote:
> Hi Slava,
>
> I guess here is the deadlock scenario described by Zhao.
> 1. DIR A has a snapshot and FILE X already in DIR A.
> 2. client submits create FILE X and readdir DIR A requests almost simultaneously.
> 3. The 2 requests are handled by 2 different kworkers.
>
> kworker1:
> 1. since FILE X already exists, A inode with I_NEW flag is set by ceph_get_inode().
> 2. rinfo->snapblob_len != 0; So, kworker1 is blocked on down_write(&mdsc->snap_rwsem);
>
> kworker2:
> 1. enter handle_reply
> 2. hold mdsc->snap_rwsem
> 3. call ceph_readdir_prepopulate()
> 4. in ceph_readdir_prepopulate(), it iterates all the ceph_mds_reply_dir_entry.
> 5. the ino of FILE X is found, and call ceph_get_inode().
> 6. However, the inode is set with I_NEW, kworker has to wait I_NEW cleared.
>
> Please correct me if I missed something. Here is the timeline of the 2 kworkers
This explanation sounds reasonable to me. If you can share the call trace of the
issue, then I assume that you have reproducing script or application. Could you
please share this reproducer? I would like to try to reproduce the issue.
It will be great if you can resend the patch with all this nice explanation in
the commit message. Could you please resend the patch?
Thanks,
Slava.
>
>
> kworker1 [handle creating] kworker2 [handle readdir]
> | |
> v v
> handle_reply handle_reply
> | |
> | v
> | down_read(&mdsc->snap_rwrem);
> | |
> | |
> v |
> /* opening an existing inode */ |
> ceph_get_inode (I_NEW) |
> | |
> v |
> rinfo->snapblob_len != 0; |
> down_write(&mdsc->snap_rwsem); |
> |
> |
> v
> ceph_fill_trace
> |
> v
> ceph_readdir_prepoulate
> |
> v
> ceph_get_inode
> |
> v
> iget5_locked
> |
> v
> ilookup5
> |
> v
> /* waiting I_NEW cleared */
> wait_on_inode
>
>
> Here comes the callstack:
> task:kworker/21:2 state:D stack: 0 pid:23053 ppid: 2 flags:0x00004000
> Workqueue: events delayed_work [ceph]
> Call Trace:
> __schedule+0x3a9/0x8d0
> schedule+0x49/0xb0
> schedule_preempt_disabled+0xa/0x10
> __mutex_lock.isra.11+0x354/0x430
> delayed_work+0x13b/0x210 [ceph]
> process_one_work+0x1cb/0x370
> worker_thread+0x30/0x390
> ? process_one_work+0x370/0x370
> kthread+0x13e/0x160
> ? set_kthread_struct+0x50/0x50
> ret_from_fork+0x1f/0x30
> task:kworker/u113:1 state:D stack: 0 pid:34454 ppid: 2 flags:0x00004000
> Workqueue: ceph-msgr ceph_con_workfn [libceph]
> Call Trace:
> __schedule+0x3a9/0x8d0
> schedule+0x49/0xb0
> rwsem_down_write_slowpath+0x30a/0x5e0
> handle_reply+0x4d7/0x7f0 [ceph]
> ? ceph_tcp_recvmsg+0x6f/0xa0 [libceph]
> mds_dispatch+0x10a/0x690 [ceph]
> ? calc_signature+0xdf/0x110 [libceph]
> ? ceph_x_check_message_signature+0x58/0xc0 [libceph]
> ceph_con_process_message+0x73/0x140 [libceph]
> ceph_con_v1_try_read+0x2f2/0x860 [libceph]
> ceph_con_workfn+0x31e/0x660 [libceph]
> process_one_work+0x1cb/0x370
> worker_thread+0x30/0x390
> ? process_one_work+0x370/0x370
> kthread+0x13e/0x160
> ? set_kthread_struct+0x50/0x50
> ret_from_fork+0x1f/0x30
> task:kworker/u113:2 state:D stack: 0 pid:54267 ppid: 2 flags:0x00004000
> Workqueue: ceph-msgr ceph_con_workfn [libceph]
> Call Trace:
> __schedule+0x3a9/0x8d0
> ? bit_wait_io+0x60/0x60
> ? bit_wait_io+0x60/0x60
> schedule+0x49/0xb0
> bit_wait+0xd/0x60
> __wait_on_bit+0x2a/0x90
> ? ceph_force_reconnect+0x90/0x90 [ceph]
> out_of_line_wait_on_bit+0x91/0xb0
> ? bitmap_empty+0x20/0x20
> ilookup5.part.29+0x69/0x90
> ? ceph_force_reconnect+0x90/0x90 [ceph]
> ? ceph_ino_compare+0x30/0x30 [ceph]
> iget5_locked+0x26/0x90
> ceph_get_inode+0x45/0x130 [ceph]
> ceph_readdir_prepopulate+0x59f/0xca0 [ceph]
> handle_reply+0x78d/0x7f0 [ceph]
> ? ceph_tcp_recvmsg+0x6f/0xa0 [libceph]
> mds_dispatch+0x10a/0x690 [ceph]
> ? calc_signature+0xdf/0x110 [libceph]
> ? ceph_x_check_message_signature+0x58/0xc0 [libceph]
> ceph_con_process_message+0x73/0x140 [libceph]
> ceph_con_v1_try_read+0x2f2/0x860 [libceph]
> ceph_con_workfn+0x31e/0x660 [libceph]
> process_one_work+0x1cb/0x370
> worker_thread+0x30/0x390
> ? process_one_work+0x370/0x370
> kthread+0x13e/0x160
> ? set_kthread_struct+0x50/0x50
> ret_from_fork+0x1f/0x30
>
>
> On Fri, 2025-08-08 at 15:08 +0800, Zhao Sun wrote:
> > A deadlock can occur when ceph_get_inode is called outside of locks:
> >
> > 1) handle_reply calls ceph_get_inode, gets a new inode with I_NEW,
> > and blocks on mdsc->snap_rwsem for write.
> >
>
> Frankly speaking, it's hard to follow to your logic. Which particular mdsc-
> > snap_rwsem lock do you mean in handle_reply()?
>
> > 2) At the same time, ceph_readdir_prepopulate calls ceph_get_inode
> > for the same inode while holding mdsc->snap_rwsem for read,
> > and blocks on I_NEW.
> >
>
> The same here. Which particular mdsc->snap_rwsem lock do you mean in
> ceph_readdir_prepopulate()?
>
> > This causes an ABBA deadlock between mdsc->snap_rwsem and the I_NEW bit.
> >
> > The issue was introduced by commit bca9fc14c70f
> > ("ceph: when filling trace, call ceph_get_inode outside of mutexes")
> > which attempted to avoid a deadlock involving ceph_check_caps.
> >
> > That concern is now obsolete since commit 6a92b08fdad2
> > ("ceph: don't take s_mutex or snap_rwsem in ceph_check_caps")
> > which made ceph_check_caps fully lock-free.
> >
> > This patch primarily reverts bca9fc14c70f to resolve the new deadlock,
> > with a few minor adjustments to fit the current codebase.
> >
>
> I assume that you hit the issue. I believe it will be good to have the
> explanation which use-case/workload trigger the issue and which symptoms do you
> see (system log's content, for example).
>
> Thanks,
> Slava.
>
> > Link: https://tracker.ceph.com/issues/72307
> > Signed-off-by: Zhao Sun <sunzhao03@kuaishou.com>
> > ---
> > fs/ceph/inode.c | 26 ++++++++++++++++++++++----
> > fs/ceph/mds_client.c | 29 -----------------------------
> > 2 files changed, 22 insertions(+), 33 deletions(-)
> >
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index 06cd2963e41e..d0f0035ee117 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -1623,10 +1623,28 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> > }
> >
> > if (rinfo->head->is_target) {
> > - /* Should be filled in by handle_reply */
> > - BUG_ON(!req->r_target_inode);
> > + in = xchg(&req->r_new_inode, NULL);
> > + tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
> > + tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
> > +
> > + /*
> > + * If we ended up opening an existing inode, discard
> > + * r_new_inode
> > + */
> > + if (req->r_op == CEPH_MDS_OP_CREATE &&
> > + !req->r_reply_info.has_create_ino) {
> > + /* This should never happen on an async create */
> > + WARN_ON_ONCE(req->r_deleg_ino);
> > + iput(in);
> > + in = NULL;
> > + }
> > +
> > + in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> > + if (IS_ERR(in)) {
> > + err = PTR_ERR(in);
> > + goto done;
> > + }
> >
> > - in = req->r_target_inode;
> > err = ceph_fill_inode(in, req->r_locked_page, &rinfo->targeti,
> > NULL, session,
> > (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) &&
> > @@ -1636,13 +1654,13 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> > if (err < 0) {
> > pr_err_client(cl, "badness %p %llx.%llx\n", in,
> > ceph_vinop(in));
> > - req->r_target_inode = NULL;
> > if (in->i_state & I_NEW)
> > discard_new_inode(in);
> > else
> > iput(in);
> > goto done;
> > }
> > + req->r_target_inode = in;
> > if (in->i_state & I_NEW)
> > unlock_new_inode(in);
> > }
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index 230e0c3f341f..8b70f2b96f46 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -3874,36 +3874,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
> > session->s_con.peer_features);
> > mutex_unlock(&mdsc->mutex);
> >
> > - /* Must find target inode outside of mutexes to avoid deadlocks */
> > rinfo = &req->r_reply_info;
> > - if ((err >= 0) && rinfo->head->is_target) {
> > - struct inode *in = xchg(&req->r_new_inode, NULL);
> > - struct ceph_vino tvino = {
> > - .ino = le64_to_cpu(rinfo->targeti.in->ino),
> > - .snap = le64_to_cpu(rinfo->targeti.in->snapid)
> > - };
> > -
> > - /*
> > - * If we ended up opening an existing inode, discard
> > - * r_new_inode
> > - */
> > - if (req->r_op == CEPH_MDS_OP_CREATE &&
> > - !req->r_reply_info.has_create_ino) {
> > - /* This should never happen on an async create */
> > - WARN_ON_ONCE(req->r_deleg_ino);
> > - iput(in);
> > - in = NULL;
> > - }
> > -
> > - in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> > - if (IS_ERR(in)) {
> > - err = PTR_ERR(in);
> > - mutex_lock(&session->s_mutex);
> > - goto out_err;
> > - }
> > - req->r_target_inode = in;
> > - }
> > -
> > mutex_lock(&session->s_mutex);
> > if (err < 0) {
> > pr_err_client(cl, "got corrupt reply mds%d(tid:%lld)\n",
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
2026-01-05 19:55 ` Viacheslav Dubeyko
@ 2026-01-07 16:01 ` 李磊
2026-01-07 19:59 ` Viacheslav Dubeyko
0 siblings, 1 reply; 9+ messages in thread
From: 李磊 @ 2026-01-07 16:01 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: idryomov, 孙朝, ceph-devel, linux-kernel, Alex Markuze
Hi Slava,
This issue is very rare on our internal cephfs clusters. We had only encountered it for about three times.
But we are working on same hacking methods to speed up the reproduction. I think it will take me one week
if everything goes smoothly and I will share the methods here.
To be honest, this patch should be a revert patch of this one:
commit : bca9fc14c70fcbbebc84954cc39994e463fb9468
ceph: when filling trace, call ceph_get_inode outside of mutexes
I'll resend this patch later.
Thanks
________________________________________
发件人: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
发送时间: 2026年1月6日 3:55
收件人: 李磊
抄送: idryomov@gmail.com; 孙朝; ceph-devel@vger.kernel.org; linux-kernel@vger.kernel.org; Alex Markuze
主题: Re: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
On Sun, 2026-01-04 at 09:19 +0000, 李磊 wrote:
> Hi Slava,
>
> I guess here is the deadlock scenario described by Zhao.
> 1. DIR A has a snapshot and FILE X already in DIR A.
> 2. client submits create FILE X and readdir DIR A requests almost simultaneously.
> 3. The 2 requests are handled by 2 different kworkers.
>
> kworker1:
> 1. since FILE X already exists, A inode with I_NEW flag is set by ceph_get_inode().
> 2. rinfo->snapblob_len != 0; So, kworker1 is blocked on down_write(&mdsc->snap_rwsem);
>
> kworker2:
> 1. enter handle_reply
> 2. hold mdsc->snap_rwsem
> 3. call ceph_readdir_prepopulate()
> 4. in ceph_readdir_prepopulate(), it iterates all the ceph_mds_reply_dir_entry.
> 5. the ino of FILE X is found, and call ceph_get_inode().
> 6. However, the inode is set with I_NEW, kworker has to wait I_NEW cleared.
>
> Please correct me if I missed something. Here is the timeline of the 2 kworkers
This explanation sounds reasonable to me. If you can share the call trace of the
issue, then I assume that you have reproducing script or application. Could you
please share this reproducer? I would like to try to reproduce the issue.
It will be great if you can resend the patch with all this nice explanation in
the commit message. Could you please resend the patch?
Thanks,
Slava.
>
>
> kworker1 [handle creating] kworker2 [handle readdir]
> | |
> v v
> handle_reply handle_reply
> | |
> | v
> | down_read(&mdsc->snap_rwrem);
> | |
> | |
> v |
> /* opening an existing inode */ |
> ceph_get_inode (I_NEW) |
> | |
> v |
> rinfo->snapblob_len != 0; |
> down_write(&mdsc->snap_rwsem); |
> |
> |
> v
> ceph_fill_trace
> |
> v
> ceph_readdir_prepoulate
> |
> v
> ceph_get_inode
> |
> v
> iget5_locked
> |
> v
> ilookup5
> |
> v
> /* waiting I_NEW cleared */
> wait_on_inode
>
>
> Here comes the callstack:
> task:kworker/21:2 state:D stack: 0 pid:23053 ppid: 2 flags:0x00004000
> Workqueue: events delayed_work [ceph]
> Call Trace:
> __schedule+0x3a9/0x8d0
> schedule+0x49/0xb0
> schedule_preempt_disabled+0xa/0x10
> __mutex_lock.isra.11+0x354/0x430
> delayed_work+0x13b/0x210 [ceph]
> process_one_work+0x1cb/0x370
> worker_thread+0x30/0x390
> ? process_one_work+0x370/0x370
> kthread+0x13e/0x160
> ? set_kthread_struct+0x50/0x50
> ret_from_fork+0x1f/0x30
> task:kworker/u113:1 state:D stack: 0 pid:34454 ppid: 2 flags:0x00004000
> Workqueue: ceph-msgr ceph_con_workfn [libceph]
> Call Trace:
> __schedule+0x3a9/0x8d0
> schedule+0x49/0xb0
> rwsem_down_write_slowpath+0x30a/0x5e0
> handle_reply+0x4d7/0x7f0 [ceph]
> ? ceph_tcp_recvmsg+0x6f/0xa0 [libceph]
> mds_dispatch+0x10a/0x690 [ceph]
> ? calc_signature+0xdf/0x110 [libceph]
> ? ceph_x_check_message_signature+0x58/0xc0 [libceph]
> ceph_con_process_message+0x73/0x140 [libceph]
> ceph_con_v1_try_read+0x2f2/0x860 [libceph]
> ceph_con_workfn+0x31e/0x660 [libceph]
> process_one_work+0x1cb/0x370
> worker_thread+0x30/0x390
> ? process_one_work+0x370/0x370
> kthread+0x13e/0x160
> ? set_kthread_struct+0x50/0x50
> ret_from_fork+0x1f/0x30
> task:kworker/u113:2 state:D stack: 0 pid:54267 ppid: 2 flags:0x00004000
> Workqueue: ceph-msgr ceph_con_workfn [libceph]
> Call Trace:
> __schedule+0x3a9/0x8d0
> ? bit_wait_io+0x60/0x60
> ? bit_wait_io+0x60/0x60
> schedule+0x49/0xb0
> bit_wait+0xd/0x60
> __wait_on_bit+0x2a/0x90
> ? ceph_force_reconnect+0x90/0x90 [ceph]
> out_of_line_wait_on_bit+0x91/0xb0
> ? bitmap_empty+0x20/0x20
> ilookup5.part.29+0x69/0x90
> ? ceph_force_reconnect+0x90/0x90 [ceph]
> ? ceph_ino_compare+0x30/0x30 [ceph]
> iget5_locked+0x26/0x90
> ceph_get_inode+0x45/0x130 [ceph]
> ceph_readdir_prepopulate+0x59f/0xca0 [ceph]
> handle_reply+0x78d/0x7f0 [ceph]
> ? ceph_tcp_recvmsg+0x6f/0xa0 [libceph]
> mds_dispatch+0x10a/0x690 [ceph]
> ? calc_signature+0xdf/0x110 [libceph]
> ? ceph_x_check_message_signature+0x58/0xc0 [libceph]
> ceph_con_process_message+0x73/0x140 [libceph]
> ceph_con_v1_try_read+0x2f2/0x860 [libceph]
> ceph_con_workfn+0x31e/0x660 [libceph]
> process_one_work+0x1cb/0x370
> worker_thread+0x30/0x390
> ? process_one_work+0x370/0x370
> kthread+0x13e/0x160
> ? set_kthread_struct+0x50/0x50
> ret_from_fork+0x1f/0x30
>
>
> On Fri, 2025-08-08 at 15:08 +0800, Zhao Sun wrote:
> > A deadlock can occur when ceph_get_inode is called outside of locks:
> >
> > 1) handle_reply calls ceph_get_inode, gets a new inode with I_NEW,
> > and blocks on mdsc->snap_rwsem for write.
> >
>
> Frankly speaking, it's hard to follow to your logic. Which particular mdsc-
> > snap_rwsem lock do you mean in handle_reply()?
>
> > 2) At the same time, ceph_readdir_prepopulate calls ceph_get_inode
> > for the same inode while holding mdsc->snap_rwsem for read,
> > and blocks on I_NEW.
> >
>
> The same here. Which particular mdsc->snap_rwsem lock do you mean in
> ceph_readdir_prepopulate()?
>
> > This causes an ABBA deadlock between mdsc->snap_rwsem and the I_NEW bit.
> >
> > The issue was introduced by commit bca9fc14c70f
> > ("ceph: when filling trace, call ceph_get_inode outside of mutexes")
> > which attempted to avoid a deadlock involving ceph_check_caps.
> >
> > That concern is now obsolete since commit q
> > ("ceph: don't take s_mutex or snap_rwsem in ceph_check_caps")
> > which made ceph_check_caps fully lock-free.
> >
> > This patch primarily reverts bca9fc14c70f to resolve the new deadlock,
> > with a few minor adjustments to fit the current codebase.
> >
>
> I assume that you hit the issue. I believe it will be good to have the
> explanation which use-case/workload trigger the issue and which symptoms do you
> see (system log's content, for example).
>
> Thanks,
> Slava.
>
> > Link: https://tracker.ceph.com/issues/72307
> > Signed-off-by: Zhao Sun <sunzhao03@kuaishou.com>
> > ---
> > fs/ceph/inode.c | 26 ++++++++++++++++++++++----
> > fs/ceph/mds_client.c | 29 -----------------------------
> > 2 files changed, 22 insertions(+), 33 deletions(-)
> >
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index 06cd2963e41e..d0f0035ee117 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -1623,10 +1623,28 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> > }
> >
> > if (rinfo->head->is_target) {
> > - /* Should be filled in by handle_reply */
> > - BUG_ON(!req->r_target_inode);
> > + in = xchg(&req->r_new_inode, NULL);
> > + tvino.ino = le64_to_cpu(rinfo->targeti.in->ino);
> > + tvino.snap = le64_to_cpu(rinfo->targeti.in->snapid);
> > +
> > + /*
> > + * If we ended up opening an existing inode, discard
> > + * r_new_inode
> > + */
> > + if (req->r_op == CEPH_MDS_OP_CREATE &&
> > + !req->r_reply_info.has_create_ino) {
> > + /* This should never happen on an async create */
> > + WARN_ON_ONCE(req->r_deleg_ino);
> > + iput(in);
> > + in = NULL;
> > + }
> > +
> > + in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> > + if (IS_ERR(in)) {
> > + err = PTR_ERR(in);
> > + goto done;
> > + }
> >
> > - in = req->r_target_inode;
> > err = ceph_fill_inode(in, req->r_locked_page, &rinfo->targeti,
> > NULL, session,
> > (!test_bit(CEPH_MDS_R_ABORTED, &req->r_req_flags) &&
> > @@ -1636,13 +1654,13 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> > if (err < 0) {
> > pr_err_client(cl, "badness %p %llx.%llx\n", in,
> > ceph_vinop(in));
> > - req->r_target_inode = NULL;
> > if (in->i_state & I_NEW)
> > discard_new_inode(in);
> > else
> > iput(in);
> > goto done;
> > }
> > + req->r_target_inode = in;
> > if (in->i_state & I_NEW)
> > unlock_new_inode(in);
> > }
> > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > index 230e0c3f341f..8b70f2b96f46 100644
> > --- a/fs/ceph/mds_client.c
> > +++ b/fs/ceph/mds_client.c
> > @@ -3874,36 +3874,7 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
> > session->s_con.peer_features);
> > mutex_unlock(&mdsc->mutex);
> >
> > - /* Must find target inode outside of mutexes to avoid deadlocks */
> > rinfo = &req->r_reply_info;
> > - if ((err >= 0) && rinfo->head->is_target) {
> > - struct inode *in = xchg(&req->r_new_inode, NULL);
> > - struct ceph_vino tvino = {
> > - .ino = le64_to_cpu(rinfo->targeti.in->ino),
> > - .snap = le64_to_cpu(rinfo->targeti.in->snapid)
> > - };
> > -
> > - /*
> > - * If we ended up opening an existing inode, discard
> > - * r_new_inode
> > - */
> > - if (req->r_op == CEPH_MDS_OP_CREATE &&
> > - !req->r_reply_info.has_create_ino) {
> > - /* This should never happen on an async create */
> > - WARN_ON_ONCE(req->r_deleg_ino);
> > - iput(in);
> > - in = NULL;
> > - }
> > -
> > - in = ceph_get_inode(mdsc->fsc->sb, tvino, in);
> > - if (IS_ERR(in)) {
> > - err = PTR_ERR(in);
> > - mutex_lock(&session->s_mutex);
> > - goto out_err;
> > - }
> > - req->r_target_inode = in;
> > - }
> > -
> > mutex_lock(&session->s_mutex);
> > if (err < 0) {
> > pr_err_client(cl, "got corrupt reply mds%d(tid:%lld)\n",
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 答复: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
2026-01-07 16:01 ` 答复: " 李磊
@ 2026-01-07 19:59 ` Viacheslav Dubeyko
2026-01-21 7:33 ` 答复: " 李磊
0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2026-01-07 19:59 UTC (permalink / raw)
To: lilei24; +Cc: Alex Markuze, idryomov, sunzhao03, linux-kernel, ceph-devel
On Wed, 2026-01-07 at 16:01 +0000, 李磊 wrote:
> Hi Slava,
>
> This issue is very rare on our internal cephfs clusters. We had only encountered it for about three times.
> But we are working on same hacking methods to speed up the reproduction. I think it will take me one week
> if everything goes smoothly and I will share the methods here.
>
> To be honest, this patch should be a revert patch of this one:
>
> commit : bca9fc14c70fcbbebc84954cc39994e463fb9468
> ceph: when filling trace, call ceph_get_inode outside of mutexes
>
> I'll resend this patch later.
Sounds good. If I remember correctly, the main issue with the initial patch was
the commit message that didn't have good explanation of the issue and why this
revert can fix the issue. So, if we have all of these details in the commit
message, then the patch should be in good shape.
Thanks,
Slava.
^ permalink raw reply [flat|nested] 9+ messages in thread
* 答复: 答复: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
2026-01-07 19:59 ` Viacheslav Dubeyko
@ 2026-01-21 7:33 ` 李磊
2026-01-21 20:24 ` Viacheslav Dubeyko
0 siblings, 1 reply; 9+ messages in thread
From: 李磊 @ 2026-01-21 7:33 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: Alex Markuze, idryomov, 孙朝, linux-kernel, ceph-devel
Hi Slava,
Zhao and I have found a way to reproduce this issue.
1. try to find 2 different directories (DIR_a DIR_b) in a cephfs cluster and make sure they have different auth mds nodes. In this
way, a client may have chances to run handle_reply on different CPU for our test (see step 4 and step 6).
2. In DIR_b, create a hard link of DIR_a/FILE_a, namely FILE_b. DIR_a/FILE_a and DIR_b/FILE_b have the same ino (123456 e.g)
3. Save ino in code below, make it sleep for stat command.
```
@@ -3950,6 +3951,10 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
goto out_err;
}
req->r_target_inode = in;
+ if (in->i_ino == 123456) {
+ pr_err("inode %lu found, ready to wait 10 seconds.\n", in->i_ino);
+ msleep(10000);
+ }
```
4. echo 3 > /proc/sys/vm/drop_caches
5. in a shell, do `stat DIR_a/FILE_a`, we suppose to be stuck on this shell because of msleep() in handle_reply().
6. in the other shell, do `ls DIR_b/` to trigger ceph_readdir_prepopulate()
Repeat step 4 to step 6 for several times (5 times is enough I guess). And we'll see the deadlock.
________________________________________
发件人: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
发送时间: 2026年1月8日 3:59
收件人: 李磊
抄送: Alex Markuze; idryomov@gmail.com; 孙朝; linux-kernel@vger.kernel.org; ceph-devel@vger.kernel.org
主题: Re: 答复: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
On Wed, 2026-01-07 at 16:01 +0000, 李磊 wrote:
> Hi Slava,
>
> This issue is very rare on our internal cephfs clusters. We had only encountered it for about three times.
> But we are working on same hacking methods to speed up the reproduction. I think it will take me one week
> if everything goes smoothly and I will share the methods here.
>
> To be honest, this patch should be a revert patch of this one:
>
> commit : bca9fc14c70fcbbebc84954cc39994e463fb9468
> ceph: when filling trace, call ceph_get_inode outside of mutexes
>
> I'll resend this patch later.
Sounds good. If I remember correctly, the main issue with the initial patch was
the commit message that didn't have good explanation of the issue and why this
revert can fix the issue. So, if we have all of these details in the commit
message, then the patch should be in good shape.
Thanks,
Slava.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 答复: 答复: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
2026-01-21 7:33 ` 答复: " 李磊
@ 2026-01-21 20:24 ` Viacheslav Dubeyko
2026-01-23 3:08 ` 李磊
0 siblings, 1 reply; 9+ messages in thread
From: Viacheslav Dubeyko @ 2026-01-21 20:24 UTC (permalink / raw)
To: lilei24; +Cc: ceph-devel, Alex Markuze, idryomov, linux-kernel, sunzhao03
On Wed, 2026-01-21 at 07:33 +0000, 李磊 wrote:
> Hi Slava,
>
> Zhao and I have found a way to reproduce this issue.
Sounds great!
>
> 1. try to find 2 different directories (DIR_a DIR_b) in a cephfs cluster and make sure they have different auth mds nodes. In this
> way, a client may have chances to run handle_reply on different CPU for our test (see step 4 and step 6).
> 2. In DIR_b, create a hard link of DIR_a/FILE_a, namely FILE_b. DIR_a/FILE_a and DIR_b/FILE_b have the same ino (123456 e.g)
> 3. Save ino in code below, make it sleep for stat command.
> ```
> @@ -3950,6 +3951,10 @@ static void handle_reply(struct ceph_mds_session *session, struct ceph_msg *msg)
> goto out_err;
> }
> req->r_target_inode = in;
> + if (in->i_ino == 123456) {
> + pr_err("inode %lu found, ready to wait 10 seconds.\n", in->i_ino);
> + msleep(10000);
> + }
> ```
> 4. echo 3 > /proc/sys/vm/drop_caches
> 5. in a shell, do `stat DIR_a/FILE_a`, we suppose to be stuck on this shell because of msleep() in handle_reply().
> 6. in the other shell, do `ls DIR_b/` to trigger ceph_readdir_prepopulate()
>
> Repeat step 4 to step 6 for several times (5 times is enough I guess). And we'll see the deadlock.
>
I am guessing... Is it possible to create some Ceph specific test-case in
xfstests suite? It will be great to have some test-case or unit-test for
checking this issue in the future.
OK. I suggest to add this reproduction path and other already shared
explanation/analysis into commit message and re-send the patch. Could you please
send the new version of the patch?
Thanks,
Slava.
>
> ________________________________________
> 发件人: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> 发送时间: 2026年1月8日 3:59
> 收件人: 李磊
> 抄送: Alex Markuze; idryomov@gmail.com; 孙朝; linux-kernel@vger.kernel.org; ceph-devel@vger.kernel.org
> 主题: Re: 答复: 答复: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
>
> On Wed, 2026-01-07 at 16:01 +0000, 李磊 wrote:
> > Hi Slava,
> >
> > This issue is very rare on our internal cephfs clusters. We had only encountered it for about three times.
> > But we are working on same hacking methods to speed up the reproduction. I think it will take me one week
> > if everything goes smoothly and I will share the methods here.
> >
> > To be honest, this patch should be a revert patch of this one:
> >
> > commit : bca9fc14c70fcbbebc84954cc39994e463fb9468
> > ceph: when filling trace, call ceph_get_inode outside of mutexes
> >
> > I'll resend this patch later.
>
> Sounds good. If I remember correctly, the main issue with the initial patch was
> the commit message that didn't have good explanation of the issue and why this
> revert can fix the issue. So, if we have all of these details in the commit
> message, then the patch should be in good shape.
>
> Thanks,
> Slava.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 【外部邮件!】Re: [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate
2026-01-21 20:24 ` Viacheslav Dubeyko
@ 2026-01-23 3:08 ` 李磊
0 siblings, 0 replies; 9+ messages in thread
From: 李磊 @ 2026-01-23 3:08 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: ceph-devel, Alex Markuze, idryomov, linux-kernel, 孙朝
>
> I am guessing... Is it possible to create some Ceph specific test-case in
> xfstests suite? It will be great to have some test-case or unit-test for
> checking this issue in the future.
It makes sense. I’ll try to add this testcase in xfstests.
>
> OK. I suggest to add this reproduction path and other already shared
> explanation/analysis into commit message and re-send the patch. Could you please
> send the new version of the patch?
Got it, I’ll add more explanation of this issue in a new version of the patch, and resend it after
some tests.
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-01-23 3:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-08 7:08 [PATCH v2] ceph: fix deadlock in ceph_readdir_prepopulate Zhao Sun
2025-08-11 19:43 ` Viacheslav Dubeyko
[not found] ` <409ca87f8add4a80a7927981059b3c5f@kuaishou.com>
2026-01-04 9:19 ` 答复: 【外部邮件!】Re: " 李磊
2026-01-05 19:55 ` Viacheslav Dubeyko
2026-01-07 16:01 ` 答复: " 李磊
2026-01-07 19:59 ` Viacheslav Dubeyko
2026-01-21 7:33 ` 答复: " 李磊
2026-01-21 20:24 ` Viacheslav Dubeyko
2026-01-23 3:08 ` 李磊
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®