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.
