diff options
author | Tejun Heo <tj@kernel.org> | 2013-06-27 22:37:23 -0400 |
---|---|---|
committer | Tejun Heo <tj@kernel.org> | 2013-06-27 22:37:23 -0400 |
commit | e2bd416f6246d11be29999c177d2534943a5c2df (patch) | |
tree | 3c100724ac876ac84c843fb3bfe9c494c138b8b0 /kernel | |
parent | a4ea1cc90604df08d471ae84eb9627319d10c844 (diff) |
cgroup: fix deadlock on cgroup_mutex via drop_parsed_module_refcounts()
eb178d06332 ("cgroup: grab cgroup_mutex in
drop_parsed_module_refcounts()") made drop_parsed_module_refcounts()
grab cgroup_mutex to make lockdep assertion in for_each_subsys()
happy. Unfortunately, cgroup_remount() calls the function while
holding cgroup_mutex in its failure path leading to the following
deadlock.
# mount -t cgroup -o remount,memory,blkio cgroup blkio
cgroup: option changes via remount are deprecated (pid=525 comm=mount)
=============================================
[ INFO: possible recursive locking detected ]
3.10.0-rc4-work+ #1 Not tainted
---------------------------------------------
mount/525 is trying to acquire lock:
(cgroup_mutex){+.+.+.}, at: [<ffffffff8110a3e1>] drop_parsed_module_refcounts+0x21/0xb0
but task is already holding lock:
(cgroup_mutex){+.+.+.}, at: [<ffffffff8110e4e1>] cgroup_remount+0x51/0x200
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(cgroup_mutex);
lock(cgroup_mutex);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by mount/525:
#0: (&type->s_umount_key#30){+.+...}, at: [<ffffffff811e9a0d>] do_mount+0x2bd/0xa30
#1: (&sb->s_type->i_mutex_key#9){+.+.+.}, at: [<ffffffff8110e4d3>] cgroup_remount+0x43/0x200
#2: (cgroup_mutex){+.+.+.}, at: [<ffffffff8110e4e1>] cgroup_remount+0x51/0x200
#3: (cgroup_root_mutex){+.+.+.}, at: [<ffffffff8110e4ef>] cgroup_remount+0x5f/0x200
stack backtrace:
CPU: 2 PID: 525 Comm: mount Not tainted 3.10.0-rc4-work+ #1
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
ffffffff829651f0 ffff88000ec2fc28 ffffffff81c24bb1 ffff88000ec2fce8
ffffffff810f420d 0000000000000006 0000000000000001 0000000000000056
ffff8800153b4640 ffff880000000000 ffffffff81c2e468 ffff8800153b4640
Call Trace:
[<ffffffff81c24bb1>] dump_stack+0x19/0x1b
[<ffffffff810f420d>] __lock_acquire+0x15dd/0x1e60
[<ffffffff810f531c>] lock_acquire+0x9c/0x1f0
[<ffffffff81c2a805>] mutex_lock_nested+0x65/0x410
[<ffffffff8110a3e1>] drop_parsed_module_refcounts+0x21/0xb0
[<ffffffff8110e63e>] cgroup_remount+0x1ae/0x200
[<ffffffff811c9bb2>] do_remount_sb+0x82/0x190
[<ffffffff811e9d41>] do_mount+0x5f1/0xa30
[<ffffffff811ea203>] SyS_mount+0x83/0xc0
[<ffffffff81c2fb82>] system_call_fastpath+0x16/0x1b
Fix it by moving the drop_parsed_module_refcounts() invocation outside
cgroup_mutex.
Signed-off-by: Tejun Heo <tj@kernel.org>
Diffstat (limited to 'kernel')
-rw-r--r-- | kernel/cgroup.c | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 4ed86773fff7..1b7b567208cd 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c | |||
@@ -1365,7 +1365,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data) | |||
1365 | if (opts.flags != root->flags || | 1365 | if (opts.flags != root->flags || |
1366 | (opts.name && strcmp(opts.name, root->name))) { | 1366 | (opts.name && strcmp(opts.name, root->name))) { |
1367 | ret = -EINVAL; | 1367 | ret = -EINVAL; |
1368 | drop_parsed_module_refcounts(opts.subsys_mask); | ||
1369 | goto out_unlock; | 1368 | goto out_unlock; |
1370 | } | 1369 | } |
1371 | 1370 | ||
@@ -1380,7 +1379,6 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data) | |||
1380 | if (ret) { | 1379 | if (ret) { |
1381 | /* rebind_subsystems failed, re-populate the removed files */ | 1380 | /* rebind_subsystems failed, re-populate the removed files */ |
1382 | cgroup_populate_dir(cgrp, false, removed_mask); | 1381 | cgroup_populate_dir(cgrp, false, removed_mask); |
1383 | drop_parsed_module_refcounts(opts.subsys_mask); | ||
1384 | goto out_unlock; | 1382 | goto out_unlock; |
1385 | } | 1383 | } |
1386 | 1384 | ||
@@ -1395,6 +1393,8 @@ static int cgroup_remount(struct super_block *sb, int *flags, char *data) | |||
1395 | mutex_unlock(&cgroup_root_mutex); | 1393 | mutex_unlock(&cgroup_root_mutex); |
1396 | mutex_unlock(&cgroup_mutex); | 1394 | mutex_unlock(&cgroup_mutex); |
1397 | mutex_unlock(&cgrp->dentry->d_inode->i_mutex); | 1395 | mutex_unlock(&cgrp->dentry->d_inode->i_mutex); |
1396 | if (ret) | ||
1397 | drop_parsed_module_refcounts(opts.subsys_mask); | ||
1398 | return ret; | 1398 | return ret; |
1399 | } | 1399 | } |
1400 | 1400 | ||