cgroup-aware OOM killer

The oom_kill_process() function consists of two logical parts:
the first one is responsible for considering task's children as a potential victim and printing the debug information. The second half is responsible for sending SIGKILL to all tasks sharing the mm struct with the given victim.

This commit splits the oom_kill_process() function with an intention to re-use the the second half: __oom_kill_process().

The cgroup-aware OOM killer will kill multiple tasks belonging to the victim cgroup. We don't need to print the debug information for the each task, as well as play with task selection (considering task's children), so we can't use the existing oom_kill_process().

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

Cc Michal Hocko <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Johannes Weiner <[email protected]>

Cc Tetsuo Handa <[email protected]>

Cc David Rientjes <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Tejun Heo <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

The oom_kill_process() function consists of two logical parts:
the first one is responsible for considering task's children as a potential victim and printing the debug information. The second half is responsible for sending SIGKILL to all tasks sharing the mm struct with the given victim.

This commit splits the oom_kill_process() function with an intention to re-use the the second half: __oom_kill_process().

The cgroup-aware OOM killer will kill multiple tasks belonging to the victim cgroup. We don't need to print the debug information for the each task, as well as play with task selection (considering task's children), so we can't use the existing oom_kill_process().

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

Cc Michal Hocko <[email protected]>

Cc Vladimir Davydov <[email protected]>

Cc Johannes Weiner <[email protected]>

Cc Tetsuo Handa <[email protected]>

Cc David Rientjes <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Tejun Heo <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

Cc [email protected]

---
 mm/oom_kill.c | 123 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 65 insertions(+), 58 deletions(-)

mm/oom_kill.c +65 -58

index 99736e026712..f061b627092c 100644
@@ -804,68 +804,12 @@ static bool task_will_free_mem(struct task_struct *task)
 	return ret;
 }
 
-static void oom_kill_process(struct oom_control *oc, const char *message)
+static void __oom_kill_process(struct task_struct *victim)
 {
-	struct task_struct *p = oc->chosen;
-	unsigned int points = oc->chosen_points;
-	struct task_struct *victim = p;
-	struct task_struct *child;
-	struct task_struct *t;
+	struct task_struct *p;
 	struct mm_struct *mm;
-	unsigned int victim_points = 0;
-	static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
-					      DEFAULT_RATELIMIT_BURST);
 	bool can_oom_reap = true;
 
-	/*
-	 * If the task is already exiting, don't alarm the sysadmin or kill
-	 * its children or threads, just give it access to memory reserves
-	 * so it can die quickly
-	 */
-	task_lock(p);
-	if (task_will_free_mem(p)) {
-		mark_oom_victim(p);
-		wake_oom_reaper(p);
-		task_unlock(p);
-		put_task_struct(p);
-		return;
-	}
-	task_unlock(p);
-
-	if (__ratelimit(&oom_rs))
-		dump_header(oc, p);
-
-	pr_err("%s: Kill process %d (%s) score %u or sacrifice child\n",
-		message, task_pid_nr(p), p->comm, points);
-
-	/*
-	 * If any of p's children has a different mm and is eligible for kill,
-	 * the one with the highest oom_badness() score is sacrificed for its
-	 * parent.  This attempts to lose the minimal amount of work done while
-	 * still freeing memory.
-	 */
-	read_lock(&tasklist_lock);
-	for_each_thread(p, t) {
-		list_for_each_entry(child, &t->children, sibling) {
-			unsigned int child_points;
-
-			if (process_shares_mm(child, p->mm))
-				continue;
-			/*
-			 * oom_badness() returns 0 if the thread is unkillable
-			 */
-			child_points = oom_badness(child,
-				oc->memcg, oc->nodemask, oc->totalpages);
-			if (child_points > victim_points) {
-				put_task_struct(victim);
-				victim = child;
-				victim_points = child_points;
-				get_task_struct(victim);
-			}
-		}
-	}
-	read_unlock(&tasklist_lock);
-
 	p = find_lock_task_mm(victim);
 	if (!p) {
 		put_task_struct(victim);

@@ -939,6 +883,69 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
 }
 #undef K
 
+static void oom_kill_process(struct oom_control *oc, const char *message)
+{
+	struct task_struct *p = oc->chosen;
+	unsigned int points = oc->chosen_points;
+	struct task_struct *victim = p;
+	struct task_struct *child;
+	struct task_struct *t;
+	unsigned int victim_points = 0;
+	static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
+					      DEFAULT_RATELIMIT_BURST);
+
+	/*
+	 * If the task is already exiting, don't alarm the sysadmin or kill
+	 * its children or threads, just give it access to memory reserves
+	 * so it can die quickly
+	 */
+	task_lock(p);
+	if (task_will_free_mem(p)) {
+		mark_oom_victim(p);
+		wake_oom_reaper(p);
+		task_unlock(p);
+		put_task_struct(p);
+		return;
+	}
+	task_unlock(p);
+
+	if (__ratelimit(&oom_rs))
+		dump_header(oc, p);
+
+	pr_err("%s: Kill process %d (%s) score %u or sacrifice child\n",
+		message, task_pid_nr(p), p->comm, points);
+
+	/*
+	 * If any of p's children has a different mm and is eligible for kill,
+	 * the one with the highest oom_badness() score is sacrificed for its
+	 * parent.  This attempts to lose the minimal amount of work done while
+	 * still freeing memory.
+	 */
+	read_lock(&tasklist_lock);
+	for_each_thread(p, t) {
+		list_for_each_entry(child, &t->children, sibling) {
+			unsigned int child_points;
+
+			if (process_shares_mm(child, p->mm))
+				continue;
+			/*
+			 * oom_badness() returns 0 if the thread is unkillable
+			 */
+			child_points = oom_badness(child,
+				oc->memcg, oc->nodemask, oc->totalpages);
+			if (child_points > victim_points) {
+				put_task_struct(victim);
+				victim = child;
+				victim_points = child_points;
+				get_task_struct(victim);
+			}
+		}
+	}
+	read_unlock(&tasklist_lock);
+
+	__oom_kill_process(victim);
+}
+
 /*
  * Determines whether the kernel must panic because of the panic_on_oom sysctl.
  */

Signature · 1 line

2.13.5

mm/memcontrol.c +13 -0

index 15af3da5af02..da2b12ea4667 100644
@@ -2661,6 +2661,231 @@ static inline bool memcg_has_children(struct mem_cgroup *memcg)
 	return ret;
 }
 
+static long memcg_oom_badness(struct mem_cgroup *memcg,
+			      const nodemask_t *nodemask,
+			      unsigned long totalpages)
+{
+	long points = 0;
+	int nid;
+	pg_data_t *pgdat;
+
+	/*
+	 * We don't have necessary stats for the root memcg,
+	 * so we define it's oom_score as the maximum oom_score
+	 * of the belonging tasks.
+	 */

mm/memcontrol.c +26 -0

+				max_score = score;
+		}
+		css_task_iter_end(&it);
+
+		return max_score;
+	}
+
+	for_each_node_state(nid, N_MEMORY) {
+		if (nodemask && !node_isset(nid, *nodemask))
+			continue;
+
+		points += mem_cgroup_node_nr_lru_pages(memcg, nid,
+				LRU_ALL_ANON | BIT(LRU_UNEVICTABLE));
+
+		pgdat = NODE_DATA(nid);
+		points += lruvec_page_state(mem_cgroup_lruvec(pgdat, memcg),
+					    NR_SLAB_UNRECLAIMABLE);
+	}
+
+	points += memcg_page_state(memcg, MEMCG_KERNEL_STACK_KB) /
+		(PAGE_SIZE / 1024);
+	points += memcg_page_state(memcg, MEMCG_SOCK);
+	points += memcg_page_state(memcg, MEMCG_SWAP);
+
+	return points;
+}

Quoted 25 lines · Roman Gushchin

mm/memcontrol.c +17 -0

index 15af3da5af02..da2b12ea4667 100644
@@ -2661,6 +2661,231 @@ static inline bool memcg_has_children(struct mem_cgroup *memcg)
 	return ret;
 }
 
+static long memcg_oom_badness(struct mem_cgroup *memcg,
+			      const nodemask_t *nodemask,
+			      unsigned long totalpages)
+{
+	long points = 0;
+	int nid;
+	pg_data_t *pgdat;
+
+	/*
+	 * We don't have necessary stats for the root memcg,
+	 * so we define it's oom_score as the maximum oom_score
+	 * of the belonging tasks.
+	 */
+	if (memcg == root_mem_cgroup) {
+		struct css_task_iter it;
+		struct task_struct *task;
+		long score, max_score = 0;

mm/memcontrol.c static inline bool memcg_has_children(struct mem_cgroup *memcg) · line 2686

Quoted 6 lines

mm/memcontrol.c +6 -0

+
+		css_task_iter_start(&memcg->css, 0, &it);
+		while ((task = css_task_iter_next(&it))) {
+			score = oom_badness(task, memcg, nodemask,
+					    totalpages);
+			if (max_score > score)

score > max_score

Add a "groupoom" cgroup v2 mount option to enable the cgroup-aware OOM killer. If not set, the OOM selection is performed in a "traditional" per-process way.

The behavior can be changed dynamically by remounting the cgroupfs.

Quoted 5 lines · Roman Gushchin

Add a "groupoom" cgroup v2 mount option to enable the cgroup-aware OOM killer. If not set, the OOM selection is performed in a "traditional" per-process way.

The behavior can be changed dynamically by remounting the cgroupfs.

I can't imagine that Tejun would be happy with a new mount option, especially when it's not required.

OOM behavior does not need to be defined at mount time and for the entire hierarchy. It's possible to very easily implement a tunable as part of mem cgroup that is propagated to descendants and controls the oom scoring behavior for that hierarchy. It does not need to be system wide and affect scoring of all processes based on which mem cgroup they are attached to at any given time.

Quoted 9 lines

I can't imagine that Tejun would be happy with a new mount option, especially when it's not required.

OOM behavior does not need to be defined at mount time and for the entire hierarchy. It's possible to very easily implement a tunable as part of mem cgroup that is propagated to descendants and controls the oom scoring behavior for that hierarchy. It does not need to be system wide and affect scoring of all processes based on which mem cgroup they are attached to at any given time.

No, I don't think that mixing per-cgroup and per-process OOM selection algorithms is a good idea.

So, there are 3 reasonable options:
1) boot option
2) sysctl
3) cgroup mount option

I believe, 3) is better, because it allows changing the behavior dynamically, and explicitly depends on v2 (what sysctl lacks).

So, the only question is should it be opt-in or opt-out option. Personally, I would prefer opt-out, but Michal has a very strong opinion here.

Quoted 11 lines · David Rientjes

Quoted 5 lines · Roman Gushchin

Add a "groupoom" cgroup v2 mount option to enable the cgroup-aware OOM killer. If not set, the OOM selection is performed in a "traditional" per-process way.

The behavior can be changed dynamically by remounting the cgroupfs.

I can't imagine that Tejun would be happy with a new mount option, especially when it's not required.

Quoted 6 lines

OOM behavior does not need to be defined at mount time and for the entire hierarchy. It's possible to very easily implement a tunable as part of mem cgroup that is propagated to descendants and controls the oom scoring behavior for that hierarchy. It does not need to be system wide and affect scoring of all processes based on which mem cgroup they are attached to at any given time.

No, I don't think that mixing per-cgroup and per-process OOM selection algorithms is a good idea.

So, there are 3 reasonable options:
1) boot option
2) sysctl
3) cgroup mount option

I believe, 3) is better, because it allows changing the behavior dynamically, and explicitly depends on v2 (what sysctl lacks).

So, the only question is should it be opt-in or opt-out option. Personally, I would prefer opt-out, but Michal has a very strong opinion here.

This patchset makes the OOM killer cgroup-aware.

v8:
- Do not kill tasks with OOM_SCORE_ADJ -1000
- Make the whole thing opt-in with cgroup mount option control
- Drop oom_priority for further discussions

- Kill the whole cgroup if oom_group is set and it's memory.max is reached
- Update docs and commit messages

Quoted 2 lines · Roman Gushchin

This patchset makes the OOM killer cgroup-aware.

Quoted 4 lines

v8:
- Do not kill tasks with OOM_SCORE_ADJ -1000
- Make the whole thing opt-in with cgroup mount option control
- Drop oom_priority for further discussions

Nack, we specifically require oom_priority for this to function correctly, otherwise we cannot prefer to kill from low priority leaf memcgs as required.

Quoted 10 lines

Quoted 6 lines

This patchset makes the OOM killer cgroup-aware.

v8:
- Do not kill tasks with OOM_SCORE_ADJ -1000
- Make the whole thing opt-in with cgroup mount option control
- Drop oom_priority for further discussions

Nack, we specifically require oom_priority for this to function correctly, otherwise we cannot prefer to kill from low priority leaf memcgs as required.

While I understand that your usecase might require priorities I do not think this part missing is a reason to nack the cgroup based selection and kill-all parts. This can be done on top. The only important part right now is the current selection semantic - only leaf memcgs vs. size of the hierarchy). I strongly believe that comparing only leaf memcgs is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

Quoted 11 lines · Michal Hocko

Quoted 10 lines

Quoted 6 lines

This patchset makes the OOM killer cgroup-aware.

v8:
- Do not kill tasks with OOM_SCORE_ADJ -1000
- Make the whole thing opt-in with cgroup mount option control
- Drop oom_priority for further discussions

Nack, we specifically require oom_priority for this to function correctly, otherwise we cannot prefer to kill from low priority leaf memcgs as required.

Quoted 9 lines

While I understand that your usecase might require priorities I do not think this part missing is a reason to nack the cgroup based selection and kill-all parts. This can be done on top. The only important part right now is the current selection semantic - only leaf memcgs vs. size of the hierarchy). I strongly believe that comparing only leaf memcgs is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

The problem is that we cannot enable the cgroup-aware oom killer and oom_group behavior because, without oom priorities, we have no ability to influence the cgroup that it chooses. It is doing two things: providing more fairness amongst cgroups by selecting based on cumulative usage rather than single large process (good!), and effectively is removing all userspace control of oom selection (bad). We want the former, but it needs to be coupled with support so that we can protect vital cgroups, regardless of their usage.

It is certainly possible to add oom priorities on top before it is merged, but I don't see why it isn't part of the patchset.

We need it before its
merged to avoid users playing with /proc/pid/oom_score_adj to prevent any killing in the most preferable memcg when they could have simply changed the oom priority.

Quoted 2 lines

It is certainly possible to add oom priorities on top before it is merged, but I don't see why it isn't part of the patchset.

Because the semantic of the priority for non-leaf memcgs is not fully clear and I would rather have the core of the functionality merged before this is sorted out.

Quoted 4 lines

We need it before its
merged to avoid users playing with /proc/pid/oom_score_adj to prevent any killing in the most preferable memcg when they could have simply changed the oom priority.

I am sorry but I do not really understand your concern. Are you suggesting that users would start oom disable all tasks in a memcg to give it a higher priority? Even if that was the case why should such an abuse be a blocker for generic memcg aware oom killer being merged?

Quoted 4 lines · David Rientjes

Quoted 2 lines · Roman Gushchin

This patchset makes the OOM killer cgroup-aware.

Quoted 8 lines

Quoted 4 lines

v8:
- Do not kill tasks with OOM_SCORE_ADJ -1000
- Make the whole thing opt-in with cgroup mount option control
- Drop oom_priority for further discussions

Nack, we specifically require oom_priority for this to function correctly, otherwise we cannot prefer to kill from low priority leaf memcgs as required.

While I understand that your usecase might require priorities I do not think this part missing is a reason to nack the cgroup based selection and kill-all parts. This can be done on top. The only important part right now is the current selection semantic - only leaf memcgs vs. size of the hierarchy).

I strongly believe that comparing only leaf memcgs
is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

I didn't get to read the new version of this series yet and hope to get to it soon.

On Wed, Sep 13, 2017 at 02:29:14PM +0200, Michal Hocko wrote:

Quoted 4 lines

I strongly believe that comparing only leaf memcgs
is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

One of two main goals of this patchset is to introduce cgroup-level
fairness: bigger cgroups should be affected more than smaller, despite the size of tasks inside. I believe the same principle should be used for cgroups.

Quoted 1 line · Roman Gushchin

On Wed, Sep 13, 2017 at 02:29:14PM +0200, Michal Hocko wrote:

[...]

Quoted 9 lines

Quoted 4 lines

I strongly believe that comparing only leaf memcgs
is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

One of two main goals of this patchset is to introduce cgroup-level
fairness: bigger cgroups should be affected more than smaller, despite the size of tasks inside. I believe the same principle should be used for cgroups.

Yes bigger cgroups should be preferred but I fail to see why bigger hierarchies should be considered as well if they are not kill-all. And whether non-leaf memcgs should allow kill-all is not entirely clear to me. What would be the usecase?

Consider that it might be not your choice (as a user) how deep is your leaf memcg. I can already see how people complain that their memcg has been killed just because it was one level deeper in the hierarchy...

I would really start simple and only allow kill-all on leaf memcgs and only compare leaf memcgs & root. If we ever need to kill whole hierarchies then allow kill-all on intermediate memcgs as well and then consider cumulative consumptions only on those that have kill-all enabled.

Or do I miss any reasonable usecase that would suffer from such a semantic?

Quoted 13 lines · Michal Hocko

Quoted 1 line · Roman Gushchin

On Wed, Sep 13, 2017 at 02:29:14PM +0200, Michal Hocko wrote:

[...]

Quoted 9 lines

Quoted 4 lines

I strongly believe that comparing only leaf memcgs
is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

One of two main goals of this patchset is to introduce cgroup-level
fairness: bigger cgroups should be affected more than smaller, despite the size of tasks inside. I believe the same principle should be used for cgroups.

Quoted 4 lines

Yes bigger cgroups should be preferred but I fail to see why bigger hierarchies should be considered as well if they are not kill-all. And whether non-leaf memcgs should allow kill-all is not entirely clear to me. What would be the usecase?

We definitely want to support kill-all for non-leaf cgroups. A workload can consist of several cgroups and we want to clean up the whole thing on OOM.

I don't see any reasons to limit this functionality to leaf cgroups only.

Hierarchies are memory consumers, we do account their usage, we do apply limits and guarantees for the hierarchies. The same is with OOM victim selection: we are reclaiming memory from the biggest consumer. Kill-all knob only defines the way _how_ we do that:
by killing one or all processes.

Just for example, we might want to take memory.low into account at some point: prefer cgroups which are above their guarantees, avoid killing those who fit. It would be hard if we're comparing cgroups from different hierarchies.

The same will be with introducing oom_priorities, which is much more required functionality.

Quoted 3 lines

Consider that it might be not your choice (as a user) how deep is your leaf memcg. I can already see how people complain that their memcg has been killed just because it was one level deeper in the hierarchy...

The kill-all functionality is enforced by parent, and it seems to be following the overall memcg design. The parent cgroup enforces memory limit, memory low limit, etc.

I don't know why OOM control should be different.

Quoted 5 lines

I would really start simple and only allow kill-all on leaf memcgs and only compare leaf memcgs & root. If we ever need to kill whole hierarchies then allow kill-all on intermediate memcgs as well and then consider cumulative consumptions only on those that have kill-all enabled.

This sounds hacky to me: the whole thing is depending on cgroup v2 and is additionally explicitly opt-in.

Why do we need to introduce such incomplete functionality first, and then suffer trying to extend it and provide backward compatibility?

Also, I think we should compare root cgroup with top-level cgroups, rather than leaf cgroups. A process in the root cgroup is definitely system-level entity, and we should compare it with other top-level entities (other containerized workloads), rather then some random leaf cgroup deep inside the tree. If we decided, that we're not comparing random tasks from different cgroups, why should we do this for leaf cgroups? Is sounds like making only one step towards right direction, while we can do more.

Quoted 19 lines · Roman Gushchin

Quoted 17 lines · Michal Hocko

Quoted 1 line · Roman Gushchin

On Wed, Sep 13, 2017 at 02:29:14PM +0200, Michal Hocko wrote:

[...]

Quoted 9 lines

Quoted 4 lines

I strongly believe that comparing only leaf memcgs
is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

One of two main goals of this patchset is to introduce cgroup-level
fairness: bigger cgroups should be affected more than smaller, despite the size of tasks inside. I believe the same principle should be used for cgroups.

Yes bigger cgroups should be preferred but I fail to see why bigger hierarchies should be considered as well if they are not kill-all. And whether non-leaf memcgs should allow kill-all is not entirely clear to me. What would be the usecase?

Quoted 3 lines

We definitely want to support kill-all for non-leaf cgroups. A workload can consist of several cgroups and we want to clean up the whole thing on OOM.

Could you be more specific about such a workload? E.g. how can be such a hierarchy handled consistently when its sub-tree gets killed due to internal memory pressure?

Or do you expect that none of the subtree will
have hard limit configured?

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Quoted 14 lines · Roman Gushchin

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Yes, oom_priority cannot select the largest priority tree-wide for exactly that reason. We need the ability to control from which subtree the kill occurs in ancestor cgroups. If multiple jobs are allocated their own cgroups and they can own memory.oom_priority for their own subcontainers, this becomes quite powerful so they can define their own oom priorities. Otherwise, they can easily override the oom priorities of other cgroups.

Quoted 25 lines · David Rientjes

Quoted 14 lines · Roman Gushchin

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Quoted 6 lines

Yes, oom_priority cannot select the largest priority tree-wide for exactly that reason. We need the ability to control from which subtree the kill occurs in ancestor cgroups. If multiple jobs are allocated their own cgroups and they can own memory.oom_priority for their own subcontainers, this becomes quite powerful so they can define their own oom priorities. Otherwise, they can easily override the oom priorities of other cgroups.

I believe, it's a solvable problem: we can require CAP_SYS_RESOURCE to set the oom_priority below parent's value, or something like this.

But it looks more complex, and I'm not sure there are real examples, when we have to compare memcgs, which are on different levels (or in different subtrees).

Quoted 33 lines · Roman Gushchin

Quoted 31 lines · David Rientjes

Quoted 14 lines · Roman Gushchin

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Yes, oom_priority cannot select the largest priority tree-wide for exactly that reason. We need the ability to control from which subtree the kill occurs in ancestor cgroups. If multiple jobs are allocated their own cgroups and they can own memory.oom_priority for their own subcontainers, this becomes quite powerful so they can define their own oom priorities. Otherwise, they can easily override the oom priorities of other cgroups.

Quoted 2 lines

I believe, it's a solvable problem: we can require CAP_SYS_RESOURCE to set the oom_priority below parent's value, or something like this.

As said in other email. We can make priorities hierarchical (in the same sense as hard limit or others) so that children cannot override their parent.

Quoted 3 lines

As said in other email. We can make priorities hierarchical (in the same sense as hard limit or others) so that children cannot override their parent.

You mean they can set the knob to any value, but parent's value is enforced, if it's greater than child's value?

If so, this sounds logical to me. Then we have size-based comparison and priority-based comparison with similar rules, and all use cases are covered.

Ok, can we stick with this design?
Then I'll return oom_priorities in place, and post a (hopefully) final version.

Quoted 23 lines

Quoted 14 lines

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Quoted 6 lines

Yes, oom_priority cannot select the largest priority tree-wide for exactly that reason. We need the ability to control from which subtree the kill occurs in ancestor cgroups. If multiple jobs are allocated their own cgroups and they can own memory.oom_priority for their own subcontainers, this becomes quite powerful so they can define their own oom priorities. Otherwise, they can easily override the oom priorities of other cgroups.

I believe, it's a solvable problem: we can require CAP_SYS_RESOURCE to set the oom_priority below parent's value, or something like this.

But it looks more complex, and I'm not sure there are real examples, when we have to compare memcgs, which are on different levels (or in different subtrees).

Quoted 30 lines · Roman Gushchin

Quoted 23 lines

Quoted 14 lines

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Quoted 6 lines

Yes, oom_priority cannot select the largest priority tree-wide for exactly that reason. We need the ability to control from which subtree the kill occurs in ancestor cgroups. If multiple jobs are allocated their own cgroups and they can own memory.oom_priority for their own subcontainers, this becomes quite powerful so they can define their own oom priorities. Otherwise, they can easily override the oom priorities of other cgroups.

Quoted 7 lines

I believe, it's a solvable problem: we can require CAP_SYS_RESOURCE to set the oom_priority below parent's value, or something like this.

But it looks more complex, and I'm not sure there are real examples, when we have to compare memcgs, which are on different levels (or in different subtrees).

It's actually much more complex because in our environment we'd need an "activity manager" with CAP_SYS_RESOURCE to control oom priorities of user subcontainers when today it need only be concerned with top-level memory cgroups. Users can create their own hierarchies with their own oom priorities at will, it doesn't alter the selection heuristic for another other user running on the same system and gives them full control over the selection in their own subtree. We shouldn't need to have a system-wide daemon with CAP_SYS_RESOURCE be required to manage subcontainers when nothing else requires it. I believe it's also much easier to document:
oom_priority is considered for all sibling cgroups at each level of the hierarchy and the cgroup with the lowest priority value gets iterated.

Quoted 11 lines

It's actually much more complex because in our environment we'd need an "activity manager" with CAP_SYS_RESOURCE to control oom priorities of user subcontainers when today it need only be concerned with top-level memory cgroups. Users can create their own hierarchies with their own oom priorities at will, it doesn't alter the selection heuristic for another other user running on the same system and gives them full control over the selection in their own subtree. We shouldn't need to have a system-wide daemon with CAP_SYS_RESOURCE be required to manage subcontainers when nothing else requires it. I believe it's also much easier to document:
oom_priority is considered for all sibling cgroups at each level of the hierarchy and the cgroup with the lowest priority value gets iterated.

I do agree actually. System-wide OOM priorities make no sense.

Always compare sibling cgroups, either by priority or size, seems to be simple, clear and powerful enough for all reasonable use cases. Am I right, that it's exactly what you've used internally? This is a perfect confirmation, I believe.

Quoted 14 lines · Roman Gushchin

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Yes, oom_priority cannot select the largest priority tree-wide for exactly that reason. We need the ability to control from which subtree the kill occurs in ancestor cgroups. If multiple jobs are allocated their own cgroups and they can own memory.oom_priority for their own subcontainers, this becomes quite powerful so they can define their own oom priorities. Otherwise, they can easily override the oom priorities of other cgroups.

Quoted 23 lines

Quoted 14 lines

Quoted 8 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

Quoted 8 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

The only real concern I have is that then we have to do the same with oom_priorities (select largest priority tree-wide), and this will limit an ability to enforce the priority by parent cgroup.

Quoted 6 lines

Yes, oom_priority cannot select the largest priority tree-wide for exactly that reason. We need the ability to control from which subtree the kill occurs in ancestor cgroups. If multiple jobs are allocated their own cgroups and they can own memory.oom_priority for their own subcontainers, this becomes quite powerful so they can define their own oom priorities. Otherwise, they can easily override the oom priorities of other cgroups.

Could you be more speicific about your usecase? What would be a problem If we allow to only increase priority in children (like other hierarchical controls).

Quoted 20 lines · Michal Hocko

Quoted 19 lines · Roman Gushchin

Quoted 17 lines · Michal Hocko

Quoted 1 line · Roman Gushchin

On Wed, Sep 13, 2017 at 02:29:14PM +0200, Michal Hocko wrote:

[...]

Quoted 9 lines

Quoted 4 lines

I strongly believe that comparing only leaf memcgs
is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

One of two main goals of this patchset is to introduce cgroup-level
fairness: bigger cgroups should be affected more than smaller, despite the size of tasks inside. I believe the same principle should be used for cgroups.

Yes bigger cgroups should be preferred but I fail to see why bigger hierarchies should be considered as well if they are not kill-all. And whether non-leaf memcgs should allow kill-all is not entirely clear to me. What would be the usecase?

Quoted 7 lines

Quoted 3 lines

We definitely want to support kill-all for non-leaf cgroups. A workload can consist of several cgroups and we want to clean up the whole thing on OOM.

Could you be more specific about such a workload? E.g. how can be such a hierarchy handled consistently when its sub-tree gets killed due to internal memory pressure?

Or just system-wide OOM.

Quoted 2 lines

Or do you expect that none of the subtree will
have hard limit configured?

And this can also be a case: the whole workload may have hard limit configured, while internal memcgs have only memory.low set for "soft" prioritization.

Quoted 9 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

Quoted 5 lines

is a different thing than

	root
        / | \
       B  C  D

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

Quoted 53 lines · Roman Gushchin

Quoted 20 lines · Michal Hocko

Quoted 19 lines · Roman Gushchin

Quoted 17 lines · Michal Hocko

Quoted 1 line · Roman Gushchin

On Wed, Sep 13, 2017 at 02:29:14PM +0200, Michal Hocko wrote:

[...]

Quoted 9 lines

Quoted 4 lines

I strongly believe that comparing only leaf memcgs
is more straightforward and it doesn't lead to unexpected results as mentioned before (kill a small memcg which is a part of the larger sub-hierarchy).

One of two main goals of this patchset is to introduce cgroup-level
fairness: bigger cgroups should be affected more than smaller, despite the size of tasks inside. I believe the same principle should be used for cgroups.

Yes bigger cgroups should be preferred but I fail to see why bigger hierarchies should be considered as well if they are not kill-all. And whether non-leaf memcgs should allow kill-all is not entirely clear to me. What would be the usecase?

Quoted 7 lines

Quoted 3 lines

We definitely want to support kill-all for non-leaf cgroups. A workload can consist of several cgroups and we want to clean up the whole thing on OOM.

Could you be more specific about such a workload? E.g. how can be such a hierarchy handled consistently when its sub-tree gets killed due to internal memory pressure?

Or just system-wide OOM.

Quoted 2 lines

Or do you expect that none of the subtree will
have hard limit configured?

And this can also be a case: the whole workload may have hard limit configured, while internal memcgs have only memory.low set for "soft" prioritization.

Quoted 14 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

is a different thing than

	root
        / | \
       B  C  D

Quoted 3 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

Quoted 2 lines · Michal Hocko

Quoted 1 line · Roman Gushchin

On Fri, Sep 15, 2017 at 12:58:26PM +0200, Michal Hocko wrote:

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

is a different thing than

	root
        / | \
       B  C  D

Quoted 3 lines

Quoted 3 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

I've tried to implement this approach, and it's really arguable. Although your example looks reasonable, the opposite example is also valid:
you might want to compare whole hierarchies, and it's a quite typical usecase.

Assume, you have several containerized workloads on a machine (probably, each will be contained in a memcg with memory.max set), with some hierarchy of cgroups inside. Then in case of global memory shortage we want to reclaim some memory from the biggest workload, and the selection should not depend on group_oom settings. It would be really strange, if setting group_oom will higher the chances to be killed.

In other words, let's imagine processes as leaf nodes in memcg tree. We decided to select the biggest memcg and kill one or more processes inside (depending on group_oom setting), but the memcg selection doesn't depend on it. We do not compare processes from different cgroups, as well as cgroups with processes. The same should apply to cgroups: why do we want to compare cgroups from different sub-trees?

While size-based comparison can be implemented with this approach, the priority-based is really weird (as David mentioned).
If priorities have no hierarchical meaning at all, we lack the very important ability to enforce hierarchy oom_priority. Otherwise we have to invent some complex rules of oom_priority propagation (e.g. is someone is raising the oom_priority in parent, should it be applied to children immediately, etc).

The oom_group knob meaning also becoms more complex. It affects both the victim selection and OOM action. _ANY_ mechanism which allows to affect OOM victim selection (either priorities, either bpf-based approach) should not have global system-wide meaning, it breaks everything.

I do understand your point, but the same is true for other stuff, right? E.g. cpu time distribution (and io, etc) depends on hierarchy configuration. It's a limitation, but it's ok, as user should create a hierarchy which reflects some logical relations between processes and groups of processes. Otherwise we're going to the configuration hell.

In any case, OOM is a last resort mechanism. The goal is to reclaim some memory and do not crash the system or do not leave it in totally broken state. Any really complex mm in userspace should be applied _before_ OOM happens. So, I don't think we have to support all possible configurations here, if we're able to achieve the main goal (kill some processes and do not leave broken systems/containers).

I would really appreciate some feedback from Tejun, Johannes here.

Quoted 3 lines · Roman Gushchin

Quoted 2 lines · Michal Hocko

Quoted 1 line · Roman Gushchin

On Fri, Sep 15, 2017 at 12:58:26PM +0200, Michal Hocko wrote:

[...]

Quoted 36 lines

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

is a different thing than

	root
        / | \
       B  C  D

Quoted 3 lines

Quoted 3 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

I've tried to implement this approach, and it's really arguable. Although your example looks reasonable, the opposite example is also valid:
you might want to compare whole hierarchies, and it's a quite typical usecase.

Assume, you have several containerized workloads on a machine (probably, each will be contained in a memcg with memory.max set), with some hierarchy of cgroups inside. Then in case of global memory shortage we want to reclaim some memory from the biggest workload, and the selection should not depend on group_oom settings. It would be really strange, if setting group_oom will higher the chances to be killed.

In other words, let's imagine processes as leaf nodes in memcg tree. We decided to select the biggest memcg and kill one or more processes inside (depending on group_oom setting), but the memcg selection doesn't depend on it. We do not compare processes from different cgroups, as well as cgroups with processes. The same should apply to cgroups: why do we want to compare cgroups from different sub-trees?

Quoted 6 lines

While size-based comparison can be implemented with this approach, the priority-based is really weird (as David mentioned).
If priorities have no hierarchical meaning at all, we lack the very important ability to enforce hierarchy oom_priority. Otherwise we have to invent some complex rules of oom_priority propagation (e.g. is someone is raising the oom_priority in parent, should it be applied to children immediately, etc).

I would really forget about the priority at this stage. This needs really much more thinking and I consider the David's usecase very specialized to use it as a template for a general purpose oom prioritization. I might be wrong here of course...

Quoted 6 lines

In any case, OOM is a last resort mechanism. The goal is to reclaim some memory and do not crash the system or do not leave it in totally broken state. Any really complex mm in userspace should be applied _before_ OOM happens. So, I don't think we have to support all possible configurations here, if we're able to achieve the main goal (kill some processes and do not leave broken systems/containers).

True but we want to have the semantic reasonably understandable. And it is quite hard to explain that the oom killer hasn't selected the largest memcg just because it happened to be in a deeper hierarchy which has been configured to cover a different resource.

Quoted 50 lines · Michal Hocko

I would really appreciate some feedback from Tejun, Johannes here.

Quoted 3 lines · Roman Gushchin

Quoted 2 lines · Michal Hocko

Quoted 1 line · Roman Gushchin

On Fri, Sep 15, 2017 at 12:58:26PM +0200, Michal Hocko wrote:

[...]

Quoted 36 lines

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

But then you just enforce a structural restriction on your configuration because

	root
        /  \
       A    D
      /\   
     B  C

is a different thing than

	root
        / | \
       B  C  D

Quoted 3 lines

Quoted 3 lines

I actually don't have a strong argument against an approach to select largest leaf or kill-all-set memcg. I think, in practice there will be no much difference.

I've tried to implement this approach, and it's really arguable. Although your example looks reasonable, the opposite example is also valid:
you might want to compare whole hierarchies, and it's a quite typical usecase.

Assume, you have several containerized workloads on a machine (probably, each will be contained in a memcg with memory.max set), with some hierarchy of cgroups inside. Then in case of global memory shortage we want to reclaim some memory from the biggest workload, and the selection should not depend on group_oom settings. It would be really strange, if setting group_oom will higher the chances to be killed.

In other words, let's imagine processes as leaf nodes in memcg tree. We decided to select the biggest memcg and kill one or more processes inside (depending on group_oom setting), but the memcg selection doesn't depend on it. We do not compare processes from different cgroups, as well as cgroups with processes. The same should apply to cgroups: why do we want to compare cgroups from different sub-trees?

Quoted 6 lines

While size-based comparison can be implemented with this approach, the priority-based is really weird (as David mentioned).
If priorities have no hierarchical meaning at all, we lack the very important ability to enforce hierarchy oom_priority. Otherwise we have to invent some complex rules of oom_priority propagation (e.g. is someone is raising the oom_priority in parent, should it be applied to children immediately, etc).

Quoted 4 lines

I would really forget about the priority at this stage. This needs really much more thinking and I consider the David's usecase very specialized to use it as a template for a general purpose oom prioritization. I might be wrong here of course...

No, I agree.

Quoted 11 lines

Quoted 6 lines

In any case, OOM is a last resort mechanism. The goal is to reclaim some memory and do not crash the system or do not leave it in totally broken state. Any really complex mm in userspace should be applied _before_ OOM happens. So, I don't think we have to support all possible configurations here, if we're able to achieve the main goal (kill some processes and do not leave broken systems/containers).

True but we want to have the semantic reasonably understandable. And it is quite hard to explain that the oom killer hasn't selected the largest memcg just because it happened to be in a deeper hierarchy which has been configured to cover a different resource.

Going back to Michal's example, say the user configured the following:

       root
      /    \
     A      D
    / \

B C

A global OOM event happens and we find this:
- A > D
- B, C, D are oomgroups

What the user is telling us is that B, C, and D are compound memory consumers. They cannot be divided into their task parts from a memory point of view.

However, the user doesn't say the same for A: the A subtree summarizes and controls aggregate consumption of B and C, but without groupoom set on A, the user says that A is in fact divisible into independent memory consumers B and C.

If we don't have to kill all of A, but we'd have to kill all of D, does it make sense to compare the two?

Let's consider an extreme case of this conundrum:

	root
      /     \
     A       B
    /|\      |

A1-A1000 B1

Again we find:
- A > B
- A1 to A1000 and B1 are oomgroups
But:
- A1 to A1000 individually are tiny, B1 is huge

Going level by level, we'd pick A as the bigger hierarchy in the system, and then kill off one of the tiny groups A1 to A1000.

Conversely, going for biggest consumer regardless of hierarchy, we'd compare A1 to A1000 and B1, then pick B1 as the biggest single atomic memory consumer in the system and kill all its tasks.

Which one of these two fits both the purpose and our historic approach to OOM killing better?

As was noted in this thread, OOM is the last resort to avoid a memory deadlock. Killing the biggest consumer is most likely to resolve this precarious situation. It is also most likely to catch buggy software with memory leaks or runaway allocations, which is a nice bonus.

Killing a potentially tiny consumer inside the biggest top-level hierarchy doesn't achieve this. I think we can all agree on this.

But also, global OOM in particular means that the hierarchical approach to allocating the system's memory among cgroups has failed. The user expressed control over memory in a way that wasn't sufficient to isolate memory consumption between the different hierarchies. IMO what follows from that is that the hierarchy itself is a questionable guide to finding a culprit.

So I'm leaning toward the second model: compare all oomgroups and standalone tasks in the system with each other, independent of the failed hierarchical control structure. Then kill the biggest of them.

I'm not against this model, as I've said before. It feels logical, and will work fine in most cases.

In this case we can drop any mount/boot options, because it preserves the existing behavior in the default configuration. A big advantage.

The only thing, I'm slightly concerned, that due to the way how we calculate the memory footprint for tasks and memory cgroups, we will have a number of weird edge cases. For instance, when putting a single process into the group_oom memcg will alter the oom_score significantly and result in significantly different chances to be killed. An obvious example will be a task with oom_score_adj set to any non-extreme (other than 0 and -1000) value, but it can also happen in case of constrained alloc, for instance.

On Mon 25-09-17 19:15:33, Roman Gushchin wrote:
[...]

Quoted 5 lines

I'm not against this model, as I've said before. It feels logical, and will work fine in most cases.

In this case we can drop any mount/boot options, because it preserves the existing behavior in the default configuration. A big advantage.

I am not sure about this. We still need an opt-in, ragardless, because selecting the largest process from the largest memcg != selecting the largest task (just consider memcgs with many processes example).

Quoted 8 lines

The only thing, I'm slightly concerned, that due to the way how we calculate the memory footprint for tasks and memory cgroups, we will have a number of weird edge cases. For instance, when putting a single process into the group_oom memcg will alter the oom_score significantly and result in significantly different chances to be killed. An obvious example will be a task with oom_score_adj set to any non-extreme (other than 0 and -1000) value, but it can also happen in case of constrained alloc, for instance.

I am not sure I understand. Are you talking about root memcg comparing to other memcgs?

Quoted 11 lines · Michal Hocko

On Mon 25-09-17 19:15:33, Roman Gushchin wrote:
[...]

Quoted 5 lines

I'm not against this model, as I've said before. It feels logical, and will work fine in most cases.

In this case we can drop any mount/boot options, because it preserves the existing behavior in the default configuration. A big advantage.

I am not sure about this. We still need an opt-in, ragardless, because selecting the largest process from the largest memcg != selecting the largest task (just consider memcgs with many processes example).

As I understand Johannes, he suggested to compare individual processes with group_oom mem cgroups. In other words, always select a killable entity with the biggest memory footprint.

This is slightly different from my v8 approach, where I treat leaf memcgs as indivisible memory consumers independent on group_oom setting, so by default I'm selecting the biggest task in the biggest memcg.

While the approach suggested by Johannes looks clear and reasonable, I'm slightly concerned about possible implementation issues, which I've described below:

Quoted 9 lines

Quoted 8 lines

The only thing, I'm slightly concerned, that due to the way how we calculate the memory footprint for tasks and memory cgroups, we will have a number of weird edge cases. For instance, when putting a single process into the group_oom memcg will alter the oom_score significantly and result in significantly different chances to be killed. An obvious example will be a task with oom_score_adj set to any non-extreme (other than 0 and -1000) value, but it can also happen in case of constrained alloc, for instance.

Quoted 2 lines

I am not sure I understand. Are you talking about root memcg comparing to other memcgs?

Not only, but root memcg in this case will be another complication. We can also use the same trick for all memcg (define memcg oom_score as maximum oom_score of the belonging tasks), it will turn group_oom into pure container cleanup solution, without changing victim selection algorithm

But, again, I'm not against approach suggested by Johannes. I think that overall it's the best possible semantics, if we're not taking some implementation details into account.

Quoted 17 lines · Roman Gushchin

Quoted 11 lines · Michal Hocko

On Mon 25-09-17 19:15:33, Roman Gushchin wrote:
[...]

Quoted 5 lines

I'm not against this model, as I've said before. It feels logical, and will work fine in most cases.

In this case we can drop any mount/boot options, because it preserves the existing behavior in the default configuration. A big advantage.

I am not sure about this. We still need an opt-in, ragardless, because selecting the largest process from the largest memcg != selecting the largest task (just consider memcgs with many processes example).

As I understand Johannes, he suggested to compare individual processes with group_oom mem cgroups. In other words, always select a killable entity with the biggest memory footprint.

Quoted 3 lines

This is slightly different from my v8 approach, where I treat leaf memcgs as indivisible memory consumers independent on group_oom setting, so by default I'm selecting the biggest task in the biggest memcg.

My reading is that he is actually proposing the same thing I've been mentioning. Simply select the biggest killable entity (leaf memcg or group_oom hierarchy) and either kill the largest task in that entity (for !group_oom) or the whole memcg/hierarchy otherwise.

Quoted 14 lines

While the approach suggested by Johannes looks clear and reasonable, I'm slightly concerned about possible implementation issues, which I've described below:

Quoted 9 lines

Quoted 8 lines

The only thing, I'm slightly concerned, that due to the way how we calculate the memory footprint for tasks and memory cgroups, we will have a number of weird edge cases. For instance, when putting a single process into the group_oom memcg will alter the oom_score significantly and result in significantly different chances to be killed. An obvious example will be a task with oom_score_adj set to any non-extreme (other than 0 and -1000) value, but it can also happen in case of constrained alloc, for instance.

Quoted 7 lines

Quoted 2 lines

I am not sure I understand. Are you talking about root memcg comparing to other memcgs?

Not only, but root memcg in this case will be another complication. We can also use the same trick for all memcg (define memcg oom_score as maximum oom_score of the belonging tasks), it will turn group_oom into pure container cleanup solution, without changing victim selection algorithm

I fail to see the problem to be honest. Simply evaluate the memcg_score you have so far with one minor detail. You only check memcgs which have tasks (rather than check for leaf node check) or it is group_oom. An intermediate memcg will get a cumulative size of the whole subhierarchy and then you know you can skip the subtree because any subtree can be larger.

Quoted 3 lines

But, again, I'm not against approach suggested by Johannes. I think that overall it's the best possible semantics, if we're not taking some implementation details into account.

I do not see those implementation details issues and let me repeat do not develop a semantic based on implementation details.

Quoted 18 lines · Michal Hocko

Quoted 17 lines · Roman Gushchin

Quoted 11 lines · Michal Hocko

On Mon 25-09-17 19:15:33, Roman Gushchin wrote:
[...]

Quoted 5 lines

I'm not against this model, as I've said before. It feels logical, and will work fine in most cases.

In this case we can drop any mount/boot options, because it preserves the existing behavior in the default configuration. A big advantage.

I am not sure about this. We still need an opt-in, ragardless, because selecting the largest process from the largest memcg != selecting the largest task (just consider memcgs with many processes example).

As I understand Johannes, he suggested to compare individual processes with group_oom mem cgroups. In other words, always select a killable entity with the biggest memory footprint.

Quoted 8 lines

Quoted 3 lines

This is slightly different from my v8 approach, where I treat leaf memcgs as indivisible memory consumers independent on group_oom setting, so by default I'm selecting the biggest task in the biggest memcg.

My reading is that he is actually proposing the same thing I've been mentioning. Simply select the biggest killable entity (leaf memcg or group_oom hierarchy) and either kill the largest task in that entity (for !group_oom) or the whole memcg/hierarchy otherwise.

He wrote the following:
"So I'm leaning toward the second model: compare all oomgroups and standalone tasks in the system with each other, independent of the failed hierarchical control structure. Then kill the biggest of them."

Quoted 28 lines · Roman Gushchin

Quoted 26 lines · Michal Hocko

Quoted 13 lines · Roman Gushchin

Quoted 11 lines · Michal Hocko

On Mon 25-09-17 19:15:33, Roman Gushchin wrote:
[...]

Quoted 5 lines

I'm not against this model, as I've said before. It feels logical, and will work fine in most cases.

In this case we can drop any mount/boot options, because it preserves the existing behavior in the default configuration. A big advantage.

I am not sure about this. We still need an opt-in, ragardless, because selecting the largest process from the largest memcg != selecting the largest task (just consider memcgs with many processes example).

Quoted 7 lines

As I understand Johannes, he suggested to compare individual processes with group_oom mem cgroups. In other words, always select a killable entity with the biggest memory footprint.

This is slightly different from my v8 approach, where I treat leaf memcgs as indivisible memory consumers independent on group_oom setting, so by default I'm selecting the biggest task in the biggest memcg.

My reading is that he is actually proposing the same thing I've been mentioning. Simply select the biggest killable entity (leaf memcg or group_oom hierarchy) and either kill the largest task in that entity (for !group_oom) or the whole memcg/hierarchy otherwise.

Quoted 4 lines

He wrote the following:
"So I'm leaning toward the second model: compare all oomgroups and standalone tasks in the system with each other, independent of the failed hierarchical control structure. Then kill the biggest of them."

I will let Johannes to comment but I believe this is just a misunderstanding. If we compared only the biggest task from each memcg then we are basically losing our fairness objective, aren't we?

Quoted 29 lines · Michal Hocko

Quoted 28 lines · Roman Gushchin

Quoted 26 lines · Michal Hocko

Quoted 13 lines · Roman Gushchin

Quoted 11 lines · Michal Hocko

On Mon 25-09-17 19:15:33, Roman Gushchin wrote:
[...]

Quoted 5 lines

I'm not against this model, as I've said before. It feels logical, and will work fine in most cases.

In this case we can drop any mount/boot options, because it preserves the existing behavior in the default configuration. A big advantage.

I am not sure about this. We still need an opt-in, ragardless, because selecting the largest process from the largest memcg != selecting the largest task (just consider memcgs with many processes example).

Quoted 7 lines

As I understand Johannes, he suggested to compare individual processes with group_oom mem cgroups. In other words, always select a killable entity with the biggest memory footprint.

This is slightly different from my v8 approach, where I treat leaf memcgs as indivisible memory consumers independent on group_oom setting, so by default I'm selecting the biggest task in the biggest memcg.

My reading is that he is actually proposing the same thing I've been mentioning. Simply select the biggest killable entity (leaf memcg or group_oom hierarchy) and either kill the largest task in that entity (for !group_oom) or the whole memcg/hierarchy otherwise.

Quoted 8 lines

Quoted 4 lines

He wrote the following:
"So I'm leaning toward the second model: compare all oomgroups and standalone tasks in the system with each other, independent of the failed hierarchical control structure. Then kill the biggest of them."

I will let Johannes to comment but I believe this is just a misunderstanding. If we compared only the biggest task from each memcg then we are basically losing our fairness objective, aren't we?

Sorry about the confusion.

Yeah I was making the case for what Michal proposed, to kill the biggest terminal consumer, which is either a task or an oomgroup.

You'd basically iterate through all the tasks and cgroups in the system and pick the biggest task that isn't in an oom group or the biggest oom group and then kill that.

Yeah, you'd have to compare the memory footprints of tasks with the memory footprints of cgroups. These aren't defined identically, and tasks don't get attributed every type of allocation that a cgroup would. But it should get us in the ballpark, and I cannot picture a scenario where this would lead to a completely undesirable outcome.

I cannot picture a scenario where this would lead to a completely undesirable outcome.

I feel like David has offered examples here, and many of us at Google have offered examples as long ago as 2013 (if I recall) of cases where the proposed heuristic is EXACTLY WRONG.

We need OOM behavior to kill in a deterministic order configured by policy.

On Tue 26-09-17 20:37:37, Tim Hockin wrote:
[...]

Quoted 3 lines

I feel like David has offered examples here, and many of us at Google have offered examples as long ago as 2013 (if I recall) of cases where the proposed heuristic is EXACTLY WRONG.

I do not think we have discussed anything resembling the current approach. And I would really appreciate some more examples where decisions based on leaf nodes would be EXACTLY WRONG.

Quoted 2 lines

We need OOM behavior to kill in a deterministic order configured by policy.

And nobody is objecting to this usecase. I think we can build a priority policy on top of leaf-based decision as well. The main point we are trying to sort out here is a reasonable semantic that would work for most workloads. Sibling based selection will simply not work on those that have to use deeper hierarchies for organizational purposes. I haven't heard a counter argument for that example yet.

On Tue 26-09-17 20:37:37, Tim Hockin wrote:
[...]

Quoted 3 lines

I feel like David has offered examples here, and many of us at Google have offered examples as long ago as 2013 (if I recall) of cases where the proposed heuristic is EXACTLY WRONG.

I do not think we have discussed anything resembling the current approach. And I would really appreciate some more examples where decisions based on leaf nodes would be EXACTLY WRONG.

Quoted 2 lines

We need OOM behavior to kill in a deterministic order configured by policy.

And nobody is objecting to this usecase. I think we can build a priority policy on top of leaf-based decision as well. The main point we are trying to sort out here is a reasonable semantic that would work for most workloads. Sibling based selection will simply not work on those that have to use deeper hierarchies for organizational purposes. I haven't heard a counter argument for that example yet.

Quoted 13 lines · Michal Hocko

On Tue 26-09-17 20:37:37, Tim Hockin wrote:
[...]

Quoted 3 lines

I feel like David has offered examples here, and many of us at Google have offered examples as long ago as 2013 (if I recall) of cases where the proposed heuristic is EXACTLY WRONG.

I do not think we have discussed anything resembling the current approach. And I would really appreciate some more examples where decisions based on leaf nodes would be EXACTLY WRONG.

Quoted 2 lines

We need OOM behavior to kill in a deterministic order configured by policy.

Quoted 6 lines

And nobody is objecting to this usecase. I think we can build a priority policy on top of leaf-based decision as well. The main point we are trying to sort out here is a reasonable semantic that would work for most workloads. Sibling based selection will simply not work on those that have to use deeper hierarchies for organizational purposes. I haven't heard a counter argument for that example yet.

We have a priority-based, multi-user cluster. That cluster runs a variety of work, including critical things like search and gmail, as well as non-critical things like batch work. We try to offer our users an SLA around how often they will be killed by factors outside themselves, but we also want to get higher utilization. We know for a fact (data, lots of data) that most jobs have spare memory capacity, set aside for spikes or simply because accurate sizing is hard. We can sell "guaranteed" resources to critical jobs, with a high SLA. We can sell "best effort" resources to non-critical jobs with a low SLA. We achieve much better overall utilization this way.

I need to represent the priority of these tasks in a way that gives me a very strong promise that, in case of system OOM, the non-critical jobs will be chosen before the critical jobs. Regardless of size. Regardless of how many non-critical jobs have to die. I'd rather kill *all* of the non-critical jobs than a single critical job. Size of the process or cgroup is simply not a factor, and honestly given 2 options of equal priority I'd say age matters more than size.

So concretely I have 2 first-level cgroups, one for "guaranteed" and one for "best effort" classes. I always want to kill from "best effort", even if that means killing 100 small cgroups, before touching "guaranteed".

I apologize if this is not as thorough as the rest of the thread - I am somewhat out of touch with the guts of it all these days. I just feel compelled to indicate that, as a historical user (via Google systems) and current user (via Kubernetes), some of the assertions being made here do not ring true for our very real use cases. I desperately want cgroup-aware OOM handing, but it has to be policy-based or it is just not useful to us.

Quoted 14 lines · Tim Hockin

Quoted 13 lines · Michal Hocko

On Tue 26-09-17 20:37:37, Tim Hockin wrote:
[...]

Quoted 3 lines

I feel like David has offered examples here, and many of us at Google have offered examples as long ago as 2013 (if I recall) of cases where the proposed heuristic is EXACTLY WRONG.

I do not think we have discussed anything resembling the current approach. And I would really appreciate some more examples where decisions based on leaf nodes would be EXACTLY WRONG.

Quoted 2 lines

We need OOM behavior to kill in a deterministic order configured by policy.

Quoted 7 lines

Quoted 6 lines

And nobody is objecting to this usecase. I think we can build a priority policy on top of leaf-based decision as well. The main point we are trying to sort out here is a reasonable semantic that would work for most workloads. Sibling based selection will simply not work on those that have to use deeper hierarchies for organizational purposes. I haven't heard a counter argument for that example yet.

Hi, Tim!

Quoted 10 lines

We have a priority-based, multi-user cluster. That cluster runs a variety of work, including critical things like search and gmail, as well as non-critical things like batch work. We try to offer our users an SLA around how often they will be killed by factors outside themselves, but we also want to get higher utilization. We know for a fact (data, lots of data) that most jobs have spare memory capacity, set aside for spikes or simply because accurate sizing is hard. We can sell "guaranteed" resources to critical jobs, with a high SLA. We can sell "best effort" resources to non-critical jobs with a low SLA. We achieve much better overall utilization this way.

This is well understood.

Quoted 14 lines

I need to represent the priority of these tasks in a way that gives me a very strong promise that, in case of system OOM, the non-critical jobs will be chosen before the critical jobs. Regardless of size. Regardless of how many non-critical jobs have to die. I'd rather kill *all* of the non-critical jobs than a single critical job. Size of the process or cgroup is simply not a factor, and honestly given 2 options of equal priority I'd say age matters more than size.

So concretely I have 2 first-level cgroups, one for "guaranteed" and one for "best effort" classes. I always want to kill from "best effort", even if that means killing 100 small cgroups, before touching "guaranteed".

Quoted 7 lines

I apologize if this is not as thorough as the rest of the thread - I am somewhat out of touch with the guts of it all these days. I just feel compelled to indicate that, as a historical user (via Google systems) and current user (via Kubernetes), some of the assertions being made here do not ring true for our very real use cases. I desperately want cgroup-aware OOM handing, but it has to be policy-based or it is just not useful to us.

A policy-based approach was suggested by Michal at a very beginning of this discussion. Although nobody had any strong objections against it, we've agreed that this is out of scope of this patchset.

The idea of this patchset is to introduce an ability to select a memcg as an OOM victim with the following optional killing of all belonging tasks. I believe, it's absolutely mandatory for _any_ further development of the OOM killer, which wants to deal with memory cgroups as OOM entities.

If you think that it makes impossible to support some use cases in the future, let's discuss it. Otherwise, I'd prefer to finish this part of the work, and proceed to the following improvements on top of it.

Thank you!

Going back to Michal's example, say the user configured the following:

       root
      /    \
     A      D
    / \

B C

A global OOM event happens and we find this:
- A > D
- B, C, D are oomgroups

What the user is telling us is that B, C, and D are compound memory consumers. They cannot be divided into their task parts from a memory point of view.

However, the user doesn't say the same for A: the A subtree summarizes and controls aggregate consumption of B and C, but without groupoom set on A, the user says that A is in fact divisible into independent memory consumers B and C.

If we don't have to kill all of A, but we'd have to kill all of D, does it make sense to compare the two?

I think Tim has given very clear explanation why comparing A & D makes perfect sense. However I think the above example, a single user system where a user has designed and created the whole hierarchy and then attaches different jobs/applications to different nodes in this hierarchy, is also a valid scenario. One solution I can think of, to cater both scenarios, is to introduce a notion of 'bypass oom' or not include a memcg for oom comparision and instead include its children in the comparison.

Quoted 17 lines

Going back to Michal's example, say the user configured the following:

       root
      /    \
     A      D
    / \

B C

A global OOM event happens and we find this:
- A > D
- B, C, D are oomgroups

What the user is telling us is that B, C, and D are compound memory consumers. They cannot be divided into their task parts from a memory point of view.

Quoted 8 lines

However, the user doesn't say the same for A: the A subtree summarizes and controls aggregate consumption of B and C, but without groupoom set on A, the user says that A is in fact divisible into independent memory consumers B and C.

If we don't have to kill all of A, but we'd have to kill all of D, does it make sense to compare the two?

I think Tim has given very clear explanation why comparing A & D makes perfect sense. However I think the above example, a single user system where a user has designed and created the whole hierarchy and then attaches different jobs/applications to different nodes in this hierarchy, is also a valid scenario.

Quoted 26 lines · Shakeel Butt

Quoted 17 lines

Going back to Michal's example, say the user configured the following:

       root
      /    \
     A      D
    / \

B C

A global OOM event happens and we find this:
- A > D
- B, C, D are oomgroups

What the user is telling us is that B, C, and D are compound memory consumers. They cannot be divided into their task parts from a memory point of view.

Quoted 8 lines

However, the user doesn't say the same for A: the A subtree summarizes and controls aggregate consumption of B and C, but without groupoom set on A, the user says that A is in fact divisible into independent memory consumers B and C.

If we don't have to kill all of A, but we'd have to kill all of D, does it make sense to compare the two?

Quoted 5 lines

I think Tim has given very clear explanation why comparing A & D makes perfect sense. However I think the above example, a single user system where a user has designed and created the whole hierarchy and then attaches different jobs/applications to different nodes in this hierarchy, is also a valid scenario.

Yes and nobody is disputing that, really. I guess the main disconnect here is that different people want to have more detailed control over the victim selection while the patchset tries to handle the most simplistic scenario when a no userspace control over the selection is required. And I would claim that this will be a last majority of setups and we should address it first.

A more fine grained control needs some more thinking to come up with a sensible and long term sustainable API. Just look back and see at the oom_score_adj story and how it ended up unusable in the end (well apart from never/always kill corner cases). Let's not repeat that again now.

I strongly believe that we can come up with something - be it priority based, BFP based or module based selection. But let's start simple with the most basic scenario first with a most sensible semantic implemented.

I believe the latest version (v9) looks sensible from the semantic point of view and we should focus on making it into a mergeable shape.

On Mon, Oct 02, 2017 at 02:24:34PM +0200, Michal Hocko wrote:

Quoted 2 lines

I believe the latest version (v9) looks sensible from the semantic point of view and we should focus on making it into a mergeable shape.

The only thing is that after some additional thinking I don't think anymore that implicit propagation of oom_group is a good idea.

Yes and nobody is disputing that, really. I guess the main disconnect here is that different people want to have more detailed control over the victim selection while the patchset tries to handle the most simplistic scenario when a no userspace control over the selection is required. And I would claim that this will be a last majority of setups and we should address it first.

Quoted 6 lines

Yes and nobody is disputing that, really. I guess the main disconnect here is that different people want to have more detailed control over the victim selection while the patchset tries to handle the most simplistic scenario when a no userspace control over the selection is required. And I would claim that this will be a last majority of setups and we should address it first.

IMHO the disconnect/disagreement is which memcgs should be compared with each other for oom victim selection. Let's forget about oom priority and just take size into the account. Should the oom selection algorithm, compare the leaves of the hierarchy or should it compare siblings? For the single user system, comparing leaves makes sense while in a multi user system, siblings should be compared for victim selection.

I am sorry to cut the rest of your proposal because it simply goes over the scope of the proposed solution while the usecase you are mentioning is still possible. If we want to compare intermediate nodes (which seems to be the case) then we can always provide a knob to opt-in - be it your oom_gang or others.

Quoted 5 lines

I am sorry to cut the rest of your proposal because it simply goes over the scope of the proposed solution while the usecase you are mentioning is still possible. If we want to compare intermediate nodes (which seems to be the case) then we can always provide a knob to opt-in - be it your oom_gang or others.

In the Roman's proposed solution we can already force the comparison of intermediate nodes using 'oom_group', I am just requesting to separate the killall semantics from it.

Quoted 9 lines · Shakeel Butt

Quoted 5 lines

I am sorry to cut the rest of your proposal because it simply goes over the scope of the proposed solution while the usecase you are mentioning is still possible. If we want to compare intermediate nodes (which seems to be the case) then we can always provide a knob to opt-in - be it your oom_gang or others.

In the Roman's proposed solution we can already force the comparison of intermediate nodes using 'oom_group', I am just requesting to separate the killall semantics from it.

oom_group _is_ about killall semantic. And comparing killable entities is just a natural thing to do. So I am not sure what you mean

--
Michal Hocko
SUSE Labs

In the example above:

       root
       /    \
     A      D
     / \

B C

Does oom_group allow me to express "compare A and D; if A is chosen compare B and C; kill the loser" ? As I understand the proposal (from reading thread, not patch) it does not.

In the example above:

       root
       /    \
     A      D
     / \

B C

Does oom_group allow me to express "compare A and D; if A is chosen compare B and C; kill the loser" ? As I understand the proposal (from reading thread, not patch) it does not.

Quoted 2 lines · Shakeel Butt

Quoted 1 line

Quoted 1 line

I am sorry to cut the rest of your proposal because it simply goes

over

Quoted 1 line

Quoted 1 line

Quoted 1 line

the scope of the proposed solution while the usecase you are

mentioning

Quoted 1 line

Quoted 1 line

Quoted 1 line

is still possible. If we want to compare intermediate nodes (which

seems

Quoted 1 line

Quoted 1 line

Quoted 1 line

to be the case) then we can always provide a knob to opt-in - be it

your

Quoted 12 lines

Quoted 5 lines

Quoted 1 line

oom_gang or others.

In the Roman's proposed solution we can already force the comparison of intermediate nodes using 'oom_group', I am just requesting to separate the killall semantics from it.

oom_group _is_ about killall semantic. And comparing killable entities is just a natural thing to do. So I am not sure what you mean

--
Michal Hocko
SUSE Labs

In the example above:

       root
       /    \
     A      D
     / \

B C

Does oom_group allow me to express "compare A and D; if A is chosen compare B and C; kill the loser" ? As I understand the proposal (from reading thread, not patch) it does not.

Quoted 6 lines · Shakeel Butt

Quoted 5 lines

I am sorry to cut the rest of your proposal because it simply goes over the scope of the proposed solution while the usecase you are mentioning is still possible. If we want to compare intermediate nodes (which seems to be the case) then we can always provide a knob to opt-in - be it your oom_gang or others.

Quoted 3 lines

In the Roman's proposed solution we can already force the comparison of intermediate nodes using 'oom_group', I am just requesting to separate the killall semantics from it.

oom_group _is_ about killall semantic. And comparing killable entities is just a natural thing to do. So I am not sure what you mean

Quoted 11 lines · Michal Hocko

Quoted 9 lines · Shakeel Butt

Quoted 5 lines

I am sorry to cut the rest of your proposal because it simply goes over the scope of the proposed solution while the usecase you are mentioning is still possible. If we want to compare intermediate nodes (which seems to be the case) then we can always provide a knob to opt-in - be it your oom_gang or others.

In the Roman's proposed solution we can already force the comparison of intermediate nodes using 'oom_group', I am just requesting to separate the killall semantics from it.

Quoted 3 lines

oom_group _is_ about killall semantic. And comparing killable entities is just a natural thing to do. So I am not sure what you mean

I am saying decouple the notion of comparable entities and killable entities.

Quoted 11 lines · Michal Hocko

Quoted 9 lines · Shakeel Butt

Quoted 5 lines

I am sorry to cut the rest of your proposal because it simply goes over the scope of the proposed solution while the usecase you are mentioning is still possible. If we want to compare intermediate nodes (which seems to be the case) then we can always provide a knob to opt-in - be it your oom_gang or others.

In the Roman's proposed solution we can already force the comparison of intermediate nodes using 'oom_group', I am just requesting to separate the killall semantics from it.

Quoted 3 lines

oom_group _is_ about killall semantic. And comparing killable entities is just a natural thing to do. So I am not sure what you mean

I am saying decouple the notion of comparable entities and killable entities.

Quoted 4 lines

True but we want to have the semantic reasonably understandable. And it is quite hard to explain that the oom killer hasn't selected the largest memcg just because it happened to be in a deeper hierarchy which has been configured to cover a different resource.

Going back to Michal's example, say the user configured the following:

       root
      /    \
     A      D
    / \

B C

A global OOM event happens and we find this:
- A > D
- B, C, D are oomgroups

What the user is telling us is that B, C, and D are compound memory consumers. They cannot be divided into their task parts from a memory point of view.

However, the user doesn't say the same for A: the A subtree summarizes and controls aggregate consumption of B and C, but without groupoom set on A, the user says that A is in fact divisible into independent memory consumers B and C.

If we don't have to kill all of A, but we'd have to kill all of D, does it make sense to compare the two?

Let's consider an extreme case of this conundrum:

	root
      /     \
     A       B
    /|\      |

A1-A1000 B1

Again we find:
- A > B
- A1 to A1000 and B1 are oomgroups
But:
- A1 to A1000 individually are tiny, B1 is huge

Going level by level, we'd pick A as the bigger hierarchy in the system, and then kill off one of the tiny groups A1 to A1000.

Conversely, going for biggest consumer regardless of hierarchy, we'd compare A1 to A1000 and B1, then pick B1 as the biggest single atomic memory consumer in the system and kill all its tasks.

Quoted 21 lines · Johannes Weiner

Quoted 4 lines

True but we want to have the semantic reasonably understandable. And it is quite hard to explain that the oom killer hasn't selected the largest memcg just because it happened to be in a deeper hierarchy which has been configured to cover a different resource.

Going back to Michal's example, say the user configured the following:

       root
      /    \
     A      D
    / \

B C

A global OOM event happens and we find this:
- A > D
- B, C, D are oomgroups

What the user is telling us is that B, C, and D are compound memory consumers. They cannot be divided into their task parts from a memory point of view.

Quoted 8 lines

However, the user doesn't say the same for A: the A subtree summarizes and controls aggregate consumption of B and C, but without groupoom set on A, the user says that A is in fact divisible into independent memory consumers B and C.

If we don't have to kill all of A, but we'd have to kill all of D, does it make sense to compare the two?

No, I agree that we shouldn't compare sibling memory cgroups based on different criteria depending on whether group_oom is set or not.

I think it would be better to compare siblings based on the same criteria independent of group_oom if the user has mounted the hierarchy with the new mode (I think we all agree that the mount option is needed). It's very easy to describe to the user and the selection is simple to understand.

Then, once a cgroup has been chosen as the victim cgroup,
kill the process with the highest badness, allowing the user to influence that with /proc/pid/oom_score_adj just as today, if group_oom is disabled; otherwise, kill all eligible processes if enabled.

That, to me, is a very clear semantic and I believe it addresses Roman's usecase. My desire to have oom priorities amongst siblings is so that userspace can influence which cgroup is chosen, just as it can influence which process is chosen.

Quoted 8 lines

No, I agree that we shouldn't compare sibling memory cgroups based on different criteria depending on whether group_oom is set or not.

I think it would be better to compare siblings based on the same criteria independent of group_oom if the user has mounted the hierarchy with the new mode (I think we all agree that the mount option is needed). It's very easy to describe to the user and the selection is simple to understand.

I disagree. Just take the most simplistic example when cgroups reflect some other higher level organization - e.g. school with teachers, students and admins as the top level cgroups to control the proper cpu share load. Now you want to have a fair OOM selection between different entities. Do you consider selecting students all the time as an expected behavior just because their are the largest group? This just doesn't make any sense to me.

Quoted 4 lines

Then, once a cgroup has been chosen as the victim cgroup,
kill the process with the highest badness, allowing the user to influence that with /proc/pid/oom_score_adj just as today, if group_oom is disabled; otherwise, kill all eligible processes if enabled.

And now, what should be the semantic of group_oom on an intermediate (non-leaf) memcg? Why should we compare it to other killable entities? Roman was mentioning a setup where a _single_ workload consists of a deeper hierarchy which has to be shut down at once. It absolutely makes sense to consider the cumulative memory of that hierarchy when we are going to kill it all.

But what you are proposing is something different from oom_score_adj. That only sets bias to the killable entities while priorities on intermediate non-killable memcgs controls how the whole oom hierarchy is traversed. So a non-killable intermediate memcg can hugely influence what gets killed in the end.

This is IMHO a tricky and I would even dare
to claim a wrong semantic. I can see priorities being very useful on killable entities for sure. I am not entirely sure what would be the best approach yet and that is why I've suggested that to postpone to after we settle with a simple approach first. Bringing priorities back to the discussion again will not help to move that forward I am afraid.

Quoted 9 lines · Michal Hocko

Quoted 8 lines

No, I agree that we shouldn't compare sibling memory cgroups based on different criteria depending on whether group_oom is set or not.

I think it would be better to compare siblings based on the same criteria independent of group_oom if the user has mounted the hierarchy with the new mode (I think we all agree that the mount option is needed). It's very easy to describe to the user and the selection is simple to understand.

Quoted 8 lines

I disagree. Just take the most simplistic example when cgroups reflect some other higher level organization - e.g. school with teachers, students and admins as the top level cgroups to control the proper cpu share load. Now you want to have a fair OOM selection between different entities. Do you consider selecting students all the time as an expected behavior just because their are the largest group? This just doesn't make any sense to me.

Are you referring to this?

	root
       /    \
students    admins
/      \    /    \
A      B    C    D

If the cumulative usage of all students exceeds the cumulative usage of all admins, yes, the choice is to kill from the /students tree.

This has been Roman's design from the very beginning.

If the preference is to kill
the single largest process, which may be attached to either subtree, you would not have opted-in to the new heuristic.

Quoted 5 lines

Quoted 4 lines

Then, once a cgroup has been chosen as the victim cgroup,
kill the process with the highest badness, allowing the user to influence that with /proc/pid/oom_score_adj just as today, if group_oom is disabled; otherwise, kill all eligible processes if enabled.

Quoted 7 lines

And now, what should be the semantic of group_oom on an intermediate (non-leaf) memcg? Why should we compare it to other killable entities? Roman was mentioning a setup where a _single_ workload consists of a deeper hierarchy which has to be shut down at once. It absolutely makes sense to consider the cumulative memory of that hierarchy when we are going to kill it all.

If group_oom is enabled on an intermediate memcg, I think the intuitive way to handle it would be that all descendants are also implicitly or explicitly group_oom.

It is compared to sibling cgroups based on
cumulative usage at the time of oom and the largest is chosen and iterated. The point is to separate out the selection heuristic (policy) from group_oom (mechanism) so that we don't bias or prefer subtrees based on group_oom, which makes this much more complex.

Quoted 22 lines · David Rientjes

Quoted 9 lines · Michal Hocko

Quoted 8 lines

No, I agree that we shouldn't compare sibling memory cgroups based on different criteria depending on whether group_oom is set or not.

I think it would be better to compare siblings based on the same criteria independent of group_oom if the user has mounted the hierarchy with the new mode (I think we all agree that the mount option is needed). It's very easy to describe to the user and the selection is simple to understand.

Quoted 8 lines

I disagree. Just take the most simplistic example when cgroups reflect some other higher level organization - e.g. school with teachers, students and admins as the top level cgroups to control the proper cpu share load. Now you want to have a fair OOM selection between different entities. Do you consider selecting students all the time as an expected behavior just because their are the largest group? This just doesn't make any sense to me.

Are you referring to this?

Quoted 8 lines

	root
       /    \
students    admins
/      \    /    \
A      B    C    D

If the cumulative usage of all students exceeds the cumulative usage of all admins, yes, the choice is to kill from the /students tree.

Which is wrong IMHO because the number of stutends is likely much more larger than admins (or teachers) yet it might be the admins one to run away. This example simply shows how comparing siblinks highly depends on the way you organize the hierarchy rather than the actual memory consumer runaways which is the primary goal of the OOM killer to handle.

Quoted 1 line

This has been Roman's design from the very beginning.

I suspect this was the case because deeper hierarchies for organizational purposes haven't been considered.

Quoted 3 lines

If the preference is to kill
the single largest process, which may be attached to either subtree, you would not have opted-in to the new heuristic.

I believe you are making a wrong assumption here. The container cleanup is sound reason to opt in and deeper hierarchies are simply required in the cgroup v2 world where you do not have separate hierarchies.

Quoted 13 lines

Quoted 5 lines

Quoted 4 lines

Then, once a cgroup has been chosen as the victim cgroup,
kill the process with the highest badness, allowing the user to influence that with /proc/pid/oom_score_adj just as today, if group_oom is disabled; otherwise, kill all eligible processes if enabled.

Quoted 7 lines

And now, what should be the semantic of group_oom on an intermediate (non-leaf) memcg? Why should we compare it to other killable entities? Roman was mentioning a setup where a _single_ workload consists of a deeper hierarchy which has to be shut down at once. It absolutely makes sense to consider the cumulative memory of that hierarchy when we are going to kill it all.

Quoted 3 lines

If group_oom is enabled on an intermediate memcg, I think the intuitive way to handle it would be that all descendants are also implicitly or explicitly group_oom.

This is an interesting point. I would tend to agree here. If somebody requires all-in clean up up the hierarchy it feels strange that a subtree would disagree (e.g. during memcg oom on the subtree). I can hardly see a usecase that would really need a different group_oom policy depending on where in the hierarchy the oom happened to be honest. Roman?

Quoted 6 lines · Roman Gushchin

This patchset makes the OOM killer cgroup-aware.

v8:
- Do not kill tasks with OOM_SCORE_ADJ -1000
- Make the whole thing opt-in with cgroup mount option control
- Drop oom_priority for further discussions

Nack, we specifically require oom_priority for this to function correctly, otherwise we cannot prefer to kill from low priority leaf memcgs as required. v8 appears to implement new functionality that we want, to compare two memcgs based on usage, but without the ability to influence that decision to protect important userspace, so now I'm in a position where (1) nothing has changed if I don't use the new mount option or (2) I get completely different oom kill selection with the new mount option but not the ability to influence it. I was much happier with the direction that v7 was taking, but since v8 causes us to regress without the ability to change memcg priority, this has to be nacked.

That's a ridiculous nak.

The fact that this patch series doesn't solve your particular problem is not a technical argument to *reject* somebody else's work to solve a different problem. It's not a regression when behavior is completely unchanged unless you explicitly opt into a new functionality.

So let's stay reasonable here.

The patch series has merit as it currently stands. It makes OOM killing in a cgrouped system fairer and less surprising. Whether you have the ability to influence this in a new way is an entirely separate discussion. It's one that involves ABI and user guarantees.

Right now Roman's patches make no guarantees on how the cgroup tree is descended. But once we define an interface for prioritization, it locks the victim algorithm into place to a certain extent.

It also involves a discussion about how much control userspace should have over OOM killing in the first place. It's a last-minute effort to save the kernel from deadlocking on memory. Whether that is the time and place to have userspace make clever resource management decisions is an entirely different thing than what Roman is doing.

But this patch series doesn't prevent any such future discussion and implementations, and it's not useless without it. So let's not conflate these two things, and hold the priority patch for now.

Quoted 7 lines · Johannes Weiner

That's a ridiculous nak.

The fact that this patch series doesn't solve your particular problem is not a technical argument to *reject* somebody else's work to solve a different problem. It's not a regression when behavior is completely unchanged unless you explicitly opt into a new functionality.

Quoted 2 lines

So let's stay reasonable here.

The issue is that if you opt-in to the new feature, then you are forced to change /proc/pid/oom_score_adj of all processes attached to a cgroup that you do not want oom killed based on size to be oom disabled.

The kernel provides no other remedy without oom priorities since the new feature would otherwise disregard oom_score_adj.

The nack originates from the general need for userspace influence over oom victim selection and to avoid userspace needing to take the rather drastic measure of setting all processes to be oom disabled to prevent oom kill in kernels before oom priorities are introduced.

Quoted 9 lines

The patch series has merit as it currently stands. It makes OOM killing in a cgrouped system fairer and less surprising. Whether you have the ability to influence this in a new way is an entirely separate discussion. It's one that involves ABI and user guarantees.

Right now Roman's patches make no guarantees on how the cgroup tree is descended. But once we define an interface for prioritization, it locks the victim algorithm into place to a certain extent.

The patchset compares memory cgroup size relative to sibling cgroups only, the same comparison for memory.oom_priority. There is a guarantee provided on how cgroup size is compared in select_victim_memcg(), it hierarchically accumulates the "size" from leaf nodes up to the root memcg and then iterates the tree comparing sizes between sibling cgroups to choose a victim memcg. That algorithm could be more elaborately described in the documentation, but we simply cannot change the implementation of select_victim_memcg() later even without oom priorities since users cannot get inconsistent results after opting into a feature between kernel versions. I believe the selection criteria should be implemented to be deterministic, as select_victim_memcg() does, and the documentation should fully describe what the selection criteria is, and then allow the user to decide.

Quoted 3 lines

The issue is that if you opt-in to the new feature, then you are forced to change /proc/pid/oom_score_adj of all processes attached to a cgroup that you do not want oom killed based on size to be oom disabled.

You're assuming that most people would want to influence the oom behavior in the first place. I think the opposite is the case: most people don't care as long as the OOM killer takes the intent the user has expressed wrt runtime containerization/grouping into account.

Quoted 2 lines

The kernel provides no other remedy without oom priorities since the new feature would otherwise disregard oom_score_adj.

As of v8, it respects this setting and doesn't kill min score tasks.

Quoted 5 lines

The patchset compares memory cgroup size relative to sibling cgroups only, the same comparison for memory.oom_priority. There is a guarantee provided on how cgroup size is compared in select_victim_memcg(), it hierarchically accumulates the "size" from leaf nodes up to the root memcg and then iterates the tree comparing sizes between sibling cgroups to

Quoted 8 lines

choose a victim memcg. That algorithm could be more elaborately described in the documentation, but we simply cannot change the implementation of select_victim_memcg() later even without oom priorities since users cannot get inconsistent results after opting into a feature between kernel versions. I believe the selection criteria should be implemented to be deterministic, as select_victim_memcg() does, and the documentation should fully describe what the selection criteria is, and then allow the user to decide.

I wholeheartedly disagree. We have changed the behavior multiple times in the past. In fact, you have arguably done the most drastic changes to the algorithm since the OOM killer was first introduced. E.g.

a63d83f427fb oom: badness heuristic rewrite

And that's completely fine. Because this thing is not a resource management tool for userspace, it's the kernel saving itself. At best in a manner that's not too surprising to userspace.

To me, your argument behind the NAK still boils down to "this doesn't support my highly specialized usecase." But since it doesn't prohibit your usecase - which isn't even supported upstream, btw - this really doesn't carry much weight.

I'd say if you want configurability on top of Roman's code, please submit patches and push the case for these in a separate effort.

It doesn't have anything to do with my particular usecase, but rather the ability of userspace to influence the decisions of the kernel. Previous to this patchset, when selection is done based on process size, userspace has full control over selection. After this patchset, userspace has no control other than setting all processes to be oom disabled if the largest

memory consumer is to be protected. Roman's memory.oom_priority provides a perfect solution for userspace to be able to influence this decision making and causes no change in behavior for users who choose not to tune memory.oom_priority. The nack originates from the general need for userspace influence over oom victim selection and to avoid userspace needing to take the rather drastic measure of setting all processes to be oom disabled to prevent oom kill in kernels before oom priorities are introduced.

The patchset compares memory cgroup size relative to sibling cgroups only, the same comparison for memory.oom_priority. There is a guarantee provided on how cgroup size is compared in select_victim_memcg(), it hierarchically accumulates the "size" from leaf nodes up to the root memcg and then iterates the tree comparing sizes between sibling cgroups to

choose a victim memcg. That algorithm could be more elaborately described in the documentation, but we simply cannot change the implementation of select_victim_memcg() later even without oom priorities since users cannot get inconsistent results after opting into a feature between kernel versions. I believe the selection criteria should be implemented to be deterministic, as select_victim_memcg() does, and the documentation should fully describe what the selection criteria is, and then allow the user to decide.

Roman is planning on introducing memory.oom_priority back into the patchset per https://marc.info/?l=linux-kernel&m=150574701126877 and I agree with the very clear semantic that it introduces: to have the size-based comparison use the same rules as the userspace priority comparison. It's very powerful and I'm happy to ack the final version that he plans on posting.

Quoted 5 lines

It doesn't have anything to do with my particular usecase, but rather the ability of userspace to influence the decisions of the kernel. Previous to this patchset, when selection is done based on process size, userspace has full control over selection. After this patchset, userspace has no control other than setting all processes to be oom disabled if the largest

Quoted 8 lines

memory consumer is to be protected. Roman's memory.oom_priority provides a perfect solution for userspace to be able to influence this decision making and causes no change in behavior for users who choose not to tune memory.oom_priority. The nack originates from the general need for userspace influence over oom victim selection and to avoid userspace needing to take the rather drastic measure of setting all processes to be oom disabled to prevent oom kill in kernels before oom priorities are introduced.

Overall, I think that OOM killing is the wrong place to implement sophisticated intelligence in. It's too late to be smart - the workload already has suffered significantly and there's only very limited amount of computing which can be performed. That said, if there's a useful and general enough mechanism to configure OOM killer behavior from userland, that can definitely be useful.

We even change the whole scheduling behaviors and try really hard to not get locked into specific implementation details which exclude future improvements. Guaranteeing OOM killing selection would be crazy. Why would we prevent ourselves from doing things better in the future? We aren't talking about the semantics of read(2) here. This is a kernel emergency mechanism to avoid deadlock at the last moment.

Here's a really simple use case. Imagine a system which hosts two containers of services and one is somewhat favored over the other and wants to set up cgroup hierarchy so that resources are split at the top level between the two containers. oom_priority is set accordingly too. Let's say a low priority maintenance job in higher priority container goes berserk, as they oftne do, and pushing the system into OOM.

With the proposed static oom_priority mechanism, the only configuration which can be expressed is "kill all of the lower top level subtree before any of the higher one", which is a silly restriction leading to silly behavior and a direct result of conflating resource distribution network with level-by-level OOM killing decsion.

Current heuristic based on processes is coupled with per-process /proc/pid/oom_score_adj. The proposed
heuristic has no ability to be influenced by userspace, and it needs one. The proposed heuristic based on memory cgroups coupled with Roman's per-memcg memory.oom_priority is appropriate and needed. It is not

"sophisticated intelligence," it merely allows userspace to protect vital memory cgroups when opting into the new features (cgroups compared based on size and memory.oom_group) that we very much want.

Quoted 6 lines

We even change the whole scheduling behaviors and try really hard to not get locked into specific implementation details which exclude future improvements. Guaranteeing OOM killing selection would be crazy. Why would we prevent ourselves from doing things better in the future? We aren't talking about the semantics of read(2) here. This is a kernel emergency mechanism to avoid deadlock at the last moment.

We merely want to prefer other memory cgroups are oom killed on system oom conditions before important ones, regardless if the important one is using more memory than the others because of the new heuristic this patchset introduces. This is exactly the same as /proc/pid/oom_score_adj for the current heuristic.

If you have this low priority maintenance job charging memory to the high priority hierarchy, you're already misconfigured unless you adjust /proc/pid/oom_score_adj because it will oom kill any larger process than itself in today's kernels anyway.

A better configuration would be attach this hypothetical low priority maintenance job to its own sibling cgroup with its own memory limit to avoid exactly that problem: it going berserk and charging too much memory to the high priority container that results in one of its processes getting oom killed.

Quoted 10 lines

If you have this low priority maintenance job charging memory to the high priority hierarchy, you're already misconfigured unless you adjust /proc/pid/oom_score_adj because it will oom kill any larger process than itself in today's kernels anyway.

A better configuration would be attach this hypothetical low priority maintenance job to its own sibling cgroup with its own memory limit to avoid exactly that problem: it going berserk and charging too much memory to the high priority container that results in one of its processes getting oom killed.

And how do you guarantee that across delegation boundaries? The points you raise on why the priority should be applied level-by-level are exactly the same points why this doesn't really work. OOM killing priority isn't something which can be distributed across cgroup hierarchy level-by-level. The resulting decision tree doesn't make any sense.

I'm not against adding something which works but strict level-by-level comparison isn't the solution.