Re: [PATCH 5/6] mm,oom: Re-enable OOM killer using timers.

[Michal Hocko]

On Wed 17-02-16 19:34:46, Tetsuo Handa wrote:
> >From 6f07b71c97766ec111d26c3424bded465ca48195 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> Date: Wed, 17 Feb 2016 16:37:01 +0900
> Subject: [PATCH 5/6] mm,oom: Re-enable OOM killer using timers.
> 
> We are trying to reduce the possibility of hitting OOM livelock by
> introducing the OOM reaper, but there are situations where the OOM reaper
> cannot reap the victim's memory. We want to introduce the OOM reaper as
> simple as possible and make the OOM reaper better via incremental
> development.
> 
> This patch adds a timer for handling corner cases where a TIF_MEMDIE
> thread got stuck by reasons not handled by the initial version of the
> OOM reaper. Since "mm,oom: exclude TIF_MEMDIE processes from candidates."
> made sure that we won't choose the same OOM victim forever and this patch
> makes sure that the kernel automatically presses SysRq-f upon OOM stalls,
> we will not OOM stall forever as long as the OOM killer is called.

Can we actually start by incremental changes first and only get to this
after we cannot find a proper way to fix existing issues?

I would like to at least make mmap_sem taken for write killable
first. This should allow the oom_reaper to make a forward progress and
allow the OOM killer to select another task when necessary (e.g. the
victim wasn't sitting on a large amount of reclaimable memory). This
has an advantage that the TIF_MEMDIE release is bound to a clearly
defined action rather than a $RANDOM timemout which will always be hard
to justify. We can talk about timeout based solutions after we are able
to livelock the system even after all well defined actions will have
failed. I really consider it premature right now.

> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
>  mm/oom_kill.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index ebc6764..fba2c62 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -45,6 +45,11 @@ int sysctl_oom_dump_tasks = 1;
>  
>  DEFINE_MUTEX(oom_lock);
>  
> +static void oomkiller_reset(unsigned long arg)
> +{
> +}
> +static DEFINE_TIMER(oomkiller_victim_wait_timer, oomkiller_reset, 0, 0);
> +
>  #ifdef CONFIG_NUMA
>  /**
>   * has_intersects_mems_allowed() - check task eligiblity for kill
> @@ -299,7 +304,8 @@ enum oom_scan_t oom_scan_process_thread(struct oom_control *oc,
>  	 */
>  	if (test_tsk_thread_flag(task, TIF_MEMDIE)) {
>  		if (!is_sysrq_oom(oc))
> -			return OOM_SCAN_ABORT;
> +			return timer_pending(&oomkiller_victim_wait_timer) ?
> +				OOM_SCAN_ABORT : OOM_SCAN_CONTINUE;
>  	}
>  	if (!task->mm)
>  		return OOM_SCAN_CONTINUE;
> @@ -452,6 +458,8 @@ void mark_oom_victim(struct task_struct *tsk)
>  	 */
>  	__thaw_task(tsk);
>  	atomic_inc(&oom_victims);
> +	/* Make sure that we won't wait for this task forever. */
> +	mod_timer(&oomkiller_victim_wait_timer, jiffies + 5 * HZ);
>  }
>  
>  /**
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs