Re: [PATCH] oom: consider multi-threaded tasks in task_will_free_mem

From: Michal Hocko
Date: Tue May 17 2016 - 16:29:03 EST


On Tue 26-04-16 15:57:52, Michal Hocko wrote:
> On Tue 12-04-16 11:19:16, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@xxxxxxxx>
> >
> > task_will_free_mem is a misnomer for a more complex PF_EXITING test
> > for early break out from the oom killer because it is believed that
> > such a task would release its memory shortly and so we do not have
> > to select an oom victim and perform a disruptive action.
> >
> > Currently we make sure that the given task is not participating in the
> > core dumping because it might get blocked for a long time - see
> > d003f371b270 ("oom: don't assume that a coredumping thread will exit
> > soon").
> >
> > The check can still do better though. We shouldn't consider the task
> > unless the whole thread group is going down. This is rather unlikely
> > but not impossible. A single exiting thread would surely leave all the
> > address space behind. If we are really unlucky it might get stuck on the
> > exit path and keep its TIF_MEMDIE and so block the oom killer.
> >
> > Signed-off-by: Michal Hocko <mhocko@xxxxxxxx>
> > ---
> >
> > Hi,
> > I hope I got it right but I would really appreciate if Oleg found some
> > time and double checked after me. The fix is more cosmetic than anything
> > else but I guess it is worth it.
>
> ping...

Andrew, this is not in the mmotm tree now because I didn't feel really
confortable with the patch without Oleg seeing it. But it seems Oleg is
ok [1] with it so could you push it to Linus along with the rest of oom
pile please?

[1] http://lkml.kernel.org/r/20160517184225.GB32068@xxxxxxxxxx

> >
> > Thanks!
> >
> > include/linux/oom.h | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/oom.h b/include/linux/oom.h
> > index 628a43242a34..b09c7dc523ff 100644
> > --- a/include/linux/oom.h
> > +++ b/include/linux/oom.h
> > @@ -102,13 +102,24 @@ extern struct task_struct *find_lock_task_mm(struct task_struct *p);
> >
> > static inline bool task_will_free_mem(struct task_struct *task)
> > {
> > + struct signal_struct *sig = task->signal;
> > +
> > /*
> > * A coredumping process may sleep for an extended period in exit_mm(),
> > * so the oom killer cannot assume that the process will promptly exit
> > * and release memory.
> > */
> > - return (task->flags & PF_EXITING) &&
> > - !(task->signal->flags & SIGNAL_GROUP_COREDUMP);
> > + if (sig->flags & SIGNAL_GROUP_COREDUMP)
> > + return false;
> > +
> > + if (!(task->flags & PF_EXITING))
> > + return false;
> > +
> > + /* Make sure that the whole thread group is going down */
> > + if (!thread_group_empty(task) && !(sig->flags & SIGNAL_GROUP_EXIT))
> > + return false;
> > +
> > + return true;
> > }
> >
> > /* sysctls */
> > --
> > 2.8.0.rc3
> >
>
> --
> Michal Hocko
> SUSE Labs

--
Michal Hocko
SUSE Labs