linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
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
Subject: Re: [patch] mm, memcg: add oom killer delay
Date: Tue, 4 Jun 2013 11:55:14 +0200	[thread overview]
Message-ID: <20130604095514.GC31242@dhcp22.suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.02.1306031411380.22083@chino.kir.corp.google.com>

On Mon 03-06-13 14:17:54, David Rientjes wrote:
> On Mon, 3 Jun 2013, Michal Hocko wrote:
> 
> > > What do you suggest when you read the "tasks" file and it returns -ENOMEM 
> > > because kmalloc() fails because the userspace oom handler's memcg is also 
> > > oom? 
> > 
> > That would require that you track kernel allocations which is currently
> > done only for explicit caches.
> > 
> 
> That will not always be the case, and I think this could be a prerequisite 
> patch for such support that we have internally. 

> I'm not sure a userspace oom notifier would want to keep a
> preallocated buffer around that is mlocked in memory for all possible
> lengths of this file.

Well, an oom handler which allocates memory under the same restricted
memory doesn't make much sense to me. Tracking all kmem allocations
makes it almost impossible to implement a non-trivial handler.

> > > Obviously it's not a situation we want to get into, but unless you 
> > > know that handler's exact memory usage across multiple versions, nothing 
> > > else is sharing that memcg, and it's a perfect implementation, you can't 
> > > guarantee it.  We need to address real world problems that occur in 
> > > practice.
> > 
> > If you really need to have such a guarantee then you can have a _global_
> > watchdog observing oom_control of all groups that provide such a vague
> > requirements for oom user handlers.
> > 
> 
> The whole point is to allow the user to implement their own oom policy.

OK, maybe I just wasn't clear enough or I am missing your point. Your
users _can_ implement and register their oom handlers. But as your
requirements are rather benevolent for handlers implementation you would
have a global watchdog which would sit on the oom_control of those
groups (which are allowed to have own handlers - all of them in your
case I guess) and trigger (user defined/global) timeout when it gets a
notification. If the group was under oom always during the timeout then
just disable oom_control until oom is settled (under_oom is 0).

Why wouldn't something like this work for your use case?

> If the policy was completely encapsulated in kernel code, we don't need to 
> ever disable the oom killer even with memory.oom_control.  Users may 
> choose to kill the largest process, the newest process, the oldest 
> process, sacrifice children instead of parents, prevent forkbombs, 
> implement their own priority scoring (which is what we do), kill the 
> allocating task, etc.
> 
> To not merge this patch, I'd ask that you show an alternative that allows 
> users to implement their own userspace oom handlers and not require admin 
> intervention when things go wrong.

Hohmm, so you are insisting on something that can be implemented in the
userspace and put it into the kernel because it is more convenient for
you and your use case. This doesn't sound like a way for accepting a
feature.

To make this absolutely clear. I do understand your requirements but you
haven't shown any _argument_ why the timeout you are proposing cannot be
implemented in the userspace. I will not ack this without this
reasoning.

And yes we should make memcg oom handling less deadlock prone and
Johannes' work in this thread is a good step forward.
-- 
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	other threads:[~2013-06-04  9:55 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-30  1:18 David Rientjes
2013-05-30 15:07 ` Michal Hocko
2013-05-30 20:47   ` David Rientjes
2013-05-31  8:10     ` Michal Hocko
2013-05-31 10:22       ` David Rientjes
2013-05-31 11:02         ` Michal Hocko
2013-05-31 11:21         ` Michal Hocko
2013-05-31 19:29           ` David Rientjes
2013-06-01  6:11             ` Johannes Weiner
2013-06-01 10:29               ` Michal Hocko
2013-06-01 15:15                 ` Johannes Weiner
2013-06-03 15:34               ` Michal Hocko
2013-06-03 16:48                 ` Johannes Weiner
2013-06-03 18:03                   ` Michal Hocko
2013-06-03 18:30                   ` Johannes Weiner
2013-06-03 21:33                     ` KOSAKI Motohiro
2013-06-04  9:17                     ` Michal Hocko
2013-06-04 18:48                       ` Johannes Weiner
2013-06-04 19:27                         ` Michal Hocko
2013-06-05 13:49                         ` Michal Hocko
2013-06-03 16:31               ` Michal Hocko
2013-06-03 16:51                 ` Johannes Weiner
2013-06-01 10:20             ` Michal Hocko
2013-06-03 18:18               ` David Rientjes
2013-06-03 18:54                 ` Johannes Weiner
2013-06-03 19:09                   ` David Rientjes
2013-06-03 21:43                     ` Johannes Weiner
2013-06-03 19:31                 ` Michal Hocko
2013-06-03 21:17                   ` David Rientjes
2013-06-04  9:55                     ` Michal Hocko [this message]
2013-06-05  6:40                       ` David Rientjes
2013-06-05  9:39                         ` Michal Hocko
2013-06-06  0:09                           ` David Rientjes
2013-06-10 14:23                             ` Michal Hocko
2013-06-11 20:33                               ` David Rientjes
2013-06-12 20:23                                 ` Michal Hocko
2013-06-12 21:27                                   ` David Rientjes
2013-06-13 15:16                                     ` Michal Hocko
2013-06-13 22:25                                       ` David Rientjes
2013-06-14  0:56                                         ` Kamezawa Hiroyuki
2013-06-14 10:12                                           ` David Rientjes
2013-06-19 21:30                                             ` David Rientjes
2013-06-25  1:39                                             ` Kamezawa Hiroyuki
2013-06-26 23:18                                               ` David Rientjes
2013-07-10 11:23                                             ` Michal Hocko
2013-05-31 21:46 ` Andrew Morton
2013-06-03 18:00   ` David Rientjes

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=20130604095514.GC31242@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-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