* [PATCH] blk-cgroup: fix circular locking dependency in blkg_conf_prep
@ 2026-08-11 20:48 Sudarsan Mahendran
2026-08-12 1:28 ` yu kuai
0 siblings, 1 reply; 2+ messages in thread
From: Sudarsan Mahendran @ 2026-08-11 20:48 UTC (permalink / raw)
To: Tejun Heo, Josef Bacik, Jens Axboe
Cc: cgroups, linux-block, linux-kernel, Khazhy Kumykov,
Sudarsan Mahendran
In blkg_conf_prep, we may do IO when allocating while
holding blkcg_mutex, however this presents a deadlock
scenario since we may grab sysfs_lock while IO is
suspended in elevator_exit, and an existing chain
exists from sysfs_mutex -> blkcg_mutex.
We can fix this by performing only GFP_NOIO allocations within
blkg_conf_prep when blkcg_mutex is held.
This patch fixes our reproducer.
Full lockdep splat:
[ 3795.003422] ======================================================
[ 3795.009595] WARNING: possible circular locking dependency detected
[ 3795.015773] 6.18.20-dbg-DEV #1 Tainted: G N
[ 3795.021432] ------------------------------------------------------
[ 3795.027604] kpython3/435194 is trying to acquire lock:
[ 3795.032744] ffffffffa36959b8 (fs_reclaim){+.+.}-{0:0}, at: kmem_cache_alloc_noprof+0x49/0x5c0
[ 3795.041260]
[ 3795.041260] but task is already holding lock:
[ 3795.047086] ffff98c8a06477b0 (&q->blkcg_mutex){+.+.}-{4:4}, at: blkg_conf_prep+0x71/0x5e0
[ 3795.055257]
[ 3795.055257] which lock already depends on the new lock.
[ 3795.055257]
[ 3795.063422]
[ 3795.063422] the existing dependency chain (in reverse order) is:
[ 3795.070893]
[ 3795.070893] -> #4 (&q->blkcg_mutex){+.+.}-{4:4}:
[ 3795.076986] __mutex_lock+0x92/0xb70
[ 3795.081085] blkcg_deactivate_policy+0x7f/0x230
[ 3795.086137] bfq_exit_queue+0xe1/0x110
[ 3795.090401] blk_mq_exit_sched+0xd1/0x170
[ 3795.094935] elevator_exit+0x50/0x70
(...)
[ 3795.135019]
[ 3795.135019] -> #3 (&eq->sysfs_lock){+.+.}-{4:4}:
[ 3795.141110] __mutex_lock+0x92/0xb70
[ 3795.145211] elevator_exit+0x45/0x70
[ 3795.149308] elevator_switch+0x98/0x290
[ 3795.153660] elevator_change+0x109/0x190
[ 3795.158106] elv_iosched_store+0x198/0x240
(...)
[ 3795.185276]
[ 3795.185276] -> #2 (&q->elevator_lock){+.+.}-{4:4}:
[ 3795.191541] __mutex_lock+0x92/0xb70
[ 3795.195642] elevator_change+0xb6/0x190
[ 3795.200001] elv_iosched_store+0x198/0x240
(...)
[ 3795.227179]
[ 3795.227179] -> #1 (&q->q_usage_counter(io)#19){++++}-{0:0}:
[ 3795.234224] blk_alloc_queue+0x33e/0x370
(...)
[ 3795.266621]
[ 3795.266621] -> #0 (fs_reclaim){+.+.}-{0:0}:
[ 3795.272280] __lock_acquire+0x157c/0x2980
[ 3795.276813] lock_acquire+0xd3/0x2a0
[ 3795.280904] fs_reclaim_acquire+0x55/0xc0
[ 3795.285435] kmem_cache_alloc_noprof+0x49/0x5c0
[ 3795.290481] __radix_tree_preload+0xef/0x250
[ 3795.295273] blkg_conf_prep+0x2aa/0x5e0
[ 3795.299633] tg_set_conf+0xa5/0x1a0
(...)
[ 3795.330808]
[ 3795.330808] other info that might help us debug this:
[ 3795.330808]
[ 3795.338806] Chain exists of:
[ 3795.338806] fs_reclaim --> &eq->sysfs_lock --> &q->blkcg_mutex
[ 3795.338806]
[ 3795.349146] Possible unsafe locking scenario:
[ 3795.349146]
[ 3795.355057] CPU0 CPU1
[ 3795.359590] ---- ----
[ 3795.364121] lock(&q->blkcg_mutex);
[ 3795.367691] lock(&eq->sysfs_lock);
[ 3795.373776] lock(&q->blkcg_mutex);
[ 3795.379862] lock(fs_reclaim);
[ 3795.383006]
[ 3795.383006] *** DEADLOCK ***
Reviewed-by: Khazhy Kumykov <khazhy@google.com>
Signed-off-by: Sudarsan Mahendran <sudarsanm@google.com>
---
block/blk-cgroup.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index d9676126c5b5..b981a80bbd94 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -867,7 +867,6 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
parent = blkcg_parent(parent);
}
- /* Drop locks to do new blkg allocation with GFP_KERNEL. */
spin_unlock_irq(&q->queue_lock);
new_blkg = blkg_alloc(pos, disk, GFP_NOIO);
@@ -876,7 +875,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
goto fail_exit;
}
- if (radix_tree_preload(GFP_KERNEL)) {
+ if (radix_tree_preload(GFP_NOIO)) {
blkg_free(new_blkg);
ret = -ENOMEM;
goto fail_exit;
--
2.55.0.679.g6767b8d81c-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] blk-cgroup: fix circular locking dependency in blkg_conf_prep
2026-08-11 20:48 [PATCH] blk-cgroup: fix circular locking dependency in blkg_conf_prep Sudarsan Mahendran
@ 2026-08-12 1:28 ` yu kuai
0 siblings, 0 replies; 2+ messages in thread
From: yu kuai @ 2026-08-12 1:28 UTC (permalink / raw)
To: Sudarsan Mahendran, Tejun Heo, Josef Bacik, Jens Axboe, yu kuai
Cc: cgroups, linux-block, linux-kernel, Khazhy Kumykov
Hi,
在 2026/8/12 4:48, Sudarsan Mahendran 写道:
> In blkg_conf_prep, we may do IO when allocating while
> holding blkcg_mutex, however this presents a deadlock
> scenario since we may grab sysfs_lock while IO is
> suspended in elevator_exit, and an existing chain
> exists from sysfs_mutex -> blkcg_mutex.
>
> We can fix this by performing only GFP_NOIO allocations within
> blkg_conf_prep when blkcg_mutex is held.
> This patch fixes our reproducer.
>
> Full lockdep splat:
>
> [ 3795.003422] ======================================================
> [ 3795.009595] WARNING: possible circular locking dependency detected
> [ 3795.015773] 6.18.20-dbg-DEV #1 Tainted: G N
> [ 3795.021432] ------------------------------------------------------
> [ 3795.027604] kpython3/435194 is trying to acquire lock:
> [ 3795.032744] ffffffffa36959b8 (fs_reclaim){+.+.}-{0:0}, at: kmem_cache_alloc_noprof+0x49/0x5c0
> [ 3795.041260]
> [ 3795.041260] but task is already holding lock:
> [ 3795.047086] ffff98c8a06477b0 (&q->blkcg_mutex){+.+.}-{4:4}, at: blkg_conf_prep+0x71/0x5e0
> [ 3795.055257]
> [ 3795.055257] which lock already depends on the new lock.
> [ 3795.055257]
> [ 3795.063422]
> [ 3795.063422] the existing dependency chain (in reverse order) is:
> [ 3795.070893]
> [ 3795.070893] -> #4 (&q->blkcg_mutex){+.+.}-{4:4}:
> [ 3795.076986] __mutex_lock+0x92/0xb70
> [ 3795.081085] blkcg_deactivate_policy+0x7f/0x230
> [ 3795.086137] bfq_exit_queue+0xe1/0x110
> [ 3795.090401] blk_mq_exit_sched+0xd1/0x170
> [ 3795.094935] elevator_exit+0x50/0x70
> (...)
> [ 3795.135019]
> [ 3795.135019] -> #3 (&eq->sysfs_lock){+.+.}-{4:4}:
> [ 3795.141110] __mutex_lock+0x92/0xb70
> [ 3795.145211] elevator_exit+0x45/0x70
> [ 3795.149308] elevator_switch+0x98/0x290
> [ 3795.153660] elevator_change+0x109/0x190
> [ 3795.158106] elv_iosched_store+0x198/0x240
> (...)
> [ 3795.185276]
> [ 3795.185276] -> #2 (&q->elevator_lock){+.+.}-{4:4}:
> [ 3795.191541] __mutex_lock+0x92/0xb70
> [ 3795.195642] elevator_change+0xb6/0x190
> [ 3795.200001] elv_iosched_store+0x198/0x240
> (...)
> [ 3795.227179]
> [ 3795.227179] -> #1 (&q->q_usage_counter(io)#19){++++}-{0:0}:
> [ 3795.234224] blk_alloc_queue+0x33e/0x370
> (...)
> [ 3795.266621]
> [ 3795.266621] -> #0 (fs_reclaim){+.+.}-{0:0}:
> [ 3795.272280] __lock_acquire+0x157c/0x2980
> [ 3795.276813] lock_acquire+0xd3/0x2a0
> [ 3795.280904] fs_reclaim_acquire+0x55/0xc0
> [ 3795.285435] kmem_cache_alloc_noprof+0x49/0x5c0
> [ 3795.290481] __radix_tree_preload+0xef/0x250
> [ 3795.295273] blkg_conf_prep+0x2aa/0x5e0
> [ 3795.299633] tg_set_conf+0xa5/0x1a0
> (...)
> [ 3795.330808]
> [ 3795.330808] other info that might help us debug this:
> [ 3795.330808]
> [ 3795.338806] Chain exists of:
> [ 3795.338806] fs_reclaim --> &eq->sysfs_lock --> &q->blkcg_mutex
> [ 3795.338806]
> [ 3795.349146] Possible unsafe locking scenario:
> [ 3795.349146]
> [ 3795.355057] CPU0 CPU1
> [ 3795.359590] ---- ----
> [ 3795.364121] lock(&q->blkcg_mutex);
> [ 3795.367691] lock(&eq->sysfs_lock);
> [ 3795.373776] lock(&q->blkcg_mutex);
> [ 3795.379862] lock(fs_reclaim);
> [ 3795.383006]
> [ 3795.383006] *** DEADLOCK ***
>
> Reviewed-by: Khazhy Kumykov <khazhy@google.com>
> Signed-off-by: Sudarsan Mahendran <sudarsanm@google.com>
> ---
> block/blk-cgroup.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index d9676126c5b5..b981a80bbd94 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -867,7 +867,6 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
> parent = blkcg_parent(parent);
> }
>
> - /* Drop locks to do new blkg allocation with GFP_KERNEL. */
> spin_unlock_irq(&q->queue_lock);
>
> new_blkg = blkg_alloc(pos, disk, GFP_NOIO);
> @@ -876,7 +875,7 @@ int blkg_conf_prep(struct blkcg *blkcg, const struct blkcg_policy *pol,
> goto fail_exit;
> }
>
> - if (radix_tree_preload(GFP_KERNEL)) {
> + if (radix_tree_preload(GFP_NOIO)) {
Thanks for the patch, however, there is a pending patchset that this radix_tree_preload will be removed,
I think which is a better solution:
[RFC PATCH v2 2/4] blk-cgroup: use a request_queue rhashtable for blkg
lookup - Yu Kuai <https://lore.kernel.org/all/20260811064744.1139446-3-yukuai@kernel.org/>
> blkg_free(new_blkg);
> ret = -ENOMEM;
> goto fail_exit;
--
Thanks,
Kuai
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-12 1:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11 20:48 [PATCH] blk-cgroup: fix circular locking dependency in blkg_conf_prep Sudarsan Mahendran
2026-08-12 1:28 ` yu kuai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox