On Tue, 15 Jun 2010 18:59:25 +0900 Minchan Kim <minchan.kim@xxxxxxxxx> wrote: > > -/* > > +/** > > + * find_lock_task_mm - Checking a process which a task belongs to has valid mm > > + * and return a locked task which has a valid pointer to mm. > > + * > > This comment should have been another patch. > BTW, below comment uses "subthread" word. > Personally it's easy to understand function's goal to me. :) > > How about following as? > Checking a process which has any subthread with vaild mm > .... > Sure. thank you. v2 is here. I removed unnecessary parts. == From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> When the OOM killer scans task, it check a task is under memcg or not when it's called via memcg's context. But, as Oleg pointed out, a thread group leader may have NULL ->mm and task_in_mem_cgroup() may do wrong decision. We have to use find_lock_task_mm() in memcg as generic OOM-Killer does. Changelog: - removed unnecessary changes in comments. Cc: Oleg Nesterov <oleg@xxxxxxxxxx> Cc: Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx> Cc: Balbir Singh <balbir@xxxxxxxxxxxxxxxxxx> Reviewed-by: Minchan Kim <minchan.kim@xxxxxxxxx> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> --- include/linux/oom.h | 2 ++ mm/memcontrol.c | 10 +++++++--- mm/oom_kill.c | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) Index: mmotm-2.6.35-0611/include/linux/oom.h =================================================================== --- mmotm-2.6.35-0611.orig/include/linux/oom.h +++ mmotm-2.6.35-0611/include/linux/oom.h @@ -45,6 +45,8 @@ static inline void oom_killer_enable(voi oom_killer_disabled = false; } +extern struct task_struct *find_lock_task_mm(struct task_struct *p); + /* sysctls */ extern int sysctl_oom_dump_tasks; extern int sysctl_oom_kill_allocating_task; Index: mmotm-2.6.35-0611/mm/memcontrol.c =================================================================== --- mmotm-2.6.35-0611.orig/mm/memcontrol.c +++ mmotm-2.6.35-0611/mm/memcontrol.c @@ -47,6 +47,7 @@ #include <linux/mm_inline.h> #include <linux/page_cgroup.h> #include <linux/cpu.h> +#include <linux/oom.h> #include "internal.h" #include <asm/uaccess.h> @@ -838,10 +839,13 @@ int task_in_mem_cgroup(struct task_struc { int ret; struct mem_cgroup *curr = NULL; + struct task_struct *p; - task_lock(task); - curr = try_get_mem_cgroup_from_mm(task->mm); - task_unlock(task); + p = find_lock_task_mm(task); + if (!p) + return 0; + curr = try_get_mem_cgroup_from_mm(p->mm); + task_unlock(p); if (!curr) return 0; /* Index: mmotm-2.6.35-0611/mm/oom_kill.c =================================================================== --- mmotm-2.6.35-0611.orig/mm/oom_kill.c +++ mmotm-2.6.35-0611/mm/oom_kill.c @@ -87,7 +87,7 @@ static bool has_intersects_mems_allowed( * pointer. Return p, or any of its subthreads with a valid ->mm, with * task_lock() held. */ -static struct task_struct *find_lock_task_mm(struct task_struct *p) +struct task_struct *find_lock_task_mm(struct task_struct *p) { struct task_struct *t = p; -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>