Re: [PATCH v3] mm,page_alloc: wait for oom_lock before retrying.

[Michal Hocko <mhocko@kernel.org>]

On Thu 14-02-19 01:30:28, Tetsuo Handa wrote:
[...]
> >From 63c5c8ee7910fa9ef1c4067f1cb35a779e9d582c Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Tue, 12 Feb 2019 20:12:35 +0900
> Subject: [PATCH v3] mm,page_alloc: wait for oom_lock before retrying.
> 
> When many hundreds of threads concurrently triggered a page fault, and
> one of them invoked the global OOM killer, the owner of oom_lock is
> preempted for minutes because they are rather depriving the owner of
> oom_lock of CPU time rather than waiting for the owner of oom_lock to
> make progress. We don't want to disable preemption while holding oom_lock
> but we want the owner of oom_lock to complete as soon as possible.
> 
> Thus, this patch kills the dangerous assumption that sleeping for one
> jiffy is sufficient for allowing the owner of oom_lock to make progress.

What does this prevent any _other_ kernel path or even high priority
userspace to preempt the oom killer path? This was the essential
question the last time around and I do not see it covered here. I
strongly suspect that all these games with the locking is just a
pointless tunning for an insane workload without fixing the underlying
issue.

> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  mm/page_alloc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 35fdde0..c867513 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3618,7 +3618,10 @@ void warn_alloc(gfp_t gfp_mask, nodemask_t *nodemask, const char *fmt, ...)
>  	 */
>  	if (!mutex_trylock(&oom_lock)) {
>  		*did_some_progress = 1;
> -		schedule_timeout_uninterruptible(1);
> +		if (mutex_lock_killable(&oom_lock) == 0)
> +			mutex_unlock(&oom_lock);
> +		else if (!tsk_is_oom_victim(current))
> +			schedule_timeout_uninterruptible(1);
>  		return NULL;
>  	}
>  
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs