Re: [PATCH] mm/page_alloc: Wait for oom_lock before retrying.

[Sergey Senozhatsky <sergey.senozhatsky@gmail.com>]

Hello,

On (12/08/16 20:00), Tetsuo Handa wrote:
[..]
> Theread-1 was trying to flush printk log buffer by printk() from
> oom_kill_process() with oom_lock held. Thread-2 was appending to printk
> log buffer by printk() from warn_alloc() because Thread-2 cannot hold
> oom_lock held by Thread-1. As a result, this formed an AB-BA livelock.
> 
> Although warn_alloc() calls printk() aggressively enough to livelock is
> problematic, at least we can say that it is wasteful to spend CPU time for
> pointless "direct reclaim and warn_alloc()" calls when waiting for the OOM
> killer to send SIGKILL. Therefore, this patch replaces mutex_trylock()
> with mutex_lock_killable().
> 
> Replacing mutex_trylock() with mutex_lock_killable() should be safe, for
> if somebody by error called __alloc_pages_may_oom() with oom_lock held,
> it will livelock because did_some_progress will be set to 1 despite
> mutex_trylock() failure.

so I have some patches in my own/personal/secret tree. the patches were
not yet published to the kernel-list (well, not all of them. I'll do it
a bit later). we are moving towards the "printk() just appends the messages
to the logbuf" idea:

https://gitlab.com/senozhatsky/linux-next-ss/commits/printk-safe-deferred


	-ss

> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  mm/page_alloc.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6de9440..6c43d8e 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3037,12 +3037,16 @@ void warn_alloc(gfp_t gfp_mask, const char *fmt, ...)
>  	*did_some_progress = 0;
>  
>  	/*
> -	 * Acquire the oom lock.  If that fails, somebody else is
> -	 * making progress for us.
> +	 * Give the OOM killer enough CPU time for sending SIGKILL.
> +	 * Do not return without a short sleep unless TIF_MEMDIE is set, for
> +	 * currently tsk_is_oom_victim(current) == true does not make
> +	 * gfp_pfmemalloc_allowed() == true via TIF_MEMDIE until
> +	 * mark_oom_victim(current) is called.
>  	 */
> -	if (!mutex_trylock(&oom_lock)) {
> +	if (mutex_lock_killable(&oom_lock)) {
>  		*did_some_progress = 1;
> -		schedule_timeout_uninterruptible(1);
> +		if (!test_thread_flag(TIF_MEMDIE))
> +			schedule_timeout_uninterruptible(1);
>  		return NULL;
>  	}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>