linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
To: mhocko@kernel.org
Cc: mjaggi@caviumnetworks.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: Possible race condition in oom-killer
Date: Fri, 28 Jul 2017 22:55:51 +0900	[thread overview]
Message-ID: <201707282255.BGI87015.FSFOVQtMOHLJFO@I-love.SAKURA.ne.jp> (raw)
In-Reply-To: <20170728132952.GQ2274@dhcp22.suse.cz>

Michal Hocko wrote:
> On Fri 28-07-17 22:15:01, Tetsuo Handa wrote:
> > task_will_free_mem(current) in out_of_memory() returning false due to
> > MMF_OOM_SKIP already set allowed each thread sharing that mm to select a new
> > OOM victim. If task_will_free_mem(current) in out_of_memory() did not return
> > false, threads sharing MMF_OOM_SKIP mm would not have selected new victims
> > to the level where all OOM killable processes are killed and calls panic().
> 
> I am not sure I understand. Do you mean this?

Yes.

> ---
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 9e8b4f030c1c..671e4a4107d0 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -779,13 +779,6 @@ static bool task_will_free_mem(struct task_struct *task)
>  	if (!__task_will_free_mem(task))
>  		return false;
>  
> -	/*
> -	 * This task has already been drained by the oom reaper so there are
> -	 * only small chances it will free some more
> -	 */
> -	if (test_bit(MMF_OOM_SKIP, &mm->flags))
> -		return false;
> -
>  	if (atomic_read(&mm->mm_users) <= 1)
>  		return true;
>  
> If yes I would have to think about this some more because that might
> have weird side effects (e.g. oom_victims counting after threads passed
> exit_oom_victim).

But this check should not be removed unconditionally. We should still return
false if returning true was not sufficient to solve the OOM situation, for
we need to select next OOM victim in that case.

> 
> Anyway the proper fix for this is to allow reaping mlocked pages.

Different approach is to set TIF_MEMDIE to all threads sharing the same
memory so that threads sharing MMF_OOM_SKIP mm do not need to call
out_of_memory() in order to get TIF_MEMDIE. 

Yet another apporach is to use __GFP_KILLABLE (we can start it as
best effort basis).

>                                                                   Is
> something other than the LTP test affected to give this more priority?
> Do we have other usecases where something mlocks the whole memory?

This panic was caused by 50 threads sharing MMF_OOM_SKIP mm exceeding
number of OOM killable processes. Whether memory is locked or not isn't
important. If a multi-threaded process which consumes little memory was
selected as an OOM victim (and reaped by the OOM reaper and MMF_OOM_SKIP
was set immediately), it might be still possible to select next OOM victims
needlessly.

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

  reply	other threads:[~2017-07-28 13:55 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <e6c83a26-1d59-4afd-55cf-04e58bdde188@caviumnetworks.com>
2017-07-28 12:32 ` Michal Hocko
2017-07-28 12:59   ` Tetsuo Handa
2017-07-28 13:07     ` Michal Hocko
2017-07-28 13:15       ` Tetsuo Handa
2017-07-28 13:29         ` Michal Hocko
2017-07-28 13:55           ` Tetsuo Handa [this message]
2017-07-28 14:07             ` Michal Hocko
2017-07-29  4:31               ` Tetsuo Handa
2017-08-01 12:14                 ` Michal Hocko
2017-08-01 14:16                   ` Tetsuo Handa
2017-08-01 14:47                     ` Michal Hocko
2017-08-01 10:46               ` Tetsuo Handa
2017-08-01 11:30                 ` Michal Hocko
2017-07-28 13:15     ` Manish Jaggi
2017-07-28 13:50   ` Manish Jaggi
2017-07-28 14:12     ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201707282255.BGI87015.FSFOVQtMOHLJFO@I-love.SAKURA.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mjaggi@caviumnetworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox