oom: make oom_unkillable_task() helper function
in [Kernel]
|
Prev: oom: oom_kill_process() need to check p is unkillable
Next: oom: give the dying task a higher priority
From: Minchan Kim on 16 Jun 2010 11:20 On Wed, Jun 16, 2010 at 08:33:17PM +0900, KOSAKI Motohiro wrote: > > Now, we have the same task check in three places. Unify it. > > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com> But please consider my previous comment. If I am not wrong, we need change this patch, too. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: David Rientjes on 21 Jun 2010 16:20 On Thu, 17 Jun 2010, KOSAKI Motohiro wrote: > > Now, we have the same task check in two places. Unify it. > We should exclude tasks from select_bad_process() and oom_kill_process() by having badness() return a score of 0, just like it's done for OOM_DISABLE. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: KOSAKI Motohiro on 30 Jun 2010 05:30 > On Thu, 17 Jun 2010, KOSAKI Motohiro wrote: > > > > > Now, we have the same task check in two places. Unify it. > > > > We should exclude tasks from select_bad_process() and oom_kill_process() > by having badness() return a score of 0, just like it's done for > OOM_DISABLE. No. again, select_bad_process() have meaningful check order. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Minchan Kim on 30 Jun 2010 10:30 On Wed, Jun 30, 2010 at 06:28:37PM +0900, KOSAKI Motohiro wrote: > Now, we have the same task check in two places. Unify it. > > Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com> > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> > --- > mm/oom_kill.c | 33 ++++++++++++++++++++++----------- > 1 files changed, 22 insertions(+), 11 deletions(-) > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index dc8589e..a4a5439 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -101,6 +101,26 @@ static struct task_struct *find_lock_task_mm(struct task_struct *p) > return NULL; > } > > +/* return true if the task is not adequate as candidate victim task. */ > +static bool oom_unkillable_task(struct task_struct *p, struct mem_cgroup *mem, > + const nodemask_t *nodemask) > +{ > + if (is_global_init(p)) > + return true; > + if (p->flags & PF_KTHREAD) > + return true; > + > + /* When mem_cgroup_out_of_memory() and p is not member of the group */ > + if (mem && !task_in_mem_cgroup(p, mem)) > + return true; > + > + /* p may not have freeable memory in nodemask */ > + if (!has_intersects_mems_allowed(p, nodemask)) > + return true; > + > + return false; > +} > + I returend this patch as review 7/11. Why didn't you check p->signal->oom_adj == OOM_DISABLE in here? I don't figure out code after your patches are applied totally. But I think it would be check it in this function as function's name says. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: KOSAKI Motohiro on 30 Jun 2010 20:10
> On Wed, Jun 30, 2010 at 06:28:37PM +0900, KOSAKI Motohiro wrote: > > Now, we have the same task check in two places. Unify it. > > > > Reviewed-by: Minchan Kim <minchan.kim(a)gmail.com> > > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro(a)jp.fujitsu.com> > > --- > > mm/oom_kill.c | 33 ++++++++++++++++++++++----------- > > 1 files changed, 22 insertions(+), 11 deletions(-) > > > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > > index dc8589e..a4a5439 100644 > > --- a/mm/oom_kill.c > > +++ b/mm/oom_kill.c > > @@ -101,6 +101,26 @@ static struct task_struct *find_lock_task_mm(struct task_struct *p) > > return NULL; > > } > > > > +/* return true if the task is not adequate as candidate victim task. */ > > +static bool oom_unkillable_task(struct task_struct *p, struct mem_cgroup *mem, > > + const nodemask_t *nodemask) > > +{ > > + if (is_global_init(p)) > > + return true; > > + if (p->flags & PF_KTHREAD) > > + return true; > > + > > + /* When mem_cgroup_out_of_memory() and p is not member of the group */ > > + if (mem && !task_in_mem_cgroup(p, mem)) > > + return true; > > + > > + /* p may not have freeable memory in nodemask */ > > + if (!has_intersects_mems_allowed(p, nodemask)) > > + return true; > > + > > + return false; > > +} > > + > > I returend this patch as review 7/11. > Why didn't you check p->signal->oom_adj == OOM_DISABLE in here? > I don't figure out code after your patches are applied totally. > But I think it would be check it in this function as function's name says. For preserve select_bad_process() semantics. It have for_each_process(p) { if (oom_unkillable_task(p, mem, nodemask)) continue; if (thread_group_empty(p) && (p->flags & PF_EXITING) && p->mm) { if (p != current) return ERR_PTR(-1UL); chosen = p; *ppoints = ULONG_MAX; } if (oom_adj == OOM_DISABLE) continue; That said, Current OOM-Killer intend to kill PF_EXITING process even if it have OOM_DISABLE. (practically, it's not kill. it only affect to give allocation bonus to PF_EXITING process) My trivial fixes series don't intend to make large semantics change. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |