On Fri, 13 Jul 2018, Roman Gushchin wrote: > > No objection, of course, this was always the mechanism vs policy > > separation that I was referring to. Having the ability to kill all > > processes attached to the cgroup when one of its processes is selected is > > useful, and we have our own patches that do just that, with the exception > > that it's triggerable by the user. > > Perfect! I'll prepare the patchset. > I mean, I separated it out completely in my own https://marc.info/?l=linux-kernel&m=152175564704473 as part of a patch series that completely fixes all of the issues with the cgroup aware oom killer, so of course there's no objection to separating it out. > > > > One of the things that I really like about cgroup v2, though, is what > > appears to be an implicit, but rather apparent, goal to minimize the > > number of files for each controller. It's very clean. So I'd suggest > > that we consider memory.group_oom, or however it is named, to allow for > > future development. > > > > For example, rather than simply being binary, we'd probably want the > > ability to kill all eligible processes attached directly to the victim's > > mem cgroup *or* all processes attached to its subtree as well. > > > > I'd suggest it be implemented to accept a string, "default"/"process", > > "local" or "tree"/"hierarchy", or better names, to define the group oom > > mechanism for the mem cgroup that is oom when one of its processes is > > selected as a victim. > > I would prefer to keep it boolean to match the simplicity of cgroup v2 API. > In v2 hierarchy processes can't be attached to non-leaf cgroups, > so I don't see the place for the 3rd meaning. > 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.
