[PATCH] blk-cgroup: fix circular locking dependency in blkg_conf_prep

Sudarsan Mahendran <[email protected]>
Newsgroups org.kernel.vger.cgroups,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
Signed-off-by: Sudarsan Mahendran <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.