cgroup: cgroup->dentry isn't a RCU pointer
authorTejun Heo <tj@kernel.org>
Mon, 19 Nov 2012 16:13:36 +0000 (08:13 -0800)
committerTejun Heo <tj@kernel.org>
Mon, 19 Nov 2012 16:13:36 +0000 (08:13 -0800)
cgroup->dentry is marked and used as a RCU pointer; however, it isn't
one - the final dentry put doesn't go through call_rcu().  cgroup and
dentry share the same RCU freeing rule via synchronize_rcu() in
cgroup_diput() (kfree_rcu() used on cgrp is unnecessary).  If cgrp is
accessible under RCU read lock, so is its dentry and dereferencing
cgrp->dentry doesn't need any further RCU protection or annotation.

While not being accurate, before the previous patch, the RCU accessors
served a purpose as memory barriers - cgroup->dentry used to be
assigned after the cgroup was made visible to cgroup_path(), so the
assignment and dereferencing in cgroup_path() needed the memory
barrier pair.  Now that list_add_tail_rcu() happens after
cgroup->dentry is assigned, this no longer is necessary.

Remove the now unnecessary and misleading RCU annotations from
cgroup->dentry.  To make up for the removal of rcu_dereference_check()
in cgroup_path(), add an explicit rcu_lockdep_assert(), which asserts
the dereference rule of @cgrp, not cgrp->dentry.

Signed-off-by: Tejun Heo <tj@kernel.org>
Acked-by: Li Zefan <lizefan@huawei.com>
include/linux/cgroup.h
kernel/cgroup.c

index 8f64b459fbd42286b515422c1bade921821cbf00..d605857c4bf3795de234adfebc2f67c3cae33b67 100644 (file)
@@ -165,7 +165,7 @@ struct cgroup {
        struct list_head files;         /* my files */
 
        struct cgroup *parent;          /* my parent */
-       struct dentry __rcu *dentry;    /* cgroup fs entry, RCU protected */
+       struct dentry *dentry;          /* cgroup fs entry, RCU protected */
 
        /* Private pointers for each registered subsystem */
        struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT];
index d62a529db2f7f0d30ca26e9587bac81fc24b0659..affc76d7f739aa8c9f3ea3f15452c1164e5ce844 100644 (file)
@@ -1756,9 +1756,11 @@ static struct kobject *cgroup_kobj;
  */
 int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
 {
+       struct dentry *dentry = cgrp->dentry;
        char *start;
-       struct dentry *dentry = rcu_dereference_check(cgrp->dentry,
-                                                     cgroup_lock_is_held());
+
+       rcu_lockdep_assert(rcu_read_lock_held() || cgroup_lock_is_held(),
+                          "cgroup_path() called without proper locking");
 
        if (!dentry || cgrp == dummytop) {
                /*
@@ -1782,8 +1784,7 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
                if (!cgrp)
                        break;
 
-               dentry = rcu_dereference_check(cgrp->dentry,
-                                              cgroup_lock_is_held());
+               dentry = cgrp->dentry;
                if (!cgrp->parent)
                        continue;
                if (--start < buf)
@@ -4124,7 +4125,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
 
        /* allocation complete, commit to creation */
        dentry->d_fsdata = cgrp;
-       rcu_assign_pointer(cgrp->dentry, dentry);
+       cgrp->dentry = dentry;
        list_add_tail(&cgrp->allcg_node, &root->allcg_list);
        list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
        root->number_of_cgroups++;