linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Dave Chinner <david@fromorbit.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 6/9] mm: oom_kill: simplify OOM killer locking
Date: Tue, 28 Apr 2015 15:43:35 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1504281540280.10203@chino.kir.corp.google.com> (raw)
In-Reply-To: <1430161555-6058-7-git-send-email-hannes@cmpxchg.org>

On Mon, 27 Apr 2015, Johannes Weiner wrote:

> The zonelist locking and the oom_sem are two overlapping locks that
> are used to serialize global OOM killing against different things.
> 
> The historical zonelist locking serializes OOM kills from allocations
> with overlapping zonelists against each other to prevent killing more
> tasks than necessary in the same memory domain.  Only when neither
> tasklists nor zonelists from two concurrent OOM kills overlap (tasks
> in separate memcgs bound to separate nodes) are OOM kills allowed to
> execute in parallel.
> 
> The younger oom_sem is a read-write lock to serialize OOM killing
> against the PM code trying to disable the OOM killer altogether.
> 
> However, the OOM killer is a fairly cold error path, there is really
> no reason to optimize for highly performant and concurrent OOM kills.
> And the oom_sem is just flat-out redundant.
> 
> Replace both locking schemes with a single global mutex serializing
> OOM kills regardless of context.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Acked-by: Michal Hocko <mhocko@suse.cz>

Acked-by: David Rientjes <rientjes@google.com>

Thanks for doing this, it cleans up the code quite a bit and I think 
there's the added benefit of not interleaving oom killer messages in the 
kernel log, and that's important since it's the only way we can currently 
discover that the kernel has killed something.

It's not vital and somewhat unrelated to your patch, but if we can't grab 
the mutex with the trylock in __alloc_pages_may_oom() then I think it 
would be more correct to do schedule_timeout_killable() rather than 
uninterruptible.  I just mention it if you happen to go through another 
revision of the series and want to switch it at the same time.

--
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:[~2015-04-28 22:43 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-27 19:05 [PATCH 0/9] mm: improve OOM mechanism v2 Johannes Weiner
2015-04-27 19:05 ` [PATCH 1/9] mm: oom_kill: remove unnecessary locking in oom_enable() Johannes Weiner
2015-04-27 19:05 ` [PATCH 2/9] mm: oom_kill: clean up victim marking and exiting interfaces Johannes Weiner
2015-04-27 19:05 ` [PATCH 3/9] mm: oom_kill: switch test-and-clear of known TIF_MEMDIE to clear Johannes Weiner
2015-04-27 19:05 ` [PATCH 4/9] mm: oom_kill: generalize OOM progress waitqueue Johannes Weiner
2015-04-28 22:40   ` David Rientjes
2015-04-27 19:05 ` [PATCH 5/9] mm: oom_kill: remove unnecessary locking in exit_oom_victim() Johannes Weiner
2015-04-28 22:40   ` David Rientjes
2015-04-27 19:05 ` [PATCH 6/9] mm: oom_kill: simplify OOM killer locking Johannes Weiner
2015-04-28 22:43   ` David Rientjes [this message]
2015-04-29  5:48     ` Tetsuo Handa
2015-04-27 19:05 ` [PATCH 7/9] mm: page_alloc: inline should_alloc_retry() Johannes Weiner
2015-04-27 19:05 ` [PATCH 8/9] mm: page_alloc: wait for OOM killer progress before retrying Johannes Weiner
2015-04-28 13:18   ` Michal Hocko
2015-04-27 19:05 ` [PATCH 9/9] mm: page_alloc: memory reserve access for OOM-killing allocations Johannes Weiner
2015-04-28 13:30   ` Michal Hocko
2015-04-28 14:59     ` Michal Hocko
2015-04-28 10:34 ` [PATCH 0/9] mm: improve OOM mechanism v2 Tetsuo Handa
2015-04-28 13:55   ` Michal Hocko
2015-04-28 15:50     ` Tetsuo Handa
2015-04-29 12:55       ` Johannes Weiner
2015-04-29 14:40         ` Michal Hocko
2015-04-29 17:27           ` Tetsuo Handa
2015-04-29 18:31             ` Michal Hocko
2015-04-30  9:44               ` Tetsuo Handa
2015-04-30 14:25                 ` Michal Hocko
2015-05-23 14:42                   ` Tetsuo Handa
2015-05-04 18:02 ` Johannes Weiner
2015-05-04 19:01   ` 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=alpine.DEB.2.10.1504281540280.10203@chino.kir.corp.google.com \
    --to=rientjes@google.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=vbabka@suse.cz \
    /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