From: Michal Hocko <mhocko@suse.cz>
To: David Rientjes <rientjes@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Johannes Weiner <hannes@cmpxchg.org>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
cgroups@vger.kernel.org,
"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [PATCH] memcg: Do not hang on OOM when killed by userspace OOM access to memory reserves
Date: Wed, 15 Jan 2014 15:26:03 +0100 [thread overview]
Message-ID: <20140115142603.GM8782@dhcp22.suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.02.1401101327430.21486@chino.kir.corp.google.com>
On Fri 10-01-14 13:33:01, David Rientjes wrote:
> On Fri, 10 Jan 2014, Michal Hocko wrote:
>
> > > > ---
> > > > mm/memcontrol.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > > index b8dfed1b9d87..b86fbb04b7c6 100644
> > > > --- a/mm/memcontrol.c
> > > > +++ b/mm/memcontrol.c
> > > > @@ -2685,7 +2685,8 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
> > > > * MEMDIE process.
> > > > */
> > > > if (unlikely(test_thread_flag(TIF_MEMDIE)
> > > > - || fatal_signal_pending(current)))
> > > > + || fatal_signal_pending(current))
> > > > + || current->flags & PF_EXITING)
> > > > goto bypass;
> > > >
> > > > if (unlikely(task_in_memcg_oom(current)))
> > >
> > > This would become problematic if significant amount of memory is charged
> > > in the exit() path.
> >
> > But this would hurt also for fatal_signal_pending tasks, wouldn't it?
>
> Yes, and as I've said twice now, that should be removed.
And you failed to provide any relevant data to back your suggestions. I
have told you that we have these heuristics for ages and we need a
strong justification to drop them. So if you really think that they are
not appropriate then back your statements with real data.
E.g. measure how much memory are we talking about.
> These bypasses should be given to one thread and one thread only,
> which would be the oom killed thread if it needs access to memory
> reserves to either allocate memory or charge memory.
There is no way to determine whether a task has been killed due to user
space OOM killer or by a regular kill.
> If you are suggesting we use the "user" and "oom" top-level memcg
> hierarchy for allowing memory to be available for userspace system oom
> handlers, then this has become important when in the past it may have been
> a minor point.
I am not sure it would be _that_ important and if that really becomes to
be the case then we should deal with it. So far I haven't see any
evidence there is a lot of memory charged on the exit path.
> > Besides that I do not see any source of allocation after exit_signals.
> >
>
> That's fine for today but may not be in the future. If memory allocation
> is done after PF_EXITING in the future, are people going to check memcg
> bypasses? No. And now we have additional memory bypass to root that will
> cause our userspace system oom hanlders to be oom themselves with the
> suggested configuration.
>
> Using the "user" and "oom" top-level memcg hierarchy is a double edged
> sword, we must attempt to prevent all of these bypasses as much as
> possible. The only relevant bypass here is for TIF_MEMDIE which would be
> set if necessary for the one thread that needs it.
TIF_MEMDIE doesn't work for userspace OOM killers. So we cannot rely on
this flag currently.
> > > I don't know of an egregious amount of memory being
> > > allocated and charged after PF_EXITING is set, but if it happens in the
> > > future then this could potentially cause system oom conditions even in
> > > memcg configurations
> >
> > Even if that happens then the global OOM killer would give the exiting
> > task access to memory reserves and wouldn't kill anything else.
> >
> > So I am not sure what problem do you see exactly.
> >
>
> Userspace system oom handlers being able to handle memcg oom conditions in
> the top-level "user" memcg as proposed by Tejun. If the global oom killer
> becomes a part of that discussion at all, then the userspace system oom
> handler never got a chance to handle the "user" oom.
>
> > Besides that allocating egregious amount of memory after exit_signals
> > sounds fundamentally broken to me.
> >
>
> Egregious could be defined as allocating a few bytes multiplied by
> thousands of threads in PF_EXITING.
Does this happen in the real life.
Look, I have no objections to make the OOM handling better but it would
help a lot to build new heuristics based on some data in hands. I tried
to repeat that again and again but it seems to not help. I do not want
to end up with new sets of heuristics that break other stuff jut because
they made sense in the context of the specific usecase.
--
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>
next prev parent reply other threads:[~2014-01-15 14:26 UTC|newest]
Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-31 1:39 [patch] mm, memcg: add memory.oom_control notification for system oom David Rientjes
2013-10-31 5:49 ` Johannes Weiner
2013-11-13 22:19 ` David Rientjes
2013-11-13 23:34 ` Johannes Weiner
2013-11-14 0:56 ` David Rientjes
2013-11-14 3:25 ` Johannes Weiner
2013-11-14 22:57 ` David Rientjes
2013-11-14 23:26 ` [patch 1/2] mm, memcg: avoid oom notification when current needs access to memory reserves David Rientjes
2013-11-14 23:26 ` [patch 2/2] mm, memcg: add memory.oom_control notification for system oom David Rientjes
2013-11-18 18:52 ` Michal Hocko
2013-11-19 1:25 ` David Rientjes
2013-11-19 12:41 ` Michal Hocko
2013-11-18 12:52 ` [patch 1/2] mm, memcg: avoid oom notification when current needs access to memory reserves Michal Hocko
2013-11-18 12:55 ` Michal Hocko
2013-11-19 1:19 ` David Rientjes
2013-11-18 15:41 ` Johannes Weiner
2013-11-18 16:51 ` Michal Hocko
2013-11-19 1:22 ` David Rientjes
2013-11-22 16:51 ` Johannes Weiner
2013-11-27 0:53 ` David Rientjes
2013-11-27 16:34 ` Johannes Weiner
2013-11-27 21:51 ` David Rientjes
2013-11-27 23:19 ` Johannes Weiner
2013-11-28 0:22 ` David Rientjes
2013-11-28 2:28 ` Johannes Weiner
2013-11-28 2:52 ` David Rientjes
2013-11-28 3:16 ` Johannes Weiner
2013-12-02 20:02 ` Michal Hocko
2013-12-02 21:25 ` Johannes Weiner
2013-12-03 12:04 ` Michal Hocko
2013-12-03 20:17 ` Johannes Weiner
2013-12-03 21:00 ` Michal Hocko
2013-12-03 21:23 ` Johannes Weiner
2013-12-03 23:50 ` David Rientjes
2013-12-04 3:34 ` Johannes Weiner
2013-12-04 11:13 ` Michal Hocko
2013-12-05 0:23 ` David Rientjes
2013-12-09 12:48 ` Michal Hocko
2013-12-09 21:46 ` David Rientjes
2013-12-09 22:51 ` Johannes Weiner
2013-12-09 23:05 ` Johannes Weiner
2014-01-10 0:34 ` David Rientjes
2013-12-10 10:38 ` Michal Hocko
2013-12-11 1:03 ` David Rientjes
2013-12-11 9:55 ` Michal Hocko
2013-12-11 22:40 ` David Rientjes
2013-12-12 10:31 ` Michal Hocko
2013-12-12 10:50 ` Michal Hocko
2013-12-12 12:11 ` Michal Hocko
2013-12-12 12:37 ` Michal Hocko
2013-12-13 23:55 ` David Rientjes
2013-12-17 16:23 ` Michal Hocko
2013-12-17 20:50 ` David Rientjes
2013-12-18 20:04 ` Michal Hocko
2013-12-19 6:09 ` David Rientjes
2013-12-19 14:41 ` Michal Hocko
2014-01-08 0:25 ` Andrew Morton
2014-01-08 10:33 ` Michal Hocko
2014-01-09 14:30 ` [PATCH] memcg: Do not hang on OOM when killed by userspace OOM " Michal Hocko
2014-01-09 21:40 ` David Rientjes
2014-01-10 8:23 ` Michal Hocko
2014-01-10 21:33 ` David Rientjes
2014-01-15 14:26 ` Michal Hocko [this message]
2014-01-15 21:19 ` David Rientjes
2014-01-16 10:12 ` Michal Hocko
2014-01-21 6:13 ` David Rientjes
2014-01-21 13:21 ` Michal Hocko
2014-01-09 21:34 ` [patch 1/2] mm, memcg: avoid oom notification when current needs " David Rientjes
2014-01-09 22:47 ` Andrew Morton
2014-01-10 0:01 ` David Rientjes
2014-01-10 0:12 ` Andrew Morton
2014-01-10 0:23 ` David Rientjes
2014-01-10 0:35 ` David Rientjes
2014-01-10 22:14 ` Johannes Weiner
2014-01-12 22:10 ` David Rientjes
2014-01-15 14:34 ` Michal Hocko
2014-01-15 21:23 ` David Rientjes
2014-01-16 9:32 ` Michal Hocko
2014-01-21 5:58 ` David Rientjes
2014-01-21 6:04 ` Greg Kroah-Hartmann
2014-01-21 6:08 ` David Rientjes
2014-01-10 8:30 ` Michal Hocko
2014-01-10 21:38 ` David Rientjes
2014-01-10 22:34 ` Johannes Weiner
2014-01-12 22:14 ` David Rientjes
2013-11-18 15:54 ` [patch] mm, memcg: add memory.oom_control notification for system oom Johannes Weiner
2013-11-18 23:15 ` One Thousand Gnomes
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=20140115142603.GM8782@dhcp22.suse.cz \
--to=mhocko@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=cgroups@vger.kernel.org \
--cc=ebiederm@xmission.com \
--cc=hannes@cmpxchg.org \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--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