[RFC v2 0/2] introduce new attribute "priority" to control group

[RFC v2 0/2] introduce new attribute "priority" to control group

[yulei.kernel-Re5JQEeQqe8AvxtiuMwx3w]

From: Yulei Zhang <yuleixzhang-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>

Last time we introduce the idea of adding prioritized tasks management
to control group. Sometimes we may assign the same amount of resources
to multiple cgroups due to the environment restriction, but we still 
hope there are preference order to handle the tasks in those cgroups,
the 'prio' attribute may help to do the ranking jobs. 

The default value of "priority" is set to 0 which means the highest
priority, and the totally levels of priority is defined by
CGROUP_PRIORITY_MAX. Each subsystem could register callback to receive the
priority change notification for their own purposes. 

In this v2 patch we apply a simple rule to the oom hanlder base on the
order of priority to demonstrate the intention about adding this attribute.
When enable the prioritized oom, it will perfer to pick up the victim from the
memory cgroup with lower priority, and try the best to keep the tasks
alive in high ranked memcg.

V2->V1:
1. Introduce prioritized oom in memcg.

Lei Chen (1):
  cgroup: add support for cgroup priority

Yulei Zhang (1):
  memcg: introduce prioritized oom in memcg

 include/linux/cgroup-defs.h |  2 +
 include/linux/cgroup.h      |  2 +
 include/linux/memcontrol.h  | 38 ++++++++++++++
 include/linux/oom.h         |  1 +
 kernel/cgroup/cgroup.c      | 90 +++++++++++++++++++++++++++++++++
 mm/memcontrol.c             | 99 ++++++++++++++++++++++++++++++++++++-
 mm/oom_kill.c               |  6 +--
 7 files changed, 233 insertions(+), 5 deletions(-)

-- 
2.28.0

[RFC v2 1/2] cgroup: add support for cgroup priority

[yulei.kernel-Re5JQEeQqe8AvxtiuMwx3w]

From: Lei Chen <lennychen-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>

Introduce new attribute "priority" to control group, which
could be used as scale by subssytem to manipulate the behaviors
of processes.
The default value of "priority" is set to 0 which means the
highest priority, and the totally levels of priority is defined
by CGROUP_PRIORITY_MAX.

Signed-off-by: Lei Chen <lennychen-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Liu Yu <allanyuliu-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Peng Zhiguang <zgpeng-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Yulei Zhang <yuleixzhang-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
---
 include/linux/cgroup-defs.h |  2 +
 include/linux/cgroup.h      |  2 +
 kernel/cgroup/cgroup.c      | 90 +++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 559ee05f8..3fa2f28a9 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -417,6 +417,7 @@ struct cgroup {
 	u16 subtree_ss_mask;
 	u16 old_subtree_control;
 	u16 old_subtree_ss_mask;
+	u16 priority;
 
 	/* Private pointers for each registered subsystem */
 	struct cgroup_subsys_state __rcu *subsys[CGROUP_SUBSYS_COUNT];
@@ -640,6 +641,7 @@ struct cgroup_subsys {
 	void (*exit)(struct task_struct *task);
 	void (*release)(struct task_struct *task);
 	void (*bind)(struct cgroup_subsys_state *root_css);
+	int (*css_priority_change)(struct cgroup_subsys_state *css, u16 old, u16 new);
 
 	bool early_init:1;
 
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 4f2f79de0..734d51aba 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -47,6 +47,7 @@ struct kernel_clone_args;
 
 /* internal flags */
 #define CSS_TASK_ITER_SKIPPED		(1U << 16)
+#define CGROUP_PRIORITY_MAX		8
 
 /* a css_task_iter should be treated as an opaque object */
 struct css_task_iter {
@@ -957,5 +958,6 @@ static inline void cgroup_bpf_get(struct cgroup *cgrp) {}
 static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
 
 #endif /* CONFIG_CGROUP_BPF */
+ssize_t cgroup_priority(struct cgroup_subsys_state *css);
 
 #endif /* _LINUX_CGROUP_H */
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 9153b20e5..aa019ad24 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1892,6 +1892,7 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
 	cgrp->dom_cgrp = cgrp;
 	cgrp->max_descendants = INT_MAX;
 	cgrp->max_depth = INT_MAX;
+	cgrp->priority = 0;
 	INIT_LIST_HEAD(&cgrp->rstat_css_list);
 	prev_cputime_init(&cgrp->prev_cputime);
 
@@ -4783,6 +4784,88 @@ static ssize_t cgroup_threads_write(struct kernfs_open_file *of,
 	return __cgroup_procs_write(of, buf, false) ?: nbytes;
 }
 
+static int cgroup_priority_show(struct seq_file *seq, void *v)
+{
+	struct cgroup *cgrp = seq_css(seq)->cgroup;
+	u16 prio = cgrp->priority;
+
+	seq_printf(seq, "%d\n", prio);
+
+	return 0;
+}
+
+static void cgroup_set_priority(struct cgroup *cgrp, unsigned int priority)
+{
+	u16 old = cgrp->priority;
+	struct cgroup_subsys_state *css;
+	int ssid;
+
+	cgrp->priority = priority;
+	for_each_css(css, ssid, cgrp) {
+		if (css->ss->css_priority_change)
+			css->ss->css_priority_change(css, old, priority);
+	}
+}
+
+static void cgroup_priority_propagate(struct cgroup *cgrp)
+{
+	struct cgroup *dsct;
+	struct cgroup_subsys_state *d_css;
+	u16 priority = cgrp->priority;
+
+	lockdep_assert_held(&cgroup_mutex);
+	cgroup_for_each_live_descendant_pre(dsct, d_css, cgrp) {
+		if (dsct->priority < priority)
+			cgroup_set_priority(dsct, priority);
+	}
+}
+
+static ssize_t cgroup_priority_write(struct kernfs_open_file *of,
+				      char *buf, size_t nbytes, loff_t off)
+{
+	struct cgroup *cgrp, *parent;
+	ssize_t ret;
+	u16 prio, orig;
+
+	buf = strstrip(buf);
+	ret = kstrtou16(buf, 0, &prio);
+	if (ret)
+		return ret;
+
+	if (prio < 0 || prio >= CGROUP_PRIORITY_MAX)
+		return -ERANGE;
+
+	cgrp = cgroup_kn_lock_live(of->kn, false);
+	if (!cgrp)
+		return -ENOENT;
+	parent = cgroup_parent(cgrp);
+	if (parent && prio < parent->priority) {
+		ret = -EINVAL;
+		goto unlock_out;
+	}
+	orig = cgrp->priority;
+	if (prio == orig)
+		goto unlock_out;
+
+	cgroup_set_priority(cgrp, prio);
+	cgroup_priority_propagate(cgrp);
+unlock_out:
+	cgroup_kn_unlock(of->kn);
+
+	return ret ?: nbytes;
+}
+
+ssize_t cgroup_priority(struct cgroup_subsys_state *css)
+{
+	struct cgroup *cgrp = css->cgroup;
+	unsigned int prio = 0;
+
+	if (cgrp)
+		prio = cgrp->priority;
+	return prio;
+}
+EXPORT_SYMBOL(cgroup_priority);
+
 /* cgroup core interface files for the default hierarchy */
 static struct cftype cgroup_base_files[] = {
 	{
@@ -4836,6 +4919,12 @@ static struct cftype cgroup_base_files[] = {
 		.seq_show = cgroup_max_depth_show,
 		.write = cgroup_max_depth_write,
 	},
+	{
+		.name = "cgroup.priority",
+		.flags = CFTYPE_NOT_ON_ROOT,
+		.seq_show = cgroup_priority_show,
+		.write = cgroup_priority_write,
+	},
 	{
 		.name = "cgroup.stat",
 		.seq_show = cgroup_stat_show,
@@ -5178,6 +5267,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
 	cgrp->self.parent = &parent->self;
 	cgrp->root = root;
 	cgrp->level = level;
+	cgrp->priority = parent->priority;
 
 	ret = psi_cgroup_alloc(cgrp);
 	if (ret)
-- 
2.28.0

[RFC v2 2/2] memcg: introduce prioritized oom in memcg

[yulei.kernel-Re5JQEeQqe8AvxtiuMwx3w]

From: Yulei Zhang <yuleixzhang-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>

In current implementation when memcg oom happens it will select
victim from the memcg hierarchy, with the assistant of priority
attribute in the first patch, it will be able to select the
victim in memcg with the order from low to high priority,
and try the best to keep the high priority tasks survived from oom.

A new trigger memory.use_priority_oom is introduced to contorl
the enabling of the policy. And for global oom it will check the
memory.use_priority_oom setting in root memcg, by default this
is disabled.

Signed-off-by: Haiwei Li <gerryhwli-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Yulei Zhang <yuleixzhang-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
---
 include/linux/memcontrol.h | 38 +++++++++++++++
 include/linux/oom.h        |  1 +
 mm/memcontrol.c            | 99 +++++++++++++++++++++++++++++++++++++-
 mm/oom_kill.c              |  6 +--
 4 files changed, 139 insertions(+), 5 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 0c04d39a7..68e1acf64 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -27,6 +27,7 @@ struct obj_cgroup;
 struct page;
 struct mm_struct;
 struct kmem_cache;
+struct oom_control;
 
 /* Cgroup-specific page state, on top of universal node page state */
 enum memcg_stat_item {
@@ -253,6 +254,9 @@ struct mem_cgroup {
 	bool		oom_lock;
 	int		under_oom;
 
+	/* memcg priority */
+	bool use_priority_oom;
+
 	int	swappiness;
 	/* OOM-Killer disable */
 	int		oom_kill_disable;
@@ -812,6 +816,24 @@ static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
 	return !!(memcg->css.flags & CSS_ONLINE);
 }
 
+/*
+ * memcg priority
+ */
+void mem_cgroup_select_bad_process(struct oom_control *oc);
+
+void mem_cgroup_oom_select_bad_process(struct oom_control *oc);
+
+static inline bool root_memcg_use_priority_oom(void)
+{
+	if (mem_cgroup_disabled())
+		return false;
+
+	if (root_mem_cgroup->use_priority_oom)
+		return true;
+
+	return false;
+}
+
 /*
  * For memory reclaim.
  */
@@ -1262,6 +1284,22 @@ static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
 	return true;
 }
 
+/*
+ * memcg priority
+ */
+static inline void mem_cgroup_select_bad_process(struct oom_control *oc)
+{
+}
+
+static inline void mem_cgroup_oom_select_bad_process(struct oom_control *oc)
+{
+}
+
+static inline bool root_memcg_use_priority_oom(void)
+{
+	return false;
+}
+
 static inline
 unsigned long mem_cgroup_get_zone_lru_size(struct lruvec *lruvec,
 		enum lru_list lru, int zone_idx)
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 2db9a1432..a47e455ab 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -121,6 +121,7 @@ extern int unregister_oom_notifier(struct notifier_block *nb);
 extern bool oom_killer_disable(signed long timeout);
 extern void oom_killer_enable(void);
 
+extern int oom_evaluate_task(struct task_struct *task, void *arg);
 extern struct task_struct *find_lock_task_mm(struct task_struct *p);
 
 /* sysctls */
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e064ac0d8..30c8aae17 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1284,8 +1284,6 @@ int mem_cgroup_scan_tasks(struct mem_cgroup *memcg,
 	struct mem_cgroup *iter;
 	int ret = 0;
 
-	BUG_ON(memcg == root_mem_cgroup);
-
 	for_each_mem_cgroup_tree(iter, memcg) {
 		struct css_task_iter it;
 		struct task_struct *task;
@@ -1302,6 +1300,73 @@ int mem_cgroup_scan_tasks(struct mem_cgroup *memcg,
 	return ret;
 }
 
+int mem_cgroup_select_victim(struct cgroup_subsys_state *css, struct oom_control *oc)
+{
+	struct css_task_iter it;
+	struct task_struct *task;
+	int ret = 0;
+
+	css_task_iter_start(css, CSS_TASK_ITER_PROCS, &it);
+	while (!ret && (task = css_task_iter_next(&it)))
+		ret = oom_evaluate_task(task, oc);
+	css_task_iter_end(&it);
+
+	return ret;
+}
+
+void mem_cgroup_priority_select_task(struct oom_control *oc)
+{
+	struct cgroup_subsys_state *chosen = NULL;
+	struct mem_cgroup *memcg, *chosen_memcg, *iter;
+	int chosen_priority = -1;
+	int ret = 0;
+
+	memcg = oc->memcg;
+	if (!memcg)
+		memcg = root_mem_cgroup;
+
+	for_each_mem_cgroup_tree(iter, memcg) {
+		struct cgroup_subsys_state *css = &iter->css;
+		int prio = cgroup_priority(css);
+
+		if (prio < chosen_priority)
+			continue;
+
+		else if (prio > chosen_priority || !chosen) {
+			chosen_priority = prio;
+			chosen_memcg = iter;
+			chosen = css;
+		} else {
+			/* equal priority check memory usage */
+			if (do_memsw_account()) {
+				if (page_counter_read(&iter->memsw) <=
+					page_counter_read(&chosen_memcg->memsw))
+					continue;
+			} else if (page_counter_read(&iter->memory) <=
+				page_counter_read(&chosen_memcg->memory))
+				continue;
+			chosen = css;
+		}
+		ret = mem_cgroup_select_victim(chosen, oc);
+		if (!ret && oc->chosen)
+			chosen_memcg = iter;
+	}
+}
+
+void mem_cgroup_oom_select_bad_process(struct oom_control *oc)
+{
+	struct mem_cgroup *memcg;
+
+	memcg = oc->memcg;
+	if (!memcg)
+		memcg = root_mem_cgroup;
+
+	if (memcg->use_priority_oom)
+		mem_cgroup_priority_select_task(oc);
+	else
+		mem_cgroup_scan_tasks(memcg, oom_evaluate_task, oc);
+}
+
 #ifdef CONFIG_DEBUG_VM
 void lruvec_memcg_debug(struct lruvec *lruvec, struct page *page)
 {
@@ -3544,6 +3609,26 @@ static int mem_cgroup_hierarchy_write(struct cgroup_subsys_state *css,
 	return -EINVAL;
 }
 
+static u64 mem_cgroup_priority_oom_read(struct cgroup_subsys_state *css,
+					struct cftype *cft)
+{
+	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
+
+	return memcg->use_priority_oom;
+}
+
+static int mem_cgroup_priority_oom_write(struct cgroup_subsys_state *css,
+					struct cftype *cft, u64 val)
+{
+	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
+
+	if (val > 1)
+		return -EINVAL;
+
+	memcg->use_priority_oom = val;
+	return 0;
+}
+
 static unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap)
 {
 	unsigned long val;
@@ -4991,6 +5076,11 @@ static struct cftype mem_cgroup_legacy_files[] = {
 		.write_u64 = mem_cgroup_hierarchy_write,
 		.read_u64 = mem_cgroup_hierarchy_read,
 	},
+	{
+		.name = "use_priority_oom",
+		.write_u64 = mem_cgroup_priority_oom_write,
+		.read_u64 = mem_cgroup_priority_oom_read,
+	},
 	{
 		.name = "cgroup.event_control",		/* XXX: for compat */
 		.write = memcg_write_event_control,
@@ -6463,6 +6553,11 @@ static struct cftype memory_files[] = {
 		.seq_show = memory_max_show,
 		.write = memory_max_write,
 	},
+	{
+		.name = "use_priority_oom",
+		.write_u64 = mem_cgroup_priority_oom_write,
+		.read_u64 = mem_cgroup_priority_oom_read,
+	},
 	{
 		.name = "events",
 		.flags = CFTYPE_NOT_ON_ROOT,
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 9efaf430c..d5484f157 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -307,7 +307,7 @@ static enum oom_constraint constrained_alloc(struct oom_control *oc)
 	return CONSTRAINT_NONE;
 }
 
-static int oom_evaluate_task(struct task_struct *task, void *arg)
+int oom_evaluate_task(struct task_struct *task, void *arg)
 {
 	struct oom_control *oc = arg;
 	long points;
@@ -367,8 +367,8 @@ static void select_bad_process(struct oom_control *oc)
 {
 	oc->chosen_points = LONG_MIN;
 
-	if (is_memcg_oom(oc))
-		mem_cgroup_scan_tasks(oc->memcg, oom_evaluate_task, oc);
+	if (is_memcg_oom(oc) || root_memcg_use_priority_oom())
+		mem_cgroup_oom_select_bad_process(oc);
 	else {
 		struct task_struct *p;
 
-- 
2.28.0

Re: [RFC v2 0/2] introduce new attribute "priority" to control group

[Tejun Heo]

On Tue, Apr 13, 2021 at 12:40:09AM +0800, yulei.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> From: Yulei Zhang <yuleixzhang-1Nz4purKYjRBDgjK7y7TUQ@public.gmane.org>
> 
> Last time we introduce the idea of adding prioritized tasks management
> to control group. Sometimes we may assign the same amount of resources
> to multiple cgroups due to the environment restriction, but we still 
> hope there are preference order to handle the tasks in those cgroups,
> the 'prio' attribute may help to do the ranking jobs. 
> 
> The default value of "priority" is set to 0 which means the highest
> priority, and the totally levels of priority is defined by
> CGROUP_PRIORITY_MAX. Each subsystem could register callback to receive the
> priority change notification for their own purposes. 
> 
> In this v2 patch we apply a simple rule to the oom hanlder base on the
> order of priority to demonstrate the intention about adding this attribute.
> When enable the prioritized oom, it will perfer to pick up the victim from the
> memory cgroup with lower priority, and try the best to keep the tasks
> alive in high ranked memcg.

I don't think the discussion in the previous thread led anywhere. Please
consider this series nacked for now.

Thanks.

-- 
tejun