Re: cgroup-aware OOM killer, how to move forward

From: Michal Hocko <mhocko@kernel.org>
To: David Rientjes <rientjes@google.com>
Cc: Roman Gushchin <guro@fb.com>,
	linux-mm@kvack.org, akpm@linux-foundation.org,
	hannes@cmpxchg.org, tj@kernel.org, gthelen@google.com
Subject: Re: cgroup-aware OOM killer, how to move forward
Date: Tue, 17 Jul 2018 14:41:55 +0200	[thread overview]
Message-ID: <20180717124155.GH7193@dhcp22.suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.21.1807162115180.157949@chino.kir.corp.google.com>

On Mon 16-07-18 21:19:18, David Rientjes wrote:
> On Fri, 13 Jul 2018, Roman Gushchin wrote:
> 
> > > > > All cgroup v2 files do not need to be boolean and the only way you can add 
> > > > > a subtree oom kill is to introduce yet another file later.  Please make it 
> > > > > tristate so that you can define a mechanism of default (process only), 
> > > > > local cgroup, or subtree, and so we can avoid adding another option later 
> > > > > that conflicts with the proposed one.  This should be easy.
> > > > 
> > > > David, we're adding a cgroup v2 knob, and in cgroup v2 a memory cgroup
> > > > either has a sub-tree, either attached processes. So, there is no difference
> > > > between local cgroup and subtree.
> > > > 
> > > 
> > > Uhm, what?  We're talking about a common ancestor reaching its limit, so 
> > > it's oom, and it has multiple immediate children with their own processes 
> > > attached.  The difference is killing all processes attached to the 
> > > victim's cgroup or all processes under the oom mem cgroup's subtree.
> > > 
> > 
> > But it's a binary decision, no?
> > If memory.group_oom set, the whole sub-tree will be killed. Otherwise not.
> > 
> 
> No, if memory.max is reached and memory.group_oom is set, my understanding 
> of your proposal is that a process is chosen and all eligible processes 
> attached to its mem cgroup are oom killed.  My desire for a tristate is so 
> that it can be specified that all processes attached to the *subtree* are 
> oom killed.  With single unified hierarchy mandated by cgroup v2, we can 
> separate descendant cgroups for use with other controllers and enforce 
> memory.max by an ancestor.
> 
> Making this a boolean value is only preventing it from becoming 
> extensible.  If memory.group_oom only is effective for the victim's mem 
> cgroup, it becomes impossible to specify that all processes in the subtree 
> should be oom killed as a result of the ancestor limit without adding yet 
> another tunable.

No, this is mangling the interface again. I have already objected to
this [1]. group_oom only tells to tear the whole group down. How you
select this particular group is a completely different story. Conflating
those two things is a bad interface to start with. Killing the whold
cgroup or only a single process should be invariant for the particular
memcg regardless of what is the oom selection policy above in the
hierarchy. Either you are indivisible workload or you are not, full
stop.

If we really need a better control over how to select subtrees then this
should be a separate control knob. How should it look like is a matter
of discussion but it will be hard to find any consensus if the
single-knob-for-single-purpose approach is not clear.

[1] http://lkml.kernel.org/r/20180117160004.GH2900@dhcp22.suse.cz
-- 
Michal Hocko
SUSE Labs