From bcf4dd43424cdfd8195f3955300a579fe58e9911 Mon Sep 17 00:00:00 2001 From: Gui Jianfeng Date: Mon, 1 Feb 2010 09:58:54 +0100 Subject: [PATCH] blk-cgroup: Fix potential deadlock in blk-cgroup I triggered a lockdep warning as following. ======================================================= [ INFO: possible circular locking dependency detected ] 2.6.33-rc2 #1 ------------------------------------------------------- test_io_control/7357 is trying to acquire lock: (blkio_list_lock){+.+...}, at: [] blkiocg_weight_write+0x82/0x9e but task is already holding lock: (&(&blkcg->lock)->rlock){......}, at: [] blkiocg_weight_write+0x3b/0x9e which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (&(&blkcg->lock)->rlock){......}: [] validate_chain+0x8bc/0xb9c [] __lock_acquire+0x723/0x789 [] lock_acquire+0x90/0xa7 [] _raw_spin_lock_irqsave+0x27/0x5a [] blkiocg_add_blkio_group+0x1a/0x6d [] cfq_get_queue+0x225/0x3de [] cfq_set_request+0x217/0x42d [] elv_set_request+0x17/0x26 [] get_request+0x203/0x2c5 [] get_request_wait+0x18/0x10e [] __make_request+0x2ba/0x375 [] generic_make_request+0x28d/0x30f [] submit_bio+0x8a/0x8f [] submit_bh+0xf0/0x10f [] ll_rw_block+0xc0/0xf9 [] ext3_find_entry+0x319/0x544 [ext3] [] ext3_lookup+0x2c/0xb9 [ext3] [] do_lookup+0xd3/0x172 [] link_path_walk+0x5fb/0x95c [] path_walk+0x3c/0x81 [] do_path_lookup+0x21/0x8a [] do_filp_open+0xf0/0x978 [] open_exec+0x1b/0xb7 [] do_execve+0xbb/0x266 [] sys_execve+0x24/0x4a [] ptregs_execve+0x12/0x18 -> #1 (&(&q->__queue_lock)->rlock){..-.-.}: [] validate_chain+0x8bc/0xb9c [] __lock_acquire+0x723/0x789 [] lock_acquire+0x90/0xa7 [] _raw_spin_lock_irqsave+0x27/0x5a [] cfq_unlink_blkio_group+0x17/0x41 [] blkiocg_destroy+0x72/0xc7 [] cgroup_diput+0x4a/0xb2 [] dentry_iput+0x93/0xb7 [] d_kill+0x1c/0x36 [] dput+0xf5/0xfe [] do_rmdir+0x95/0xbe [] sys_rmdir+0x10/0x12 [] sysenter_do_call+0x12/0x32 -> #0 (blkio_list_lock){+.+...}: [] validate_chain+0x61c/0xb9c [] __lock_acquire+0x723/0x789 [] lock_acquire+0x90/0xa7 [] _raw_spin_lock+0x1e/0x4e [] blkiocg_weight_write+0x82/0x9e [] cgroup_file_write+0xc6/0x1c0 [] vfs_write+0x8c/0x116 [] sys_write+0x3b/0x60 [] sysenter_do_call+0x12/0x32 other info that might help us debug this: 1 lock held by test_io_control/7357: #0: (&(&blkcg->lock)->rlock){......}, at: [] blkiocg_weight_write+0x3b/0x9e stack backtrace: Pid: 7357, comm: test_io_control Not tainted 2.6.33-rc2 #1 Call Trace: [] print_circular_bug+0x91/0x9d [] validate_chain+0x61c/0xb9c [] __lock_acquire+0x723/0x789 [] lock_acquire+0x90/0xa7 [] ? blkiocg_weight_write+0x82/0x9e [] _raw_spin_lock+0x1e/0x4e [] ? blkiocg_weight_write+0x82/0x9e [] blkiocg_weight_write+0x82/0x9e [] cgroup_file_write+0xc6/0x1c0 [] ? trace_hardirqs_off+0xb/0xd [] ? cpu_clock+0x2e/0x44 [] ? security_file_permission+0xf/0x11 [] ? rw_verify_area+0x8a/0xad [] ? cgroup_file_write+0x0/0x1c0 [] vfs_write+0x8c/0x116 [] sys_write+0x3b/0x60 [] sysenter_do_call+0x12/0x32 To prevent deadlock, we should take locks as following sequence: blkio_list_lock -> queue_lock -> blkcg_lock. The following patch should fix this bug. Signed-off-by: Gui Jianfeng Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 1fa2654db0a6..e7dbbaf5fb3e 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -147,16 +147,16 @@ blkiocg_weight_write(struct cgroup *cgroup, struct cftype *cftype, u64 val) return -EINVAL; blkcg = cgroup_to_blkio_cgroup(cgroup); + spin_lock(&blkio_list_lock); spin_lock_irq(&blkcg->lock); blkcg->weight = (unsigned int)val; hlist_for_each_entry(blkg, n, &blkcg->blkg_list, blkcg_node) { - spin_lock(&blkio_list_lock); list_for_each_entry(blkiop, &blkio_list, list) blkiop->ops.blkio_update_group_weight_fn(blkg, blkcg->weight); - spin_unlock(&blkio_list_lock); } spin_unlock_irq(&blkcg->lock); + spin_unlock(&blkio_list_lock); return 0; } -- 2.20.1