Commit febfcef6 authored by Tejun Heo's avatar Tejun Heo

cgroup: cgroup->dentry isn't a RCU pointer

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: default avatarTejun Heo <tj@kernel.org>
Acked-by: default avatarLi Zefan <lizefan@huawei.com>
parent 4e139afc
...@@ -165,7 +165,7 @@ struct cgroup { ...@@ -165,7 +165,7 @@ struct cgroup {
struct list_head files; /* my files */ struct list_head files; /* my files */
struct cgroup *parent; /* my parent */ 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 */ /* Private pointers for each registered subsystem */
struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT]; struct cgroup_subsys_state *subsys[CGROUP_SUBSYS_COUNT];
......
...@@ -1756,9 +1756,11 @@ static struct kobject *cgroup_kobj; ...@@ -1756,9 +1756,11 @@ static struct kobject *cgroup_kobj;
*/ */
int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen) int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
{ {
struct dentry *dentry = cgrp->dentry;
char *start; 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) { if (!dentry || cgrp == dummytop) {
/* /*
...@@ -1782,8 +1784,7 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen) ...@@ -1782,8 +1784,7 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen)
if (!cgrp) if (!cgrp)
break; break;
dentry = rcu_dereference_check(cgrp->dentry, dentry = cgrp->dentry;
cgroup_lock_is_held());
if (!cgrp->parent) if (!cgrp->parent)
continue; continue;
if (--start < buf) if (--start < buf)
...@@ -4124,7 +4125,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry, ...@@ -4124,7 +4125,7 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
/* allocation complete, commit to creation */ /* allocation complete, commit to creation */
dentry->d_fsdata = cgrp; 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(&cgrp->allcg_node, &root->allcg_list);
list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children); list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children);
root->number_of_cgroups++; root->number_of_cgroups++;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment