Re: [patch v3 -mm 3/6] mm, memcg: add hierarchical usage oom policy

From: David Rientjes
Date: Mon Jul 23 2018 - 19:22:11 EST


On Mon, 23 Jul 2018, Roman Gushchin wrote:

> > Roman, I'm trying to make progress so that the cgroup aware oom killer is
> > in a state that it can be merged. Would you prefer a second tunable here
> > to specify a cgroup's points includes memory from its subtree?
>
> Hi, David!
>
> It's hard to tell, because I don't have a clear picture of what you're
> suggesting now.

Each patch specifies what it does rather elaborately. If there's
confusion on what this patch, or any of the patches in this patchset, is
motivated by or addresses, please call it out specifically.

> My biggest concern about your last version was that it's hard
> to tell what oom_policy really defines. Each value has it's own application
> rules, which is a bit messy (some values are meaningful for OOMing cgroup only,
> other are reading on hierarchy traversal).
> If you know how to make it clear and non-contradictory,
> please, describe the proposed interface.
>

As my initial response stated, "tree" has cgroup aware properties but it
considers the subtree usage as its own. I do not know of any usecase,
today or in the future, that would want subtree usage accounted to its own
when being considered as a single indivisible memory consumer yet still
want per-process oom kill selection.

If you do not prefer that overloading, I can break the two out from one
another such that one tunable defines cgroup vs process, and another
defines subtree usage being considered or not. That's a perfectly fine
suggestion and I have no problem implementing it. The only reason I did
not was because I do not know of any user that would want process &&
subtree and that would reduce the number of files for mem cgroup by one.

If you'd like me to separate these out by adding another tunable, please
let me know. We will already have another tunable later, but is not
required for this to be merged as the cover letter states, to allow the
user to adjust the calculation for a subtree such that it may protect
important cgroups that are allowed to use more memory than others.

> > It would be helpful if you would also review the rest of the patchset.
>
> I think, that we should focus on interface semantics right now.
> If we can't agree on how the things should work, it makes no sense
> to discuss the implementation.
>

Yes, I have urged that we consider the interface in both the
memory.oom_group discussion as well as the discussion here, which is why
this patchset removes the mount option, does not lock down the entire
hierarchy into a single policy, and is extensible to be generally useful
outside of very special usecases.