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. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>