Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: linux-mm@kvack.org, rientjes@google.com, hannes@cmpxchg.org,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC 2/4] mm: replace TIF_MEMDIE checks by tsk_is_oom_victim
Date: Fri, 9 Sep 2016 16:05:09 +0200
Message-ID: <20160909140508.GO4844@dhcp22.suse.cz> (raw)
In-Reply-To: <201609041049.JBG69723.JOFFFVOtQOLMSH@I-love.SAKURA.ne.jp>

On Sun 04-09-16 10:49:52, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 9ee178ba7b71..df58733ca48e 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1899,7 +1899,7 @@ static int try_charge(struct mem_cgroup *memcg, gfp_t gfp_mask,
> >  	 * bypass the last charges so that they can exit quickly and
> >  	 * free their memory.
> >  	 */
> > -	if (unlikely(test_thread_flag(TIF_MEMDIE) ||
> > +	if (unlikely(tsk_is_oom_victim(current) ||
> >  		     fatal_signal_pending(current) ||
> >  		     current->flags & PF_EXITING))
> >  		goto force;
> 
> Does this test_thread_flag(TIF_MEMDIE) (or tsk_is_oom_victim(current)) make sense?
> 
> If current thread is OOM-killed, SIGKILL must be pending before arriving at
> do_exit() and PF_EXITING must be set after arriving at do_exit(). But I can't
> find locations which do memory allocation between clearing SIGKILL and setting
> PF_EXITING.
> 
> When can test_thread_flag(TIF_MEMDIE) == T (or tsk_is_oom_victim(current) == T) &&
> fatal_signal_pending(current) == F && (current->flags & PF_EXITING) == 0 happen?

Maybe you are right. I would have to double check. I am still drown in
a pile of emails after vacation so it will take some time to do that.
Anyway I believe this would be worth a separate patch. This one just
mechanically replaces TIF_MEMDIE check by tsk_is_oom_victim.

> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index b11977585c7b..e26529edcee3 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -1078,7 +1078,7 @@ void pagefault_out_of_memory(void)
> >  		 * be a racing OOM victim for which oom_killer_disable()
> >  		 * is waiting for.
> >  		 */
> > -		WARN_ON(test_thread_flag(TIF_MEMDIE));
> > +		WARN_ON(tsk_is_oom_victim(current));
> >  	}
> >  
> >  	mutex_unlock(&oom_lock);
> 
> Does this WARN_ON() make sense?
> 
> When some user thread called oom_killer_disable(), there are running
> kernel threads but is no running user threads except the one which
> called oom_killer_disable(). Since oom_killer_disable() waits for
> oom_lock, out_of_memory() called from here shall not return false
> before oom_killer_disable() sets oom_killer_disabled = true. Thus,
> possible situation out_of_memory() called from here can return false
> are limited to
> 
>  (a) the one which triggered pagefaults after returning from
>      oom_killer_disable()
>  (b) An OOM victim which was thawed triggered pagefaults from do_exit()
>      after the one which called oom_killer_disable() released oom_lock
>  (c) kernel threads which triggered pagefaults after use_mm()
> 
> . And since kernel threads are not subjected to mark_oom_victim(),
> test_thread_flag(TIF_MEMDIE) == F (or tsk_is_oom_victim(current) == F)
> for kernel threads. Thus, possible situation out_of_memory() called from
> here can return false and we hit this WARN_ON() is limited to (b).
> 
> Even if (a) or (b) is possible, does continuously emitting backtraces
> help? It seems to me that the system is under OOM livelock situation and
> we need to take a different action (e.g. try to allocate a page using
> ALLOC_NO_WATERMARKS in order to make the pagefault be solved, and panic()
> if failed) than emitting same backtraces forever.

I have to think about this some more, so I cannot give it any answer
right now. But again, this is just a mechanical change. If your concern
is correct and the WARN_ON is really pointless it should be similarly
pointless with the TIF_MEMDIE check unless I am missing something.

-- 
Michal Hocko
SUSE Labs

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

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-01  9:51 [RFC 0/4] mm, oom: get rid of TIF_MEMDIE Michal Hocko
2016-09-01  9:51 ` [RFC 1/4] mm, oom: do not rely on TIF_MEMDIE for memory reserves access Michal Hocko
2016-09-04  1:49   ` Tetsuo Handa
2016-09-09 14:00     ` Michal Hocko
2016-09-01  9:51 ` [RFC 2/4] mm: replace TIF_MEMDIE checks by tsk_is_oom_victim Michal Hocko
2016-09-04  1:49   ` Tetsuo Handa
2016-09-09 14:05     ` Michal Hocko [this message]
2016-09-01  9:51 ` [RFC 3/4] mm, oom: do not rely on TIF_MEMDIE for exit_oom_victim Michal Hocko
2016-09-04  1:50   ` Tetsuo Handa
2016-09-09 14:08     ` Michal Hocko
2016-09-10  6:29       ` Tetsuo Handa
2016-09-10 12:55         ` Tetsuo Handa
2016-09-12  9:11           ` Michal Hocko
2016-09-13  6:25             ` Tetsuo Handa
2016-09-13  7:21               ` Michal Hocko
2016-09-14 13:50   ` Michal Hocko
2016-09-01  9:51 ` [RFC 4/4] arch: get rid of TIF_MEMDIE Michal Hocko
2016-09-15 14:41 ` [RFC 0/4] mm, oom: " Johannes Weiner
2016-09-16  7:15   ` Michal Hocko
2016-09-19 16:18     ` Johannes Weiner
2016-09-19 19:02       ` Michal Hocko

Reply instructions:

You may reply publically 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=20160909140508.GO4844@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=rientjes@google.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

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git