cgroup-aware OOM killer

The select_bad_process() function will be used further to select a process to kill in the victim cgroup. This cgroup doesn't necessary match oc->memcg, which is a cgroup, which limits were caused cgroup-wide OOM (or NULL in case of global OOM).

So, refactor select_bad_process() to take a pointer to a cgroup to iterate over as an argument.

We use a heavily modified system and memcg oom killer and I'm wondering if there is some opportunity for collaboration because we may have some shared goals.

I can summarize how we currently use the oom killer at a high level so that it is not overwhelming with implementation details and give some rationale for why we have converged onto this strategy over the period of a few years.

For victim selection, we have strict priority based oom killing both at the memcg level and the process level.

Each process has its own "badness" value that is independent of oom_score_adj, although some conversion is done if a third-party thread chooses to disable itself from oom killing for backwards compatibility. Lower values are more preferable victims, but that detail should not matter significantly. If two processes share the same badness value, tiebreaks are done by selecting the largest rss.

Each memcg in a hierarchy also has its own badness value which semantically means the same as the per-process value, although it considers the entire memcg as a unit, similar to your approach, when iterating the hierarchy to choose a process. The benefit of the per-memcg and per-process approach is that you can kill the lowest priority process from the lowest priority memcg.

The above scoring is enabled with a VM sysctl for the system and is used for both system (global) and memcg oom kills. For system overcommit, this means we can kill the lowest priority job on the system; for memcg, we can allow users to define their oom kill priorities at each level of their own hierarchy.

When the system or root of an oom memcg hierarchy encounters its limit, we iterate each level of the memcg hierarchy to find the lowest priority job. This is done by comparing the badness of the sibling memcgs at each level, finding the lowest, and iterating that subtree. If there are lower priority processes per the per-process badness value compared to all sibling memcgs, that process is killed.

We also have complete userspace oom handling support. This complements the existing memory.oom_control notification when a memcg is oom with a separate notifier that notifies when the kernel has oom killed a process. It is possible to delay the oom killer from killing a process for memcg oom kills with a configurable, per-memcg, oom delay. If set, the kernel will wait for userspace to respond to its oom notification and effect its own policy decisions until memory is uncharged to that memcg hierarchy, or the oom delay expires. If the oom delay expires, the kernel oom killer kills a process based on badness.

Our oom kill notification file used to get an fd to register with cgroup.event_control also provides oom kill statistics based on system, memcg, local, hierarchical, and user-induced oom kills when read().

We also have a convenient "kill all" knob that userspace can write when handling oom conditions to iterate all threads attached to a particular memcg and kill them. This is merely to prevent races where userspace does the oom killing itself, which is not problematic in itself, but additional tasks continue to be attached to an oom memcg.

A caveat here is that we also support fully inclusive kmem accounting to memcg hierarchies, so we call the oom killer as part of the memcg charge path rather than only upon returning from fault with VM_FAULT_OOM. We have our own oom killer livelock detection that isn't necessarily important in this thread, but we haven't encountered a situation where we livelock by calling the oom killer during charge, and this is a requirement for memcg charging as part of slab allocation.

I could post many patches to implement all of this functionality that we have used for a few years, but I first wanted to send this email to see if there is any common ground or to better understand your methodology for using the kernel oom killer for both system and memcg oom kills.

Otherwise, very interesting stuff!

Hi David!

Thank you for sharing this!

It's very interesting, and it looks like,
it's not that far from what I've suggested.

So we definitily need to come up with some common solution.

Quoted 2 lines · Roman Gushchin

Hi David!

Quoted 7 lines

Thank you for sharing this!

It's very interesting, and it looks like,
it's not that far from what I've suggested.

So we definitily need to come up with some common solution.

Yes, definitely. I could post a series of patches to do everything that was listed in my email sans the fully inclusive kmem accounting, which may be pursued at a later date, if it would be helpful to see where there is common ground?

Another question is what you think about userspace oom handling? We implement our own oom kill policies in userspace for both the system and for user-controlled memcg hierarchies because it often does not match the kernel implementation and there is some action that can be taken other than killing a process. Have you tried to implement functionality to do userspace oom handling, or are you considering it? This is the main motivation behind allowing an oom delay to be configured.

This option defines whether a cgroup should be treated as a single entity by the OOM killer.

If set, the OOM killer will compare the whole cgroup with other memory consumers (other cgroups and tasks in the root cgroup), and in case of an OOM will kill all belonging tasks.

Disabled by default.

mm/memcontrol.c +0 -0

@@ -5265,6 +5292,12 @@ static struct cftype memory_files[] = {
 		.write = memory_max_write,
 	},

Introduce a per-memory-cgroup oom_score_adj setting. A read-write single value file which exits on non-root cgroups. The default is "0".

It will have a similar meaning to a per-process value, available via /proc/<pid>/oom_score_adj. Should be in a range [-1000, 1000].

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

This behavior doesn't suit well the system with many running containers. There are two main issues:

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

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

3) Per-process oom_score_adj affects global OOM, so it's a breache in the isolation.

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

Under OOM conditions, it tries to find the biggest memory consumer, and free memory by killing corresponding task(s). The difference the "traditional" OOM killer is that it can treat memory cgroups as memory consumers as well as single processes.

By default, it will look for the biggest leaf cgroup, and kill the largest task inside.

But a user can change this behavior by enabling the per-cgroup oom_kill_all_tasks option. If set, it causes the OOM killer treat the whole cgroup as an indivisible memory consumer. In case if it's selected as on OOM victim, all belonging tasks will be killed.

Tasks in the root cgroup are treated as independent memory consumers, and are compared with other memory consumers (e.g. leaf cgroups). The root cgroup doesn't support the oom_kill_all_tasks feature.

mm/memcontrol.c +0 -0

index f979ac7..855d335 100644
@@ -2625,6 +2625,184 @@ static inline bool memcg_has_children(struct mem_cgroup *memcg)
 	return ret;

mm/memcontrol.c +43 -0

+		points += memcg_page_state(iter, MEMCG_KERNEL_STACK_KB) /
+			(PAGE_SIZE / 1024);
+		points += memcg_page_state(iter, MEMCG_SLAB_UNRECLAIMABLE);
+		points += memcg_page_state(iter, MEMCG_SOCK);
+	}
+
+	return points;
+}
+
+bool mem_cgroup_select_oom_victim(struct oom_control *oc)
+{
+	struct cgroup_subsys_state *css = NULL;
+	struct mem_cgroup *iter = NULL;
+	struct mem_cgroup *chosen_memcg = NULL;
+	struct mem_cgroup *parent = root_mem_cgroup;
+	unsigned long totalpages = oc->totalpages;
+	long chosen_memcg_points = 0;
+	long points = 0;
+
+	oc->chosen = NULL;
+	oc->chosen_memcg = NULL;
+
+	if (mem_cgroup_disabled())
+		return false;
+
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
+		return false;
+
+	pr_info("Choosing a victim memcg because of the %s",
+		oc->memcg ?
+		"memory limit reached of cgroup " :
+		"system-wide OOM\n");
+	if (oc->memcg) {
+		pr_cont_cgroup_path(oc->memcg->css.cgroup);
+		pr_cont("\n");
+
+		chosen_memcg = oc->memcg;
+		parent = oc->memcg;
+	}
+
+	rcu_read_lock();
+
+	for (;;) {

mm/memcontrol.c +19 -0

+			chosen_memcg_points = 0;
+
+			parent = chosen_memcg;
+			chosen_memcg = NULL;
+
+			continue;
+		}
+
+		if (!chosen_memcg && parent != root_mem_cgroup)
+			chosen_memcg = parent;
+
+		break;
+	}
+
 
+	if (!oc->memcg) {
+		/*
+		 * We should also consider tasks in the root cgroup
+		 * with badness larger than oc->chosen_points
+		 */

mm/memcontrol.c +7 -0

+
+	if (!oc->chosen && chosen_memcg) {
+		pr_info("Chosen cgroup ");
+		pr_cont_cgroup_path(chosen_memcg->css.cgroup);
+		pr_cont(": %ld\n", oc->chosen_points);
+
+		if (chosen_memcg->oom_kill_all_tasks) {

mm/memcontrol.c +11 -0

+		}
+	} else if (oc->chosen)
+		pr_info("Chosen task %s (%d) in root cgroup: %ld\n",
+			oc->chosen->comm, oc->chosen->pid, oc->chosen_points);
+
+	rcu_read_unlock();
+
+	oc->chosen_points = 0;
+	return !!oc->chosen || !!oc->chosen_memcg;
+}
+

mm/memcontrol.c +8 -0

+{
+	if (oc->chosen_memcg) {
+		/*
+		 * Kill all tasks in the cgroup hierarchy
+		 */
+		mem_cgroup_scan_tasks(oc->chosen_memcg,
+				      __oom_kill_task, NULL);
+

On Thu, Jun 01, 2017 at 07:35:14PM +0100, Roman Gushchin wrote:
...

Quoted 8 lines

mm/memcontrol.c +0 -0

index f979ac7..855d335 100644
@@ -2625,6 +2625,184 @@ static inline bool memcg_has_children(struct mem_cgroup *memcg)
 	return ret;
 }
 

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

Quoted 8 lines

mm/memcontrol.c +8 -0

+static long mem_cgroup_oom_badness(struct mem_cgroup *memcg,
+				   const nodemask_t *nodemask)
+{
+	long points = 0;
+	int nid;
+	struct mem_cgroup *iter;
+
+	for_each_mem_cgroup_tree(iter, memcg) {

AFAIU this function is called on every iteration over the cgroup tree, which might be costly in case of a deep hierarchy, as it has quadratic complexity at worst. We could eliminate the nested loop by computing badness of all eligible cgroups before starting looking for a victim and saving the values in struct mem_cgroup. Not sure if it's worth it, as OOM is a pretty cold path.

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

Quoted 7 lines

mm/memcontrol.c +6 -0

+		for_each_node_state(nid, N_MEMORY) {
+			if (nodemask && !node_isset(nid, *nodemask))
+				continue;
+
+			points += mem_cgroup_node_nr_lru_pages(iter, nid,
+					LRU_ALL_ANON | BIT(LRU_UNEVICTABLE));

Hmm, is there a reason why we shouldn't take into account file pages?

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

Quoted 4 lines

mm/memcontrol.c +3 -0

+		}
+
+		points += mem_cgroup_get_nr_swap_pages(iter);

AFAICS mem_cgroup_get_nr_swap_pages() returns the number of pages that can still be charged to the cgroup. IIUC we want to account pages that have already been charged to the cgroup, i.e. the value of the 'swap' page counter or MEMCG_SWAP stat counter.

Quoted 48 lines

mm/memcontrol.c +47 -0

+		points += memcg_page_state(iter, MEMCG_KERNEL_STACK_KB) /
+			(PAGE_SIZE / 1024);
+		points += memcg_page_state(iter, MEMCG_SLAB_UNRECLAIMABLE);
+		points += memcg_page_state(iter, MEMCG_SOCK);
+	}
+
+	return points;
+}
+
+bool mem_cgroup_select_oom_victim(struct oom_control *oc)
+{
+	struct cgroup_subsys_state *css = NULL;
+	struct mem_cgroup *iter = NULL;
+	struct mem_cgroup *chosen_memcg = NULL;
+	struct mem_cgroup *parent = root_mem_cgroup;
+	unsigned long totalpages = oc->totalpages;
+	long chosen_memcg_points = 0;
+	long points = 0;
+
+	oc->chosen = NULL;
+	oc->chosen_memcg = NULL;
+
+	if (mem_cgroup_disabled())
+		return false;
+
+	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
+		return false;
+
+	pr_info("Choosing a victim memcg because of the %s",
+		oc->memcg ?
+		"memory limit reached of cgroup " :
+		"system-wide OOM\n");
+	if (oc->memcg) {
+		pr_cont_cgroup_path(oc->memcg->css.cgroup);
+		pr_cont("\n");
+
+		chosen_memcg = oc->memcg;
+		parent = oc->memcg;
+	}
+
+	rcu_read_lock();
+
+	for (;;) {
+		css = css_next_child(css, &parent->css);
+		if (css) {
+			iter = mem_cgroup_from_css(css);
+

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

Quoted 6 lines

mm/memcontrol.c +6 -0

+			points = mem_cgroup_oom_badness(iter, oc->nodemask);
+			points += iter->oom_score_adj * (totalpages / 1000);
+
+			pr_info("Cgroup ");
+			pr_cont_cgroup_path(iter->css.cgroup);
+			pr_cont(": %ld\n", points);

Not sure if everyone wants to see these messages in the log.

Quoted 11 lines

mm/memcontrol.c +10 -0

+
+			if (points > chosen_memcg_points) {
+				chosen_memcg = iter;
+				chosen_memcg_points = points;
+				oc->chosen_points = points;
+			}
+
+			continue;
+		}
+

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

Quoted 3 lines

mm/memcontrol.c +3 -0

+		if (chosen_memcg && !chosen_memcg->oom_kill_all_tasks) {
+			/* Go deeper in the cgroup hierarchy */
+			totalpages = chosen_memcg_points;

We set 'totalpages' to the target cgroup limit (or the total RAM size) when computing a victim score. Why do you prefer to use chosen_memcg_points here instead? Why not the limit of the chosen cgroup?

Quoted 26 lines

mm/memcontrol.c +25 -0

+			chosen_memcg_points = 0;
+
+			parent = chosen_memcg;
+			chosen_memcg = NULL;
+
+			continue;
+		}
+
+		if (!chosen_memcg && parent != root_mem_cgroup)
+			chosen_memcg = parent;
+
+		break;
+	}
+
 
+	if (!oc->memcg) {
+		/*
+		 * We should also consider tasks in the root cgroup
+		 * with badness larger than oc->chosen_points
+		 */
+
+		struct css_task_iter it;
+		struct task_struct *task;
+		int ret = 0;
+
+		css_task_iter_start(&root_mem_cgroup->css, &it);

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

Quoted 4 lines

mm/memcontrol.c +4 -0

+		while (!ret && (task = css_task_iter_next(&it)))
+			ret = oom_evaluate_task(task, oc);
+		css_task_iter_end(&it);
+	}

IMHO it isn't quite correct to compare tasks from the root cgroup with leaf cgroups, because they are at different levels. Shouldn't we compare their scores only with the top level cgroups?

As an alternative approach, may be, we could remove this branch altogether and ignore root tasks here (i.e. have any root task a higher priority a priori)? Perhaps, it could be acceptable, because normally the root cgroup only hosts kernel processes and init (at least this is the default systemd setup IIRC).

Quoted 7 lines

mm/memcontrol.c +7 -0

+
+	if (!oc->chosen && chosen_memcg) {
+		pr_info("Chosen cgroup ");
+		pr_cont_cgroup_path(chosen_memcg->css.cgroup);
+		pr_cont(": %ld\n", oc->chosen_points);
+
+		if (chosen_memcg->oom_kill_all_tasks) {

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

Quoted 10 lines

mm/memcontrol.c +9 -0

+			css_get(&chosen_memcg->css);
+			oc->chosen_memcg = chosen_memcg;
+		} else {
+			/*
+			 * If we don't need to kill all tasks in the cgroup,
+			 * let's select the biggest task.
+			 */
+			oc->chosen_points = 0;
 
+			select_bad_process(oc, chosen_memcg);

I think we'd better use mem_cgroup_scan_task() here directly, without exporting select_bad_process() from oom_kill.c. IMHO it would be more straightforward, because select_bad_process() has a branch handling the global OOM, which isn't used in this case. Come to think of it, wouldn't it be better to return the chosen cgroup in @oc and let out_of_memory() select a process within it or kill it as a whole depending on the value of the oom_kill_all_tasks flag?

Also, if the chosen cgroup has no tasks (which is perfectly possible if all memory within the cgroup is consumed by shmem e.g.), shouldn't we retry the cgroup selection?

Quoted 12 lines

mm/memcontrol.c +11 -0

+		}
+	} else if (oc->chosen)
+		pr_info("Chosen task %s (%d) in root cgroup: %ld\n",
+			oc->chosen->comm, oc->chosen->pid, oc->chosen_points);
+
+	rcu_read_unlock();
+
+	oc->chosen_points = 0;
+	return !!oc->chosen || !!oc->chosen_memcg;
+}
+

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

Quoted 10 lines

mm/memcontrol.c +10 -0

+static int __oom_kill_task(struct task_struct *tsk, void *arg)
+{
+	if (!is_global_init(tsk) && !(tsk->flags & PF_KTHREAD)) {
+		get_task_struct(tsk);
+		__oom_kill_process(tsk);
+	}
+	return 0;
+}
+
+bool mem_cgroup_kill_oom_victim(struct oom_control *oc)

I think it'd be OK to define this function in oom_kill.c - we have everything we need for that. We wouldn't have to export __oom_kill_process without oom_kill_process then, which is kinda ugly IMHO.

Quoted 16 lines

mm/memcontrol.c +16 -0

+{
+	if (oc->chosen_memcg) {
+		/*
+		 * Kill all tasks in the cgroup hierarchy
+		 */
+		mem_cgroup_scan_tasks(oc->chosen_memcg,
+				      __oom_kill_task, NULL);
+
+		/*
+		 * Release oc->chosen_memcg
+		 */
+		css_put(&oc->chosen_memcg->css);
+		oc->chosen_memcg = NULL;
+	}
+
+	if (oc->chosen && oc->chosen != (void *)-1UL) {

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

Quoted 1 line

mm/memcontrol.c +1 -0

+		__oom_kill_process(oc->chosen);

Why don't you use oom_kill_process (without leading underscores) here?

This patchset makes the OOM killer cgroup-aware.

Patches 1-3 are simple refactorings of the OOM killer code, required to reuse the code in the memory controller. Patches 4 & 5 are introducing new memcg settings:
oom_kill_all_tasks and oom_score_adj.
Patch 6 introduces the cgroup-aware OOM killer. Patch 7 is docs update.

Quoted 8 lines · Roman Gushchin

This patchset makes the OOM killer cgroup-aware.

Patches 1-3 are simple refactorings of the OOM killer code, required to reuse the code in the memory controller. Patches 4 & 5 are introducing new memcg settings:
oom_kill_all_tasks and oom_score_adj.
Patch 6 introduces the cgroup-aware OOM killer. Patch 7 is docs update.

I have only had a look at the cumulative diff (sorry I've been quite busy throughout the week) and here are my high level comments. I can see few rather serious issues which will need to be resolved before this can move on.
- the first problem is a pre-existing one but it will get more urgent with the fact that more tasks will be killed with your approach. OOM victims are allowed to consume memory reserves without any bound. The current throttling is quite arguable and it relies on the fact that we try to limit the number of tasks to have this access to reserves. Theoretically, though, a heavily multithread application can deplete the reserves completely even now. With more processes being killed this

will get much more likely. Johannes and me have already posted patches to address that. The last patch was
http://lkml.kernel.org/r/[email protected]
- I do not see any explicit lockout mechanism to prevent from too eager oom invocation while the previous oom killed memcg is still not torn down completely. We use tsk_is_oom_victim check in oom_evaluate_task for that purpose. You seem to rely on the fact that such a memcg would be still the largest one, right? I am not really sure this is sufficient.

- You seem to completely ignore per task oom_score_adj and override it by the memcg value. This makes some sense but it can lead to an unexpected behavior when somebody relies on the original behavior. E.g. a workload that would corrupt data when killed unexpectedly and so it is protected by OOM_SCORE_ADJ_MIN. Now this assumption will break when running inside a container. I do not have a good answer what is the desirable behavior and maybe there is no universal answer. Maybe you just do not to kill those tasks? But then you have to be careful when selecting a memcg victim. Hairy...

- While we are at it oom_score_adj has turned out to be quite unusable for a sensible oom prioritization from my experience. Practically it reduced to disable/enforce the task for selection. The scale is quite small as well. There certainly is a need for prioritization and maybe a completely different api would be better. Maybe a simple priority in (0, infinity) range will be better. Priority could be used either as the only criterion or as a tie breaker when consumption of more memcgs is too close (this could be implemented for each strategy in a different way if we go modules way)

- oom_kill_all_tasks should be hierarchical and consistent within a hierarchy. Or maybe it should be applicable to memcgs with tasks (leaf cgroups). Although selecting a memcg higher in the hierarchy kill all tasks in that hierarchy makes some sense as well IMHO. Say you delegate a hierarchy to an unprivileged user and still want to contain that user.

Hi, Michal!

Thank you very much for the review. I've tried to address your comments in v3 (sent yesterday), so that is why it took some time to reply.

Quoted 9 lines

- You seem to completely ignore per task oom_score_adj and override it by the memcg value. This makes some sense but it can lead to an unexpected behavior when somebody relies on the original behavior. E.g. a workload that would corrupt data when killed unexpectedly and so it is protected by OOM_SCORE_ADJ_MIN. Now this assumption will break when running inside a container. I do not have a good answer what is the desirable behavior and maybe there is no universal answer. Maybe you just do not to kill those tasks? But then you have to be careful when selecting a memcg victim. Hairy...

I do not ignore it completely, but it matters only for root cgroup tasks and inside a cgroup when oom_kill_all_tasks is off.

I believe, that cgroup v2 requirement is a good enough. I mean you can't move from v1 to v2 without changing cgroup settings, and if we will provide per-cgroup oom_score_adj, it will be enough to reproduce the old behavior.

Also, if you think it's necessary, I can add a sysctl to turn the cgroup-aware oom killer off completely and provide compatibility mode.
We can't really save the old system-wide behavior of per-process oom_score_adj, it makes no sense in the containerized environment.

Quoted 9 lines

- While we are at it oom_score_adj has turned out to be quite unusable for a sensible oom prioritization from my experience. Practically it reduced to disable/enforce the task for selection. The scale is quite small as well. There certainly is a need for prioritization and maybe a completely different api would be better. Maybe a simple priority in (0, infinity) range will be better. Priority could be used either as the only criterion or as a tie breaker when consumption of more memcgs is too close (this could be implemented for each strategy in a different way if we go modules way)

I had such a version, but the the question how to compare root cgroup tasks and cgroups becomes even harder. But if you have any ideas here, it's just great. I do not like this [-1000, 1000] range at all.

Quoted 6 lines

- oom_kill_all_tasks should be hierarchical and consistent within a hierarchy. Or maybe it should be applicable to memcgs with tasks (leaf cgroups). Although selecting a memcg higher in the hierarchy kill all tasks in that hierarchy makes some sense as well IMHO. Say you delegate a hierarchy to an unprivileged user and still want to contain that user.

It's already hierarchical, or did I missed something? Please, explain what you mean. If turned on for any cgroup (except root), it forces oom killer to treat the whole cgroup as a single enitity, and kill all belonging tasks if it is selected to be the oom victim.

Quoted 4 lines · Roman Gushchin

Hi, Michal!

Thank you very much for the review. I've tried to address your comments in v3 (sent yesterday), so that is why it took some time to reply.

I will try to look at it sometimes next week hopefully

Quoted 13 lines

Quoted 9 lines

- You seem to completely ignore per task oom_score_adj and override it by the memcg value. This makes some sense but it can lead to an unexpected behavior when somebody relies on the original behavior. E.g. a workload that would corrupt data when killed unexpectedly and so it is protected by OOM_SCORE_ADJ_MIN. Now this assumption will break when running inside a container. I do not have a good answer what is the desirable behavior and maybe there is no universal answer. Maybe you just do not to kill those tasks? But then you have to be careful when selecting a memcg victim. Hairy...

I do not ignore it completely, but it matters only for root cgroup tasks and inside a cgroup when oom_kill_all_tasks is off.

Quoted 8 lines

I believe, that cgroup v2 requirement is a good enough. I mean you can't move from v1 to v2 without changing cgroup settings, and if we will provide per-cgroup oom_score_adj, it will be enough to reproduce the old behavior.

Also, if you think it's necessary, I can add a sysctl to turn the cgroup-aware oom killer off completely and provide compatibility mode.
We can't really save the old system-wide behavior of per-process oom_score_adj, it makes no sense in the containerized environment.

So what you are going to do with those applications that simply cannot be killed and which set OOM_SCORE_ADJ_MIN explicitly. Are they unsupported? How does a user find out? One way around this could be to simply to not kill tasks with OOM_SCORE_ADJ_MIN.

Quoted 7 lines · Michal Hocko

Quoted 4 lines · Roman Gushchin

Hi, Michal!

Thank you very much for the review. I've tried to address your comments in v3 (sent yesterday), so that is why it took some time to reply.

I will try to look at it sometimes next week hopefully

Thanks!

Quoted 22 lines

Quoted 21 lines

Quoted 9 lines

- You seem to completely ignore per task oom_score_adj and override it by the memcg value. This makes some sense but it can lead to an unexpected behavior when somebody relies on the original behavior. E.g. a workload that would corrupt data when killed unexpectedly and so it is protected by OOM_SCORE_ADJ_MIN. Now this assumption will break when running inside a container. I do not have a good answer what is the desirable behavior and maybe there is no universal answer. Maybe you just do not to kill those tasks? But then you have to be careful when selecting a memcg victim. Hairy...

I do not ignore it completely, but it matters only for root cgroup tasks and inside a cgroup when oom_kill_all_tasks is off.

I believe, that cgroup v2 requirement is a good enough. I mean you can't move from v1 to v2 without changing cgroup settings, and if we will provide per-cgroup oom_score_adj, it will be enough to reproduce the old behavior.

Also, if you think it's necessary, I can add a sysctl to turn the cgroup-aware oom killer off completely and provide compatibility mode.
We can't really save the old system-wide behavior of per-process oom_score_adj, it makes no sense in the containerized environment.

Quoted 4 lines

So what you are going to do with those applications that simply cannot be killed and which set OOM_SCORE_ADJ_MIN explicitly. Are they unsupported? How does a user find out? One way around this could be to simply to not kill tasks with OOM_SCORE_ADJ_MIN.

They won't be killed by cgroup OOM, but under some circumstances can be killed by the global OOM (e.g. there are no other tasks in the selected cgroup, cgroup v2 is used, and per-cgroup oom score adjustment is not set).

I believe, that per-process oom_score_adj should not play any role outside of the containing cgroup, it's violation of isolation.

Right now if tasks with oom_score_adj=-1000 eating all memory in a cgroup, they will be looping forever, OOM killer can't fix this.