Messages in this thread | | | Date | Tue, 28 Feb 2012 10:31:51 +0400 | From | Konstantin Khlebnikov <> | Subject | Re: [PATCH v3 02/21] memcg: make mm_match_cgroup() hirarchical |
| |
KAMEZAWA Hiroyuki wrote: > On Thu, 23 Feb 2012 17:51:46 +0400 > Konstantin Khlebnikov<khlebnikov@openvz.org> wrote: > >> Check mm-owner cgroup membership hierarchically. >> >> Signed-off-by: Konstantin Khlebnikov<khlebnikov@openvz.org> > > > Ack. but ... see below. > >> --- >> include/linux/memcontrol.h | 11 ++--------- >> mm/memcontrol.c | 20 ++++++++++++++++++++ >> 2 files changed, 22 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h >> index 8c4d74f..4822d53 100644 >> --- a/include/linux/memcontrol.h >> +++ b/include/linux/memcontrol.h >> @@ -87,15 +87,8 @@ extern struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm); >> extern struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *memcg); >> extern struct mem_cgroup *mem_cgroup_from_cont(struct cgroup *cont); >> >> -static inline >> -int mm_match_cgroup(const struct mm_struct *mm, const struct mem_cgroup *cgroup) >> -{ >> - struct mem_cgroup *memcg; >> - rcu_read_lock(); >> - memcg = mem_cgroup_from_task(rcu_dereference((mm)->owner)); >> - rcu_read_unlock(); >> - return cgroup == memcg; >> -} >> +extern int mm_match_cgroup(const struct mm_struct *mm, >> + const struct mem_cgroup *cgroup); >> >> extern struct cgroup_subsys_state *mem_cgroup_css(struct mem_cgroup *memcg); >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index b8039d2..77f5d48 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -821,6 +821,26 @@ struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p) >> struct mem_cgroup, css); >> } >> >> +/** >> + * mm_match_cgroup - cgroup hierarchy mm membership test >> + * @mm mm_struct to test >> + * @cgroup target cgroup >> + * >> + * Returns true if mm belong this cgroup or any its child in hierarchy > > belongs to ? > >> + */ >> +int mm_match_cgroup(const struct mm_struct *mm, const struct mem_cgroup *cgroup) >> +{ > > Please use "memcg" for representing "memory cgroup" (other function's arguments uses "memcg") > >> + struct mem_cgroup *memcg; > > So, rename this as *cur_memcg or some. > >> + >> + rcu_read_lock(); >> + memcg = mem_cgroup_from_task(rcu_dereference((mm)->owner)); >> + while (memcg != cgroup&& memcg&& memcg->use_hierarchy) >> + memcg = parent_mem_cgroup(memcg); > > IIUC, parent_mem_cgroup() checks mem->res.parent. mem->res.parent is set only when > parent->use_hierarchy == true. Then, > > while (memcg != cgroup) > memcg = parent_mem_cgroup(memcg); > > will be enough.
Here will be mem_cgroup_same_or_subtree(), see reply from Johannes Weiner.
> > Thanks, > -Kame >
| |