cgroup-aware OOM killer

Traditionally, the OOM killer is operating on a process level. Under oom conditions, it finds a process with the highest oom score and kills it.

This behavior doesn't suit well the system with many running containers:

1) There is no fairness between containers. A small container with few large processes will be chosen over a large one with huge number of small processes.

2) Containers often do not expect that some random process inside will be killed. In many cases much safer behavior is to kill all tasks in the container. Traditionally, this was implemented in userspace, but doing it in the kernel has some advantages, especially in a case of a system-wide OOM.

To address these issues, the cgroup-aware OOM killer is introduced.

This patch introduces the core functionality: an ability to select a memory cgroup as an OOM victim. Under OOM conditions the OOM killer looks for the biggest leaf memory cgroup and kills the biggest task belonging to it.

The following patches will extend this functionality to consider non-leaf memory cgroups as OOM victims, and also provide an ability to kill all tasks belonging to the victim cgroup.

The root cgroup is treated as a leaf memory cgroup, so it's score is compared with other leaf memory cgroups.
Due to memcg statistics implementation a special approximation is used for estimating oom_score of root memory cgroup: we sum oom_score of the belonging processes (or, to be more precise, tasks owning their mm structures).

Signed-off-by Roman Gushchin <[email protected]>

Acked-by Michal Hocko <[email protected]>

mm/oom_kill.c +8 -1

@@ -1080,27 +1102,39 @@ bool out_of_memory(struct oom_control *oc)
 	    current->mm && !oom_unkillable_task(current, NULL, oc->nodemask) &&
 	    current->signal->oom_score_adj != OOM_SCORE_ADJ_MIN) {
 		get_task_struct(current);
-		oc->chosen = current;
+		oc->chosen_task = current;
 		oom_kill_process(oc, "Out of memory (oom_kill_allocating_task)");
 		return true;
 	}
 
+	if (mem_cgroup_select_oom_victim(oc)) {
+		if (oom_kill_memcg_victim(oc))
+		    delay = true;
+
+		goto out;
+	}
+

mm/memcontrol.c +9 -0

+
+static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
+{
+       struct mem_cgroup *iter;
+
+       oc->chosen_memcg = NULL;
+       oc->chosen_points = 0;
+
+       /*

mm/memcontrol.c +29 -0

+               score = oom_evaluate_memcg(iter, oc->nodemask, oc->totalpages);
+
+               /*
+                * Ignore empty and non-eligible memory cgroups.
+                */
+               if (score == 0)
+                       continue;
+
+               /*
+                * If there are inflight OOM victims, we don't need
+                * to look further for new victims.
+                */
+               if (score == -1) {
+                       oc->chosen_memcg = INFLIGHT_VICTIM;
+                       mem_cgroup_iter_break(root, iter);
+                       break;
+               }
+
+               if (score > oc->chosen_points) {
+                       oc->chosen_points = score;
+                       oc->chosen_memcg = iter;
+               }
+       }
+
+       if (oc->chosen_memcg && oc->chosen_memcg != INFLIGHT_VICTIM)
+               css_get(&oc->chosen_memcg->css);
+
+       rcu_read_unlock();
+}

Quoted 18 lines

mm/memcontrol.c +18 -0

+
+static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
+{
+       struct mem_cgroup *iter;
+
+       oc->chosen_memcg = NULL;
+       oc->chosen_points = 0;
+
+       /*
+        * The oom_score is calculated for leaf memory cgroups (including
+        * the root memcg).
+        */
+       rcu_read_lock();
+       for_each_mem_cgroup_tree(iter, root) {
+               long score;
+
+               if (memcg_has_children(iter) && iter != root_mem_cgroup)
+                       continue;

Quoted 1 line

+

Cgroup v2 does not support charge migration between memcgs. So, there can be intermediate nodes which may contain the major charge of the processes in their leave descendents. Skipping such intermediate nodes will kind of protect such processes from oom-killer (lower on the list to be killed). Is it ok to not handle such scenario? If yes, shouldn't we document it?

Quoted 18 lines · Shakeel Butt

Quoted 18 lines

mm/memcontrol.c +18 -0

+
+static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
+{
+       struct mem_cgroup *iter;
+
+       oc->chosen_memcg = NULL;
+       oc->chosen_points = 0;
+
+       /*
+        * The oom_score is calculated for leaf memory cgroups (including
+        * the root memcg).
+        */
+       rcu_read_lock();
+       for_each_mem_cgroup_tree(iter, root) {
+               long score;
+
+               if (memcg_has_children(iter) && iter != root_mem_cgroup)
+                       continue;

Quoted 8 lines

Quoted 1 line

+

Cgroup v2 does not support charge migration between memcgs. So, there can be intermediate nodes which may contain the major charge of the processes in their leave descendents. Skipping such intermediate nodes will kind of protect such processes from oom-killer (lower on the list to be killed). Is it ok to not handle such scenario? If yes, shouldn't we document it?

Yes, this is a real problem and the one which is not really solvable without the charge migration. You simply have no clue _who_ owns the memory so I assume that admins will need to setup the hierarchy which allows subgroups to migrate tasks to be oom_group.

Quoted 28 lines · Michal Hocko

Quoted 18 lines · Shakeel Butt

Quoted 18 lines

mm/memcontrol.c +18 -0

+
+static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
+{
+       struct mem_cgroup *iter;
+
+       oc->chosen_memcg = NULL;
+       oc->chosen_points = 0;
+
+       /*
+        * The oom_score is calculated for leaf memory cgroups (including
+        * the root memcg).
+        */
+       rcu_read_lock();
+       for_each_mem_cgroup_tree(iter, root) {
+               long score;
+
+               if (memcg_has_children(iter) && iter != root_mem_cgroup)
+                       continue;

Quoted 8 lines

Quoted 1 line

+

Cgroup v2 does not support charge migration between memcgs. So, there can be intermediate nodes which may contain the major charge of the processes in their leave descendents. Skipping such intermediate nodes will kind of protect such processes from oom-killer (lower on the list to be killed). Is it ok to not handle such scenario? If yes, shouldn't we document it?

Quoted 4 lines

Yes, this is a real problem and the one which is not really solvable without the charge migration. You simply have no clue _who_ owns the memory so I assume that admins will need to setup the hierarchy which allows subgroups to migrate tasks to be oom_group.

Hmm, scratch that. I have completely missed that the memory controller disables tasks migration completely in v2. I thought the standard restriction about the write access to the target cgroup and a common ancestor holds for all controllers but now I've noticed that we simply disallow the migration altogether. This wasn't the case before 1f7dd3e5a6e4 ("cgroup: fix handling of multi-destination migration from subtree_control enabling") which I wasn't aware of.

Quoted 18 lines

mm/memcontrol.c +18 -0

+
+static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
+{
+       struct mem_cgroup *iter;
+
+       oc->chosen_memcg = NULL;
+       oc->chosen_points = 0;
+
+       /*
+        * The oom_score is calculated for leaf memory cgroups (including
+        * the root memcg).
+        */
+       rcu_read_lock();
+       for_each_mem_cgroup_tree(iter, root) {
+               long score;
+
+               if (memcg_has_children(iter) && iter != root_mem_cgroup)
+                       continue;

Quoted 1 line

+

Cgroup v2 does not support charge migration between memcgs. So, there can be intermediate nodes which may contain the major charge of the processes in their leave descendents. Skipping such intermediate nodes will kind of protect such processes from oom-killer (lower on the list to be killed). Is it ok to not handle such scenario? If yes, shouldn't we document it?

Quoted 18 lines · Shakeel Butt

Quoted 18 lines

mm/memcontrol.c +18 -0

+
+static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
+{
+       struct mem_cgroup *iter;
+
+       oc->chosen_memcg = NULL;
+       oc->chosen_points = 0;
+
+       /*
+        * The oom_score is calculated for leaf memory cgroups (including
+        * the root memcg).
+        */
+       rcu_read_lock();
+       for_each_mem_cgroup_tree(iter, root) {
+               long score;
+
+               if (memcg_has_children(iter) && iter != root_mem_cgroup)
+                       continue;

Quoted 8 lines

Quoted 1 line

+

Cgroup v2 does not support charge migration between memcgs. So, there can be intermediate nodes which may contain the major charge of the processes in their leave descendents. Skipping such intermediate nodes will kind of protect such processes from oom-killer (lower on the list to be killed). Is it ok to not handle such scenario? If yes, shouldn't we document it?

Tasks cannot be in intermediate nodes, so the only way you can end up in a situation like this is to start tasks fully, let them fault in their full workingset, then create child groups and move them there.

That has attribution problems much wider than the OOM killer: any local limits you would set on a leaf cgroup like this ALSO won't control the memory of its tasks - as it's all sitting in the parent.

We created the "no internal competition" rule exactly to prevent this situation.

To be consistent with that rule, we might want to disallow the creation of child groups once a cgroup has local memory charges.

It's trivial to change the setup sequence to create the leaf cgroup first, then launch the workload from within.

Either way, this is nothing specific about the OOM killer.

Quoted 32 lines · Johannes Weiner

Quoted 18 lines · Shakeel Butt

Quoted 18 lines

mm/memcontrol.c +18 -0

+
+static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
+{
+       struct mem_cgroup *iter;
+
+       oc->chosen_memcg = NULL;
+       oc->chosen_points = 0;
+
+       /*
+        * The oom_score is calculated for leaf memory cgroups (including
+        * the root memcg).
+        */
+       rcu_read_lock();
+       for_each_mem_cgroup_tree(iter, root) {
+               long score;
+
+               if (memcg_has_children(iter) && iter != root_mem_cgroup)
+                       continue;

Quoted 8 lines

Quoted 1 line

+

Cgroup v2 does not support charge migration between memcgs. So, there can be intermediate nodes which may contain the major charge of the processes in their leave descendents. Skipping such intermediate nodes will kind of protect such processes from oom-killer (lower on the list to be killed). Is it ok to not handle such scenario? If yes, shouldn't we document it?

Tasks cannot be in intermediate nodes, so the only way you can end up in a situation like this is to start tasks fully, let them fault in their full workingset, then create child groups and move them there.

Quoted 6 lines

That has attribution problems much wider than the OOM killer: any local limits you would set on a leaf cgroup like this ALSO won't control the memory of its tasks - as it's all sitting in the parent.

We created the "no internal competition" rule exactly to prevent this situation.

Rather than the "no internal competition" restriction I think "charge migration" would have resolved that situation? Also "no internal competition" restriction (I am assuming 'no internal competition' is no tasks in internal nodes, please correct me if I am wrong) has made "charge migration" hard to implement and thus not added in cgroup v2.

I know this is parallel discussion and excuse my ignorance, what are other reasons behind "no internal competition" specifically for memory controller?

This patchset makes the OOM killer cgroup-aware.

Quoted 1 line · Roman Gushchin

This patchset makes the OOM killer cgroup-aware.

Hi Andrew,

I believe this code is ready for merging upstream, and it seems Michal is in agreement. There are two main things to consider, however.

David would have really liked for this patchset to include knobs to influence how the algorithm picks cgroup victims. The rest of us agreed that this is beyond the scope of these patches, that the patches don't need it to be useful, and that there is nothing preventing anyone from adding configurability later on. David subsequently nacked the series as he considers it incomplete. Neither Michal nor I see technical merit in David's nack.

Michal acked the implementation, but on the condition that the new behavior be opt-in, to not surprise existing users.

I *think* we agree
that respecting the cgroup topography during global OOM is what we should have been doing when cgroups were initially introduced;

where
we disagree is that I think users shouldn't have to opt in to improvements. We have done much more invasive changes to the victim selection without actual regressions in the past. Further, this change only applies to mounts of the new cgroup2.

Tejun also wasn't convinced
of the risk for regression, and too would prefer cgroup-awareness to be the default in cgroup2. I would ask for patch 5/6 to be dropped.

David would have really liked for this patchset to include knobs to influence how the algorithm picks cgroup victims. The rest of us agreed that this is beyond the scope of these patches, that the patches don't need it to be useful, and that there is nothing preventing anyone from adding configurability later on. David subsequently nacked the series as he considers it incomplete. Neither Michal nor I see technical merit in David's nack.

Michal acked the implementation, but on the condition that the new behavior be opt-in, to not surprise existing users. I *think* we agree that respecting the cgroup topography during global OOM is what we should have been doing when cgroups were initially introduced; where we disagree is that I think users shouldn't have to opt in to improvements. We have done much more invasive changes to the victim selection without actual regressions in the past. Further, this change only applies to mounts of the new cgroup2. Tejun also wasn't convinced of the risk for regression, and too would prefer cgroup-awareness to be the default in cgroup2. I would ask for patch 5/6 to be dropped.

Quoted 8 lines · Johannes Weiner

David would have really liked for this patchset to include knobs to influence how the algorithm picks cgroup victims. The rest of us agreed that this is beyond the scope of these patches, that the patches don't need it to be useful, and that there is nothing preventing anyone from adding configurability later on. David subsequently nacked the series as he considers it incomplete. Neither Michal nor I see technical merit in David's nack.

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

(3) the inability of userspace to effectively control oom victim selection.

Quoted 11 lines · David Rientjes

Quoted 8 lines · Johannes Weiner

David would have really liked for this patchset to include knobs to influence how the algorithm picks cgroup victims. The rest of us agreed that this is beyond the scope of these patches, that the patches don't need it to be useful, and that there is nothing preventing anyone from adding configurability later on. David subsequently nacked the series as he considers it incomplete. Neither Michal nor I see technical merit in David's nack.

Quoted 4 lines

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

Most users who are going to use this feature right now will have most of the userspace in their containers rather than in the root memcg. The root memcg will always be special and as such there will never be a universal best way to handle it. We should to satisfy most of usecases. I would consider this something that is an open for a further discussion but nothing that should stand in the way.

Quoted 3 lines

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

This doesn't differ from the current state where a task can purposefully or unpurposefully hide itself from the global memory killer by spawning new processes.

Quoted 2 lines

(3) the inability of userspace to effectively control oom victim selection.

this is not requested by the current usecase and it has been pointed out that this will be possible to implement on top of the foundation of this patchset.

Quoted 8 lines · Johannes Weiner

David would have really liked for this patchset to include knobs to influence how the algorithm picks cgroup victims. The rest of us agreed that this is beyond the scope of these patches, that the patches don't need it to be useful, and that there is nothing preventing anyone from adding configurability later on. David subsequently nacked the series as he considers it incomplete. Neither Michal nor I see technical merit in David's nack.

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

(3) the inability of userspace to effectively control oom victim selection.

Quoted 9 lines

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

Quoted 2 lines

(3) the inability of userspace to effectively control oom victim selection.

My apologies if my summary was too reductionist.

That being said, the arguments you repeat here have come up in previous threads and been responded to. This doesn't change my conclusion that your NAK is bogus.

Quoted 9 lines · Johannes Weiner

Quoted 9 lines

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

Quoted 9 lines

Quoted 2 lines

(3) the inability of userspace to effectively control oom victim selection.

My apologies if my summary was too reductionist.

That being said, the arguments you repeat here have come up in previous threads and been responded to. This doesn't change my conclusion that your NAK is bogus.

They actually haven't been responded to, Roman was working through v11 and made a change on how the root mem cgroup usage was calculated that was better than previous iterations but still not an apples to apples comparison with other cgroups. The problem is that it the calculation for leaf cgroups includes additional memory classes, so it biases against processes that are moved to non-root mem cgroups. Simply creating mem cgroups and attaching processes should not independently cause them to become more preferred: it should be a fair comparison between the root mem cgroup and the set of leaf mem cgroups as implemented. That is very trivial to do with hierarchical oom cgroup scoring.

Since the ability of userspace to control oom victim selection is not addressed whatsoever by this patchset, and the suggested method cannot be implemented on top of this patchset as you have argued because it requires a change to the heuristic itself, the patchset needs to become complete before being mergeable.

The thing is that the hierarchical approach (as in v8), which are you pushing, has it's own limitations, which we've discussed in details earlier. There are reasons why v12 is different, and we can't really simple go back. I mean if there are better ideas how to resolve concerns raised in discussions around v8, let me know, but ignoring them is not an option.

Quoted 6 lines · Roman Gushchin

The thing is that the hierarchical approach (as in v8), which are you pushing, has it's own limitations, which we've discussed in details earlier. There are reasons why v12 is different, and we can't really simple go back. I mean if there are better ideas how to resolve concerns raised in discussions around v8, let me know, but ignoring them is not an option.

I'm not ignoring them, I have stated that we need the ability to protect important cgroups on the system without oom disabling all attached processes. If that is implemented as a memory.oom_score_adj with the same semantics as /proc/pid/oom_score_adj, i.e. a proportion of available memory (the limit), it can also address the issues pointed out with the hierarchical approach in v8.

If this is not the case, could you elaborate
on what your exact concern is and why we do not care that users can completely circumvent victim selection by creating child cgroups for other controllers?

Since the ability to protect important cgroups on the system may require a heuristic change, I think it should be solved now rather than constantly insisting that we can make this patchset complete later and in the meantime force the user to set all attached processes to be oom disabled.

Quoted 6 lines

I'm not ignoring them, I have stated that we need the ability to protect important cgroups on the system without oom disabling all attached processes. If that is implemented as a memory.oom_score_adj with the same semantics as /proc/pid/oom_score_adj, i.e. a proportion of available memory (the limit), it can also address the issues pointed out with the hierarchical approach in v8.

No it cannot and it would be a terrible interface to have as well. You do not want to permanently tune oom_score_adj to compensate for structural restrictions on the hierarchy.

I believe, and Roman has pointed that out as well already, that further improvements can be implemented without changing user visible behavior as and add-on. If you disagree then you better come with a solid proof that all of us wrong and reasonable semantic cannot be achieved that way.

Quoted 11 lines · Michal Hocko

Quoted 6 lines

I'm not ignoring them, I have stated that we need the ability to protect important cgroups on the system without oom disabling all attached processes. If that is implemented as a memory.oom_score_adj with the same semantics as /proc/pid/oom_score_adj, i.e. a proportion of available memory (the limit), it can also address the issues pointed out with the hierarchical approach in v8.

No it cannot and it would be a terrible interface to have as well. You do not want to permanently tune oom_score_adj to compensate for structural restrictions on the hierarchy.

memory.oom_score_adj would never need to be permanently tuned, just as /proc/pid/oom_score_adj need never be permanently tuned. My response was an answer to Roman's concern that "v8 has it's own limitations," but I haven't seen a concrete example where the oom killer is forced to kill from the non-preferred cgroup while the user has power of biasing against certain cgroups with memory.oom_score_adj. Do you have such a concrete example that we can work with?

Quoted 5 lines

I believe, and Roman has pointed that out as well already, that further improvements can be implemented without changing user visible behavior as and add-on. If you disagree then you better come with a solid proof that all of us wrong and reasonable semantic cannot be achieved that way.

We simply cannot determine if improvements can be implemented in the future without user-visible changes if those improvements are unknown or undecided at this time.

It may require hierarchical accounting when
making a choice between siblings, as suggested with oom_score_adj. The only thing that we need to agree on is that userspace needs to have some kind of influence over victim selection: the oom killer killing an important user process is an extremely sensitive thing.

If the patchset
lacks the ability to have that influence, and such an ability would impact the heuristic overall, it's better to introduce that together as a complete patchset rather than merging an incomplete feature when it's known the user needs some control, asking the user to workaround it by setting all processes to oom disabled in a preferred mem cgroup, and then changing the heuristic again.

Quoted 7 lines

memory.oom_score_adj would never need to be permanently tuned, just as /proc/pid/oom_score_adj need never be permanently tuned. My response was an answer to Roman's concern that "v8 has it's own limitations," but I haven't seen a concrete example where the oom killer is forced to kill from the non-preferred cgroup while the user has power of biasing against certain cgroups with memory.oom_score_adj. Do you have such a concrete example that we can work with?

Yes, the one with structural requirements due to other controllers or due to general organizational purposes where hierarchical (sibling oriented) comparison just doesn't work.

Take the students, teachers,
admins example. You definitely do not want to kill from students subgroups by default just because this is the largest entity type. Tuning memory.oom_score_adj doesn't work for that usecase as soon as new subgroups come and go.

Quoted 3 lines

We simply cannot determine if improvements can be implemented in the future without user-visible changes if those improvements are unknown or undecided at this time.

Come on. There have been at least two examples on how this could be achieved. One priority based which would use cumulative memory consumption if set on intermediate nodes which would allow you to compare siblings. And another one was to add a new knob which would make an intermediate node an aggregate for accounting purposes.

And I am pretty sure we have already agreed that something like this is useful for some usecases and nobody objected this would get merged in future. All we are saying now is that this is not in scope of _this_ patchseries because the vast majority of usecases simply do not care about influencing the oom selection. They only do care about having per cgroup behavior and/or kill all semantic. I really do not understand what is hard about that.

Quoted 9 lines · Johannes Weiner

Quoted 9 lines

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

Quoted 8 lines

Quoted 2 lines

(3) the inability of userspace to effectively control oom victim selection.

My apologies if my summary was too reductionist.

That being said, the arguments you repeat here have come up in previous threads and been responded to. This doesn't change my conclusion that your NAK is bogus.

They actually haven't been responded to, Roman was working through v11 and made a change on how the root mem cgroup usage was calculated that was better than previous iterations but still not an apples to apples comparison with other cgroups. The problem is that it the calculation for leaf cgroups includes additional memory classes, so it biases against processes that are moved to non-root mem cgroups. Simply creating mem cgroups and attaching processes should not independently cause them to become more preferred: it should be a fair comparison between the root mem cgroup and the set of leaf mem cgroups as implemented. That is very trivial to do with hierarchical oom cgroup scoring.

Since the ability of userspace to control oom victim selection is not addressed whatsoever by this patchset, and the suggested method cannot be implemented on top of this patchset as you have argued because it requires a change to the heuristic itself, the patchset needs to become complete before being mergeable.

Quoted 18 lines · David Rientjes

Quoted 9 lines · Johannes Weiner

Quoted 9 lines

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

Quoted 7 lines

Quoted 2 lines

(3) the inability of userspace to effectively control oom victim selection.

My apologies if my summary was too reductionist.

That being said, the arguments you repeat here have come up in previous threads and been responded to. This doesn't change my conclusion that your NAK is bogus.

Quoted 10 lines

They actually haven't been responded to, Roman was working through v11 and made a change on how the root mem cgroup usage was calculated that was better than previous iterations but still not an apples to apples comparison with other cgroups. The problem is that it the calculation for leaf cgroups includes additional memory classes, so it biases against processes that are moved to non-root mem cgroups. Simply creating mem cgroups and attaching processes should not independently cause them to become more preferred: it should be a fair comparison between the root mem cgroup and the set of leaf mem cgroups as implemented. That is very trivial to do with hierarchical oom cgroup scoring.

There is absolutely no value in your repeating the same stuff over and over again without considering what other people are telling you.

Hierarchical oom scoring has other downsides, and most of us agree that they aren't preferable over the differences in scoring the root vs scoring other cgroups - in particular because the root cannot be controlled, doesn't even have local statistics, and so is unlikely to contain important work on a containerized system. Getting the ballpark right for the vast majority of usecases is more than good enough here.

Quoted 5 lines

Since the ability of userspace to control oom victim selection is not addressed whatsoever by this patchset, and the suggested method cannot be implemented on top of this patchset as you have argued because it requires a change to the heuristic itself, the patchset needs to become complete before being mergeable.

It is complete. It just isn't a drop-in replacement for what you've been doing out-of-tree for years. Stop making your problem everybody else's problem.

You can change the the heuristics later, as you have done before. Or you can add another configuration flag and we can phase out the old mode, like we do all the time.

Quoted 48 lines · Johannes Weiner

Quoted 18 lines · David Rientjes

Quoted 9 lines · Johannes Weiner

Quoted 9 lines

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

Quoted 7 lines

Quoted 2 lines

(3) the inability of userspace to effectively control oom victim selection.

My apologies if my summary was too reductionist.

That being said, the arguments you repeat here have come up in previous threads and been responded to. This doesn't change my conclusion that your NAK is bogus.

Quoted 10 lines

They actually haven't been responded to, Roman was working through v11 and made a change on how the root mem cgroup usage was calculated that was better than previous iterations but still not an apples to apples comparison with other cgroups. The problem is that it the calculation for leaf cgroups includes additional memory classes, so it biases against processes that are moved to non-root mem cgroups. Simply creating mem cgroups and attaching processes should not independently cause them to become more preferred: it should be a fair comparison between the root mem cgroup and the set of leaf mem cgroups as implemented. That is very trivial to do with hierarchical oom cgroup scoring.

There is absolutely no value in your repeating the same stuff over and over again without considering what other people are telling you.

Hierarchical oom scoring has other downsides, and most of us agree that they aren't preferable over the differences in scoring the root vs scoring other cgroups - in particular because the root cannot be controlled, doesn't even have local statistics, and so is unlikely to contain important work on a containerized system. Getting the ballpark right for the vast majority of usecases is more than good enough here.

Quoted 5 lines

Since the ability of userspace to control oom victim selection is not addressed whatsoever by this patchset, and the suggested method cannot be implemented on top of this patchset as you have argued because it requires a change to the heuristic itself, the patchset needs to become complete before being mergeable.

It is complete. It just isn't a drop-in replacement for what you've been doing out-of-tree for years. Stop making your problem everybody else's problem.

Quoted 4 lines

You can change the the heuristics later, as you have done before. Or you can add another configuration flag and we can phase out the old mode, like we do all the time.

I think this problem is related to the removal of the lowmemorykiller, where this is the life-line when the user-space for some reason fails.

So I guess quite a few will have this problem.

Quoted 56 lines · peter enderborg

Quoted 44 lines · Johannes Weiner

Quoted 18 lines · David Rientjes

Quoted 9 lines · Johannes Weiner

Quoted 9 lines

The nack is for three reasons:

(1) unfair comparison of root mem cgroup usage to bias against that mem cgroup from oom kill in system oom conditions,

(2) the ability of users to completely evade the oom killer by attaching all processes to child cgroups either purposefully or unpurposefully, and

Quoted 7 lines

Quoted 2 lines

(3) the inability of userspace to effectively control oom victim selection.

My apologies if my summary was too reductionist.

That being said, the arguments you repeat here have come up in previous threads and been responded to. This doesn't change my conclusion that your NAK is bogus.

Quoted 10 lines

They actually haven't been responded to, Roman was working through v11 and made a change on how the root mem cgroup usage was calculated that was better than previous iterations but still not an apples to apples comparison with other cgroups. The problem is that it the calculation for leaf cgroups includes additional memory classes, so it biases against processes that are moved to non-root mem cgroups. Simply creating mem cgroups and attaching processes should not independently cause them to become more preferred: it should be a fair comparison between the root mem cgroup and the set of leaf mem cgroups as implemented. That is very trivial to do with hierarchical oom cgroup scoring.

There is absolutely no value in your repeating the same stuff over and over again without considering what other people are telling you.

Hierarchical oom scoring has other downsides, and most of us agree that they aren't preferable over the differences in scoring the root vs scoring other cgroups - in particular because the root cannot be controlled, doesn't even have local statistics, and so is unlikely to contain important work on a containerized system. Getting the ballpark right for the vast majority of usecases is more than good enough here.

Quoted 5 lines

Since the ability of userspace to control oom victim selection is not addressed whatsoever by this patchset, and the suggested method cannot be implemented on top of this patchset as you have argued because it requires a change to the heuristic itself, the patchset needs to become complete before being mergeable.

Quoted 8 lines

It is complete. It just isn't a drop-in replacement for what you've been doing out-of-tree for years. Stop making your problem everybody else's problem.

You can change the the heuristics later, as you have done before. Or you can add another configuration flag and we can phase out the old mode, like we do all the time.

I think this problem is related to the removal of the lowmemorykiller, where this is the life-line when the user-space for some reason fails.

Quoted 1 line

So I guess quite a few will have this problem.

Could you be more specific please? We are _not_ removing possibility of the user space influenced oom victim selection. You can still use the _current_ oom selection heuristic. The patch adds a new selection method which is opt-in so only those who want to opt-in will not be allowed to have any influence on the victim selection. And as it has been pointed out this can be implemented later so it is not like "this won't be possible anymore in future"