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

From: Michal Hocko
Date: Tue Apr 26 2016 - 09:57:59 EST


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...

>
> 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