On Wed 13-07-16 00:42:51, Tetsuo Handa wrote: > Michal Hocko wrote: > > On Tue 12-07-16 22:29:20, Tetsuo Handa wrote: [...] > > > Since later patch in the series will change oom_scan_process_thread() not > > > to depend on atomic_read(&task->signal->oom_victims) != 0 && > > > find_lock_task_mm(task) != NULL, this patch removes exit_oom_victim() > > > on remote thread. > > > > I have already suggested doing this in a separate patch. Because > > dropping exit_oom_victim has other side effectes (namely for > > oom_killer_disable convergence guarantee). > > You can apply > http://lkml.kernel.org/r/1467365190-24640-3-git-send-email-mhocko@xxxxxxxxxx > at this point. I would still prefer if exit_oom_victim was done in a separate patch. Unless you have a strong reason to do it in this patch. I plan to rework the above to remove exit_oom_victim along with the oom_killer_disable change. > > Also I would suggest doing set_bit(MMF_OOM_REAPED) from exit_oom_mm and > > (in a follow up patch) rename it to MMF_SKIP_OOM_MM. > > > > I haven't spotted any other issues. > > > Oops. Please fold below fix into > "[PATCH 5/8] mm,oom_reaper: Make OOM reaper use list of mm_struct.". > > >From ae051fb92b285c0dc4ebc4953fadc755b1ae8a31 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> > Date: Wed, 13 Jul 2016 00:24:32 +0900 > Subject: [PATCH] mm,oom_reaper: Close race on exit_oom_mm(). > > Previous patch forgot to take a reference on mm, for __mmput() from > mmput() from exit_mm() can drop mm->mm_count till 0 before the OOM > reaper calls exit_oom_mm(). I have missed that as well. Please fold this in. > > Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> > --- > mm/oom_kill.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index 715f77d..4c8b686 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -626,21 +626,24 @@ static int oom_reaper(void *unused) > if (!list_empty(&oom_mm_list)) { > mm = list_first_entry(&oom_mm_list, struct mm_struct, > oom_mm.list); > - victim = mm->oom_mm.victim; > /* > - * Take a reference on current victim thread in case > - * oom_reap_task() raced with mark_oom_victim() by > - * other threads sharing this mm. > + * Take references on mm and victim in case > + * oom_reap_task() raced with mark_oom_victim() or > + * __mmput(). > */ > + atomic_inc(&mm->mm_count); > + victim = mm->oom_mm.victim; > get_task_struct(victim); > } > spin_unlock(&oom_mm_lock); > if (!mm) > continue; > oom_reap_task(victim, mm); > - put_task_struct(victim); > /* Drop references taken by mark_oom_victim() */ > exit_oom_mm(mm); > + /* Drop references taken above. */ > + put_task_struct(victim); > + mmdrop(mm); > } > > return 0; > -- > 1.8.3.1 > > -- > 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> -- Michal Hocko SUSE Labs -- 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>