From: Michal Hocko <mhocko@suse.cz>
To: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
Andrew Morton <akpm@linux-foundation.org>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 2/2] memcg: do not sleep on OOM waitqueue with full charge context
Date: Thu, 6 Jun 2013 17:23:40 +0200 [thread overview]
Message-ID: <20130606152340.GC24115@dhcp22.suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.02.1306052058340.25115@chino.kir.corp.google.com>
On Wed 05-06-13 21:10:51, David Rientjes wrote:
> On Wed, 5 Jun 2013, Johannes Weiner wrote:
[...]
> > While reworking the OOM routine, also remove a needless OOM waitqueue
> > wakeup when invoking the killer. Only uncharges and limit increases,
> > things that actually change the memory situation, should do wakeups.
> >
>
> It's not needless at all, it's vitally required! The oom killed process
> needs to be removed from the waitqueue and scheduled now with TIF_MEMDIE
> that the memcg oom killer provided so the allocation succeeds in the page
> allocator and memcg bypasses the charge so it can exit.
The tasks are waiting with TASK_KILLABLE flags so it gets woken up and
the bypass happens. Calling memcg_wakeup_oom is actually wrong here
because it wakes all tasks up despite there is no reason for that. No
charges have been released yet so another retry loop could be pointless.
We need to be patient and wait for wake up from a uncharge path.
> Exactly what problem are you trying to address with this patch? I don't
> see any description of the user-visible effects or a specific xample of
> the scenario you're trying to address here.
Maybe I am biased because I've tried to handle the same problem some time
ago, but the changelog clearly says that memcg oom handling is fragile
and deadlock prone because of locks that are held while memory is
charged and oom handled and so oom targets might not get killed because
they are stuck at the same lock which will not get released until the
charge succeeds.
It addresses the problem by moving oom handling outside of any locks
which solves this category of dead locks. I agree that the changelog
could better (well, each one can be). It could use some examples (e.g.
the i_mutex we have seen few months ago or a simple unkillable brk which
is hanging on mmap_sem for writing while a page fault is handled and
memcg oom triggered).
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@suse.cz>
To: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
Andrew Morton <akpm@linux-foundation.org>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 2/2] memcg: do not sleep on OOM waitqueue with full charge context
Date: Thu, 6 Jun 2013 17:23:40 +0200 [thread overview]
Message-ID: <20130606152340.GC24115@dhcp22.suse.cz> (raw)
Message-ID: <20130606152340.ehqfMcVSlJJn_299fnvVfpvAWqg3SKA2we3Yzazf7W8@z> (raw)
In-Reply-To: <alpine.DEB.2.02.1306052058340.25115@chino.kir.corp.google.com>
On Wed 05-06-13 21:10:51, David Rientjes wrote:
> On Wed, 5 Jun 2013, Johannes Weiner wrote:
[...]
> > While reworking the OOM routine, also remove a needless OOM waitqueue
> > wakeup when invoking the killer. Only uncharges and limit increases,
> > things that actually change the memory situation, should do wakeups.
> >
>
> It's not needless at all, it's vitally required! The oom killed process
> needs to be removed from the waitqueue and scheduled now with TIF_MEMDIE
> that the memcg oom killer provided so the allocation succeeds in the page
> allocator and memcg bypasses the charge so it can exit.
The tasks are waiting with TASK_KILLABLE flags so it gets woken up and
the bypass happens. Calling memcg_wakeup_oom is actually wrong here
because it wakes all tasks up despite there is no reason for that. No
charges have been released yet so another retry loop could be pointless.
We need to be patient and wait for wake up from a uncharge path.
> Exactly what problem are you trying to address with this patch? I don't
> see any description of the user-visible effects or a specific xample of
> the scenario you're trying to address here.
Maybe I am biased because I've tried to handle the same problem some time
ago, but the changelog clearly says that memcg oom handling is fragile
and deadlock prone because of locks that are held while memory is
charged and oom handled and so oom targets might not get killed because
they are stuck at the same lock which will not get released until the
charge succeeds.
It addresses the problem by moving oom handling outside of any locks
which solves this category of dead locks. I agree that the changelog
could better (well, each one can be). It could use some examples (e.g.
the i_mutex we have seen few months ago or a simple unkillable brk which
is hanging on mmap_sem for writing while a page fault is handled and
memcg oom triggered).
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2013-06-06 15:23 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-06 3:09 [patch 1/2] arch: invoke oom-killer from page fault Johannes Weiner
2013-06-06 3:09 ` Johannes Weiner
2013-06-06 3:09 ` [patch 2/2] memcg: do not sleep on OOM waitqueue with full charge context Johannes Weiner
2013-06-06 3:09 ` Johannes Weiner
2013-06-06 4:10 ` David Rientjes
2013-06-06 4:10 ` David Rientjes
2013-06-06 5:33 ` Johannes Weiner
2013-06-06 5:33 ` Johannes Weiner
2013-06-06 17:33 ` Johannes Weiner
2013-06-06 17:33 ` Johannes Weiner
2013-06-06 20:11 ` David Rientjes
2013-06-06 20:11 ` David Rientjes
2013-06-06 21:54 ` Johannes Weiner
2013-06-06 21:54 ` Johannes Weiner
2013-06-06 22:18 ` David Rientjes
2013-06-06 22:18 ` David Rientjes
2013-06-07 0:02 ` Johannes Weiner
2013-06-07 0:02 ` Johannes Weiner
2013-06-11 21:57 ` David Rientjes
2013-06-11 21:57 ` David Rientjes
2013-06-12 8:28 ` Michal Hocko
2013-06-12 8:28 ` Michal Hocko
[not found] ` <20130612082817.GA6706-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
2013-06-12 20:12 ` David Rientjes
2013-06-12 20:12 ` David Rientjes
2013-06-12 20:37 ` Michal Hocko
2013-06-12 20:37 ` Michal Hocko
2013-06-12 20:49 ` David Rientjes
2013-06-12 20:49 ` David Rientjes
2013-06-13 13:48 ` Michal Hocko
2013-06-13 13:48 ` Michal Hocko
2013-06-13 20:34 ` David Rientjes
2013-06-13 20:34 ` David Rientjes
2013-06-14 9:29 ` Michal Hocko
2013-06-14 9:29 ` Michal Hocko
2013-06-06 15:23 ` Michal Hocko [this message]
2013-06-06 15:23 ` Michal Hocko
2013-06-06 3:57 ` [patch 1/2] arch: invoke oom-killer from page fault David Rientjes
2013-06-06 3:57 ` David Rientjes
2013-06-06 4:36 ` Johannes Weiner
2013-06-06 4:36 ` Johannes Weiner
2013-06-06 4:43 ` David Rientjes
2013-06-06 6:49 ` Vineet Gupta
2013-06-06 6:49 ` Vineet Gupta
[not found] ` <20130606043620.GA9406-druUgvl0LCNAfugRpC6u6w@public.gmane.org>
2013-06-06 14:59 ` Michal Hocko
2013-06-06 14:59 ` Michal Hocko
2013-06-06 4:55 ` 刘胜蛟
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=20130606152340.GC24115@dhcp22.suse.cz \
--to=mhocko@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--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
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;
as well as URLs for NNTP newsgroup(s).