Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757640Ab0FPAIJ (ORCPT ); Tue, 15 Jun 2010 20:08:09 -0400 Received: from fgwmail6.fujitsu.co.jp ([192.51.44.36]:46685 "EHLO fgwmail6.fujitsu.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752888Ab0FPAIH (ORCPT ); Tue, 15 Jun 2010 20:08:07 -0400 X-SecurityPolicyCheck-FJ: OK by FujitsuOutboundMailChecker v1.3.1 Date: Wed, 16 Jun 2010 09:03:34 +0900 From: KAMEZAWA Hiroyuki To: Minchan Kim Cc: "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "nishimura@mxp.nes.nec.co.jp" , "balbir@linux.vnet.ibm.com" , Oleg Nesterov , "akpm@linux-foundation.org" Subject: [PATCH] use find_lock_task_mm in memory cgroups oom v2 Message-Id: <20100616090334.d27e0c4e.kamezawa.hiroyu@jp.fujitsu.com> In-Reply-To: References: <20100615152450.f82c1f8c.kamezawa.hiroyu@jp.fujitsu.com> Organization: FUJITSU Co. LTD. X-Mailer: Sylpheed 3.0.2 (GTK+ 2.10.14; i686-pc-mingw32) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3348 Lines: 106 On Tue, 15 Jun 2010 18:59:25 +0900 Minchan Kim 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 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 Cc: Daisuke Nishimura Cc: Balbir Singh Reviewed-by: Minchan Kim Signed-off-by: KAMEZAWA Hiroyuki --- 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 #include #include +#include #include "internal.h" #include @@ -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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/