diff options
author | Linus Torvalds <torvalds@linux-foundation.org> | 2012-07-08 12:09:27 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2012-07-08 12:09:27 -0400 |
commit | 8c84bf4166a4698296342841a549bbee03860ac0 (patch) | |
tree | 1f02ce8cea5df3fcf6c332ab08fe0ed21a542612 | |
parent | bd0a521e88aa7a06ae7aabaed7ae196ed4ad867a (diff) | |
parent | 5db9a4d99b0157a513944e9a44d29c9cec2e91dc (diff) |
Merge branch 'for-3.5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup
Pull cgroup fixes from Tejun Heo:
"The previous cgroup pull request contained a patch to fix a race
condition during cgroup hierarchy umount. Unfortunately, while the
patch reduced the race window such that the test case I and Sasha were
using didn't trigger it anymore, it wasn't complete - Shyju and Li
could reliably trigger the race condition using a different test case.
The problem wasn't the gap between dentry deletion and release which
the previous patch tried to fix. The window was between the last
dput() of a root's child and the resulting dput() of the root. For
cgroup dentries, the deletion and release always happen synchronously.
As this releases the s_active ref, the refcnt of the root dentry,
which doesn't hold s_active, stays above zero without the
corresponding s_active. If umount was in progress, the last
deactivate_super() proceeds to destory the superblock and triggers
BUG() on the non-zero root dentry refcnt after shrinking.
This issue surfaced because cgroup dentries are now allowed to linger
after rmdir(2) since 3.5-rc1. Before, rmdir synchronously drained the
dentry refcnt and the s_active acquired by rmdir from vfs layer
protected the whole thing. After 3.5-rc1, cgroup may internally hold
and put dentry refs after rmdir finishes and the delayed dput()
doesn't have surrounding s_active ref exposing this issue.
This pull request contains two patches - one reverting the previous
incorrect fix and the other adding the surrounding s_active ref around
the delayed dput().
This is quite late in the release cycle but the change is on the safer
side and fixes the test cases reliably, so I don't think it's too
crazy."
* 'for-3.5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup:
cgroup: fix cgroup hierarchy umount race
Revert "cgroup: superblock can't be released with active dentries"
-rw-r--r-- | kernel/cgroup.c | 23 |
1 files changed, 8 insertions, 15 deletions
diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 2097684cf194..b303dfc7dce0 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c | |||
@@ -901,13 +901,10 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode) | |||
901 | mutex_unlock(&cgroup_mutex); | 901 | mutex_unlock(&cgroup_mutex); |
902 | 902 | ||
903 | /* | 903 | /* |
904 | * We want to drop the active superblock reference from the | 904 | * Drop the active superblock reference that we took when we |
905 | * cgroup creation after all the dentry refs are gone - | 905 | * created the cgroup |
906 | * kill_sb gets mighty unhappy otherwise. Mark | ||
907 | * dentry->d_fsdata with cgroup_diput() to tell | ||
908 | * cgroup_d_release() to call deactivate_super(). | ||
909 | */ | 906 | */ |
910 | dentry->d_fsdata = cgroup_diput; | 907 | deactivate_super(cgrp->root->sb); |
911 | 908 | ||
912 | /* | 909 | /* |
913 | * if we're getting rid of the cgroup, refcount should ensure | 910 | * if we're getting rid of the cgroup, refcount should ensure |
@@ -933,13 +930,6 @@ static int cgroup_delete(const struct dentry *d) | |||
933 | return 1; | 930 | return 1; |
934 | } | 931 | } |
935 | 932 | ||
936 | static void cgroup_d_release(struct dentry *dentry) | ||
937 | { | ||
938 | /* did cgroup_diput() tell me to deactivate super? */ | ||
939 | if (dentry->d_fsdata == cgroup_diput) | ||
940 | deactivate_super(dentry->d_sb); | ||
941 | } | ||
942 | |||
943 | static void remove_dir(struct dentry *d) | 933 | static void remove_dir(struct dentry *d) |
944 | { | 934 | { |
945 | struct dentry *parent = dget(d->d_parent); | 935 | struct dentry *parent = dget(d->d_parent); |
@@ -1547,7 +1537,6 @@ static int cgroup_get_rootdir(struct super_block *sb) | |||
1547 | static const struct dentry_operations cgroup_dops = { | 1537 | static const struct dentry_operations cgroup_dops = { |
1548 | .d_iput = cgroup_diput, | 1538 | .d_iput = cgroup_diput, |
1549 | .d_delete = cgroup_delete, | 1539 | .d_delete = cgroup_delete, |
1550 | .d_release = cgroup_d_release, | ||
1551 | }; | 1540 | }; |
1552 | 1541 | ||
1553 | struct inode *inode = | 1542 | struct inode *inode = |
@@ -3894,8 +3883,12 @@ static void css_dput_fn(struct work_struct *work) | |||
3894 | { | 3883 | { |
3895 | struct cgroup_subsys_state *css = | 3884 | struct cgroup_subsys_state *css = |
3896 | container_of(work, struct cgroup_subsys_state, dput_work); | 3885 | container_of(work, struct cgroup_subsys_state, dput_work); |
3886 | struct dentry *dentry = css->cgroup->dentry; | ||
3887 | struct super_block *sb = dentry->d_sb; | ||
3897 | 3888 | ||
3898 | dput(css->cgroup->dentry); | 3889 | atomic_inc(&sb->s_active); |
3890 | dput(dentry); | ||
3891 | deactivate_super(sb); | ||
3899 | } | 3892 | } |
3900 | 3893 | ||
3901 | static void init_cgroup_css(struct cgroup_subsys_state *css, | 3894 | static void init_cgroup_css(struct cgroup_subsys_state *css, |