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 Signed-off-by: Sudarsan Mahendran --- 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