Re: [PATCH 1/2] cgroup: helper do determine group name
From: Michal Hocko <hidden>
Date: 2012-11-23 08:55:38
On Thu 22-11-12 14:29:49, Glauber Costa wrote:
With more than one user, it is useful to have a helper function in the cgroup core to derive a group's name. We'll just return a pointer, and it is not expected to get incredibly complicated. But it is useful to have it so we can abstract away the vfs relation from its users. Signed-off-by: Glauber Costa <redacted> CC: Tejun Heo <tj@kernel.org> CC: Michal Hocko <redacted> CC: Kamezawa Hiroyuki <redacted> CC: Johannes Weiner <hannes@cmpxchg.org>
Looks good to me in general. Minor comments bellow. Anyway. Acked-by: Michal Hocko <redacted>
--- Tejun: I know the rcu is no longer necessary. I am using mhocko's tree, that doesn't seem to have your last stream of patches yet.
Which patches are we talking about? Are they in a pullable (for -mm) branch or I have to cherry-pick them?
quoted hunk ↗ jump to hunk
If you approve the interface, we'll need a follow up on this to remove the rcu dereference of the dentry. include/linux/cgroup.h | 1 + kernel/cgroup.c | 9 +++++++++ mm/memcontrol.c | 11 ++++------- 3 files changed, 14 insertions(+), 7 deletions(-)diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index a178a91..57c4ab1 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h@@ -401,6 +401,7 @@ int cgroup_rm_cftypes(struct cgroup_subsys *ss, const struct cftype *cfts); int cgroup_is_removed(const struct cgroup *cgrp); int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen); +extern const char *cgroup_name(const struct cgroup *cgrp); int cgroup_task_count(const struct cgroup *cgrp);diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 3d68aad..d0d291e 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c@@ -1757,6 +1757,15 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen) } EXPORT_SYMBOL_GPL(cgroup_path);
This expects css reference at caller, right. Please make it explicit
here in case somebody wants to use this somewhere else.
Besides that rcu_read_{un}lock are not necessary if you keep the
reference, right? The last dput happens only after the last css_put.
quoted hunk ↗ jump to hunk
+const char *cgroup_name(const struct cgroup *cgrp) +{ + struct dentry *dentry; + rcu_read_lock(); + dentry = rcu_dereference_check(cgrp->dentry, cgroup_lock_is_held()); + rcu_read_unlock(); + return dentry->d_name.name; +} + /* * Control Group taskset */diff --git a/mm/memcontrol.c b/mm/memcontrol.c index e3d805f..05b87aa 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c@@ -3141,16 +3141,13 @@ void mem_cgroup_destroy_cache(struct kmem_cache *cachep) static char *memcg_cache_name(struct mem_cgroup *memcg, struct kmem_cache *s) { char *name; - struct dentry *dentry; + const char *cgname; - rcu_read_lock(); - dentry = rcu_dereference(memcg->css.cgroup->dentry); - rcu_read_unlock(); - - BUG_ON(dentry == NULL); + cgname = cgroup_name(memcg->css.cgroup); + BUG_ON(cgname == NULL); name = kasprintf(GFP_KERNEL, "%s(%d:%s)", s->name, - memcg_cache_id(memcg), dentry->d_name.name); + memcg_cache_id(memcg), cgname); return name; }-- 1.7.11.7 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
-- Michal Hocko SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>