Messages in this thread | | | Date | Tue, 26 May 2015 19:38:22 +0200 | From | Oleg Nesterov <> | Subject | Re: [RFC 3/3] memcg: get rid of mm_struct::owner |
| |
On 05/26, Michal Hocko wrote: > > On Tue 26-05-15 18:36:46, Oleg Nesterov wrote: > > > > > +static struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p) > > > +{ > > > + if (!p->mm) > > > + return NULL; > > > + return rcu_dereference(p->mm->memcg); > > > +} > > > > Probably I missed something, but it seems that the callers do not > > expect it can return NULL. > > This hasn't changed by this patch. mem_cgroup_from_task was allowed to > return NULL even before. I've just made it static because it doesn't > have any external users anymore.
I see, but it could only return NULL if mem_cgroup_from_css() returns NULL. Now it returns NULL for sure if the caller is task_in_mem_cgroup(),
// called when task->mm == NULL
task_memcg = mem_cgroup_from_task(task); css_get(&task_memcg->css);
and this css_get() doesn't look nice if task_memcg == NULL ;)
> I will double check
Yes, please. Perhaps I missed something.
> > And in fact I can't understand what mem_cgroup_from_task() actually > > means, with or without these changes. > > It performs task_struct->mem_cgroup mapping. We cannot use cgroup > mapping here because the charges are bound to mm_struct rather than > task.
Sure, this is what I can understand. I meant... OK, lets ignore "without these changes", because without these changes there are much more oddities ;) With these changes only ->mm == NULL case looks unclear.
And btw,
if (!p->mm) return NULL; return rcu_dereference(p->mm->memcg);
perhaps this needs a comment. It is not clear what protects ->mm. But. After this series "p" is always current (if ->mm != NULL), so this is fine.
Nevermind. Please forget. I feel this needs a bit of cleanup, but we can always do this later.
Oleg.
| |