[PATCH] mm: vmscan: memcg: Add global shrink priority

[PATCH] mm: vmscan: memcg: Add global shrink priority

[Hui Zhu]

Currently, memcg has some config to limit memory usage and config
the shrink behavior.
In the memory-constrained environment, put different priority tasks
into different cgroups with different memory limits to protect the
performance of the high priority tasks.  Because the global memory
shrink will affect the performance of all tasks.  The memory limit
cgroup can make shrink happen inside the cgroup.  Then it can decrease
the memory shrink of the high priority task to protect its performance.

But the memory footprint of the task is not static.  It will change as
the working pressure changes.  And the version changes will affect it too.
Then set the appropriate memory limit to decrease the global memory shrink
is a difficult job and lead to wasted memory or performance loss sometimes.

This commit adds global shrink priority to memcg to try to handle this
problem.
The default global shrink priority of each cgroup is DEF_PRIORITY.
Its behavior in global shrink is not changed.
And when global shrink priority of a cgroup is smaller than DEF_PRIORITY,
its memory will be shrink when memcg->global_shrink_priority greater than
or equal to sc->priority.

The following is an example to use global shrink priority in a VM that
has 2 CPUs, 1G memory and 4G swap:
 # These are test shells that call usemem that get from
 # https://git.kernel.org/pub/scm/linux/kernel/git/wfg/vm-scalability.git
cat 1.sh
sleep 9999
 # -s 3600: Sleep 3600 seconds after test complete then usemem will
 # not release the memory at once.
 # -Z:  read memory again after access the memory.
 # The first time access memory need shrink memory to allocate page.
 # Then the access speed of high priority will not increase a lot.
 # The read again speed of high priority will increase.
 # $((850 * 1024 * 1024 + 8)): Different sizes are used to distinguish
 # the results of the two tests.
usemem -s 3600 -Z -a -n 1 $((850 * 1024 * 1024 + 8))
cat 2.sh
sleep 9999
usemem -s 3600 -Z -a -n 1 $((850 * 1024 * 1024))

 # Setup swap
swapon /swapfile
 # Setup 2 cgroups
mkdir /sys/fs/cgroup/memory/t1/
mkdir /sys/fs/cgroup/memory/t2/

 # Run tests with same global shrink priority
cat /sys/fs/cgroup/memory/t1/memory.global_shrink_priority
12
cat /sys/fs/cgroup/memory/t2/memory.global_shrink_priority
12
echo $$ > /sys/fs/cgroup/memory/t1/cgroup.procs
sh 1.sh &
echo $$ > /sys/fs/cgroup/memory/t2/cgroup.procs
sh 2.sh &
echo $$ > /sys/fs/cgroup/memory/cgroup.procs
killall sleep
 # This the test results
1002700800 bytes / 2360359 usecs = 414852 KB/s
1002700809 bytes / 2676181 usecs = 365894 KB/s
read again 891289600 bytes / 13515142 usecs = 64401 KB/s
read again 891289608 bytes / 13252268 usecs = 65679 KB/s
killall usemem

 # Run tests with 12 and 8
cat /sys/fs/cgroup/memory/t1/memory.global_shrink_priority
12
echo 8 > /sys/fs/cgroup/memory/t2/memory.global_shrink_priority
echo $$ > /sys/fs/cgroup/memory/t1/cgroup.procs
sh 1.sh &
echo $$ > /sys/fs/cgroup/memory/t2/cgroup.procs
sh 2.sh &
echo $$ > /sys/fs/cgroup/memory/cgroup.procs
killall sleep
 # This the test results
1002700800 bytes / 1809056 usecs = 541276 KB/s
1002700809 bytes / 2184337 usecs = 448282 KB/s
read again 891289600 bytes / 6666224 usecs = 130568 KB/s
read again 891289608 bytes / 9171440 usecs = 94903 KB/s
killall usemem

 # This is the test results of 12 and 6
1002700800 bytes / 1827914 usecs = 535692 KB/s
1002700809 bytes / 2135124 usecs = 458615 KB/s
read again 891289600 bytes / 1498419 usecs = 580878 KB/s
read again 891289608 bytes / 7328362 usecs = 118771 KB/s

Signed-off-by: Hui Zhu <teawaterz@linux.alibaba.com>
---
 include/linux/memcontrol.h |  2 ++
 mm/memcontrol.c            | 32 ++++++++++++++++++++++++++++++++
 mm/vmscan.c                | 39 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index a7a0a1a5..8ad2437 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -244,6 +244,8 @@ struct mem_cgroup {
 	/* OOM-Killer disable */
 	int		oom_kill_disable;
 
+	s8 global_shrink_priority;
+
 	/* memory.events and memory.events.local */
 	struct cgroup_file events_file;
 	struct cgroup_file events_local_file;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c5b5f74..39fdc84 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4646,6 +4646,32 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
 	return ret;
 }
 
+static ssize_t mem_global_shrink_priority_write(struct kernfs_open_file *of,
+				char *buf, size_t nbytes, loff_t off)
+{
+	struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
+	s8 val;
+	int ret;
+
+	ret = kstrtos8(buf, 0, &val);
+	if (ret < 0)
+		return ret;
+	if (val > DEF_PRIORITY)
+		return -EINVAL;
+
+	memcg->global_shrink_priority = val;
+
+	return nbytes;
+}
+
+static s64 mem_global_shrink_priority_read(struct cgroup_subsys_state *css,
+					struct cftype *cft)
+{
+	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
+
+	return memcg->global_shrink_priority;
+}
+
 static struct cftype mem_cgroup_legacy_files[] = {
 	{
 		.name = "usage_in_bytes",
@@ -4774,6 +4800,11 @@ static struct cftype mem_cgroup_legacy_files[] = {
 		.write = mem_cgroup_reset,
 		.read_u64 = mem_cgroup_read_u64,
 	},
+	{
+		.name = "global_shrink_priority",
+		.write = mem_global_shrink_priority_write,
+		.read_s64 = mem_global_shrink_priority_read,
+	},
 	{ },	/* terminate */
 };
 
@@ -4996,6 +5027,7 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
 
 	memcg->high = PAGE_COUNTER_MAX;
 	memcg->soft_limit = PAGE_COUNTER_MAX;
+	memcg->global_shrink_priority = DEF_PRIORITY;
 	if (parent) {
 		memcg->swappiness = mem_cgroup_swappiness(parent);
 		memcg->oom_kill_disable = parent->oom_kill_disable;
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 74e8edc..5e11d45 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2637,17 +2637,33 @@ static inline bool should_continue_reclaim(struct pglist_data *pgdat,
 	return inactive_lru_pages > pages_for_compaction;
 }
 
+static bool get_is_global_shrink(struct scan_control *sc)
+{
+	if (!sc->target_mem_cgroup ||
+		mem_cgroup_is_root(sc->target_mem_cgroup))
+		return true;
+
+	return false;
+}
+
 static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
 {
 	struct mem_cgroup *target_memcg = sc->target_mem_cgroup;
 	struct mem_cgroup *memcg;
+	bool is_global_shrink = get_is_global_shrink(sc);
 
 	memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
 	do {
-		struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
+		struct lruvec *lruvec;
 		unsigned long reclaimed;
 		unsigned long scanned;
 
+		if (is_global_shrink &&
+			memcg->global_shrink_priority < sc->priority)
+			continue;
+
+		lruvec = mem_cgroup_lruvec(memcg, pgdat);
+
 		switch (mem_cgroup_protected(target_memcg, memcg)) {
 		case MEMCG_PROT_MIN:
 			/*
@@ -2682,11 +2698,21 @@ static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
 		reclaimed = sc->nr_reclaimed;
 		scanned = sc->nr_scanned;
 
+		if (is_global_shrink &&
+			memcg->global_shrink_priority != DEF_PRIORITY)
+			sc->priority += DEF_PRIORITY
+					- memcg->global_shrink_priority;
+
 		shrink_lruvec(lruvec, sc);
 
 		shrink_slab(sc->gfp_mask, pgdat->node_id, memcg,
 			    sc->priority);
 
+		if (is_global_shrink &&
+			memcg->global_shrink_priority != DEF_PRIORITY)
+			sc->priority -= DEF_PRIORITY
+					- memcg->global_shrink_priority;
+
 		/* Record the group's reclaim efficiency */
 		vmpressure(sc->gfp_mask, memcg, false,
 			   sc->nr_scanned - scanned,
@@ -3395,11 +3421,18 @@ static void age_active_anon(struct pglist_data *pgdat,
 
 	memcg = mem_cgroup_iter(NULL, NULL, NULL);
 	do {
+		if (memcg->global_shrink_priority < sc->priority)
+			continue;
+
 		lruvec = mem_cgroup_lruvec(memcg, pgdat);
+		/*
+		 * Not set sc->priority according even if this is
+		 * a global shrink because nr_to_scan is set to
+		 * SWAP_CLUSTER_MAX and there is not other part use it.
+		 */
 		shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
 				   sc, LRU_ACTIVE_ANON);
-		memcg = mem_cgroup_iter(NULL, memcg, NULL);
-	} while (memcg);
+	} while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)));
 }
 
 static bool pgdat_watermark_boosted(pg_data_t *pgdat, int classzone_idx)
-- 
2.7.4

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[Yafang Shao]

On Wed, Dec 18, 2019 at 5:44 PM Hui Zhu <teawaterz@linux.alibaba.com> wrote:
>
> Currently, memcg has some config to limit memory usage and config
> the shrink behavior.
> In the memory-constrained environment, put different priority tasks
> into different cgroups with different memory limits to protect the
> performance of the high priority tasks.  Because the global memory
> shrink will affect the performance of all tasks.  The memory limit
> cgroup can make shrink happen inside the cgroup.  Then it can decrease
> the memory shrink of the high priority task to protect its performance.
>
> But the memory footprint of the task is not static.  It will change as
> the working pressure changes.  And the version changes will affect it too.
> Then set the appropriate memory limit to decrease the global memory shrink
> is a difficult job and lead to wasted memory or performance loss sometimes.
>
> This commit adds global shrink priority to memcg to try to handle this
> problem.
> The default global shrink priority of each cgroup is DEF_PRIORITY.
> Its behavior in global shrink is not changed.
> And when global shrink priority of a cgroup is smaller than DEF_PRIORITY,
> its memory will be shrink when memcg->global_shrink_priority greater than
> or equal to sc->priority.
>

Just a kind reminder that sc->priority is really propotional, rather
than priority.
The relcaimer scans (total_size >> priority) pages at once.
If the relcaimer can't relaim enough memory, it will decrease
sc->priority and scan MEMCGs again until the sc->pirority drops to 0.
(sc->priority is really a misleading wording. )
So comparing the memcg priority with  sc->priority may cause unexpected issues.

> The following is an example to use global shrink priority in a VM that
> has 2 CPUs, 1G memory and 4G swap:
>  # These are test shells that call usemem that get from
>  # https://git.kernel.org/pub/scm/linux/kernel/git/wfg/vm-scalability.git
> cat 1.sh
> sleep 9999
>  # -s 3600: Sleep 3600 seconds after test complete then usemem will
>  # not release the memory at once.
>  # -Z:  read memory again after access the memory.
>  # The first time access memory need shrink memory to allocate page.
>  # Then the access speed of high priority will not increase a lot.
>  # The read again speed of high priority will increase.
>  # $((850 * 1024 * 1024 + 8)): Different sizes are used to distinguish
>  # the results of the two tests.
> usemem -s 3600 -Z -a -n 1 $((850 * 1024 * 1024 + 8))
> cat 2.sh
> sleep 9999
> usemem -s 3600 -Z -a -n 1 $((850 * 1024 * 1024))
>
>  # Setup swap
> swapon /swapfile
>  # Setup 2 cgroups
> mkdir /sys/fs/cgroup/memory/t1/
> mkdir /sys/fs/cgroup/memory/t2/
>
>  # Run tests with same global shrink priority
> cat /sys/fs/cgroup/memory/t1/memory.global_shrink_priority
> 12
> cat /sys/fs/cgroup/memory/t2/memory.global_shrink_priority
> 12
> echo $$ > /sys/fs/cgroup/memory/t1/cgroup.procs
> sh 1.sh &
> echo $$ > /sys/fs/cgroup/memory/t2/cgroup.procs
> sh 2.sh &
> echo $$ > /sys/fs/cgroup/memory/cgroup.procs
> killall sleep
>  # This the test results
> 1002700800 bytes / 2360359 usecs = 414852 KB/s
> 1002700809 bytes / 2676181 usecs = 365894 KB/s
> read again 891289600 bytes / 13515142 usecs = 64401 KB/s
> read again 891289608 bytes / 13252268 usecs = 65679 KB/s
> killall usemem
>
>  # Run tests with 12 and 8
> cat /sys/fs/cgroup/memory/t1/memory.global_shrink_priority
> 12
> echo 8 > /sys/fs/cgroup/memory/t2/memory.global_shrink_priority
> echo $$ > /sys/fs/cgroup/memory/t1/cgroup.procs
> sh 1.sh &
> echo $$ > /sys/fs/cgroup/memory/t2/cgroup.procs
> sh 2.sh &
> echo $$ > /sys/fs/cgroup/memory/cgroup.procs
> killall sleep
>  # This the test results
> 1002700800 bytes / 1809056 usecs = 541276 KB/s
> 1002700809 bytes / 2184337 usecs = 448282 KB/s
> read again 891289600 bytes / 6666224 usecs = 130568 KB/s
> read again 891289608 bytes / 9171440 usecs = 94903 KB/s
> killall usemem
>
>  # This is the test results of 12 and 6
> 1002700800 bytes / 1827914 usecs = 535692 KB/s
> 1002700809 bytes / 2135124 usecs = 458615 KB/s
> read again 891289600 bytes / 1498419 usecs = 580878 KB/s
> read again 891289608 bytes / 7328362 usecs = 118771 KB/s
>
> Signed-off-by: Hui Zhu <teawaterz@linux.alibaba.com>
> ---
>  include/linux/memcontrol.h |  2 ++
>  mm/memcontrol.c            | 32 ++++++++++++++++++++++++++++++++
>  mm/vmscan.c                | 39 ++++++++++++++++++++++++++++++++++++---
>  3 files changed, 70 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index a7a0a1a5..8ad2437 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -244,6 +244,8 @@ struct mem_cgroup {
>         /* OOM-Killer disable */
>         int             oom_kill_disable;
>
> +       s8 global_shrink_priority;
> +
>         /* memory.events and memory.events.local */
>         struct cgroup_file events_file;
>         struct cgroup_file events_local_file;
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c5b5f74..39fdc84 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4646,6 +4646,32 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
>         return ret;
>  }
>
> +static ssize_t mem_global_shrink_priority_write(struct kernfs_open_file *of,
> +                               char *buf, size_t nbytes, loff_t off)
> +{
> +       struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
> +       s8 val;
> +       int ret;
> +
> +       ret = kstrtos8(buf, 0, &val);
> +       if (ret < 0)
> +               return ret;
> +       if (val > DEF_PRIORITY)
> +               return -EINVAL;
> +
> +       memcg->global_shrink_priority = val;
> +
> +       return nbytes;
> +}
> +
> +static s64 mem_global_shrink_priority_read(struct cgroup_subsys_state *css,
> +                                       struct cftype *cft)
> +{
> +       struct mem_cgroup *memcg = mem_cgroup_from_css(css);
> +
> +       return memcg->global_shrink_priority;
> +}
> +
>  static struct cftype mem_cgroup_legacy_files[] = {
>         {
>                 .name = "usage_in_bytes",
> @@ -4774,6 +4800,11 @@ static struct cftype mem_cgroup_legacy_files[] = {
>                 .write = mem_cgroup_reset,
>                 .read_u64 = mem_cgroup_read_u64,
>         },
> +       {
> +               .name = "global_shrink_priority",
> +               .write = mem_global_shrink_priority_write,
> +               .read_s64 = mem_global_shrink_priority_read,
> +       },
>         { },    /* terminate */
>  };
>
> @@ -4996,6 +5027,7 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
>
>         memcg->high = PAGE_COUNTER_MAX;
>         memcg->soft_limit = PAGE_COUNTER_MAX;
> +       memcg->global_shrink_priority = DEF_PRIORITY;
>         if (parent) {
>                 memcg->swappiness = mem_cgroup_swappiness(parent);
>                 memcg->oom_kill_disable = parent->oom_kill_disable;
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 74e8edc..5e11d45 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2637,17 +2637,33 @@ static inline bool should_continue_reclaim(struct pglist_data *pgdat,
>         return inactive_lru_pages > pages_for_compaction;
>  }
>
> +static bool get_is_global_shrink(struct scan_control *sc)
> +{
> +       if (!sc->target_mem_cgroup ||
> +               mem_cgroup_is_root(sc->target_mem_cgroup))
> +               return true;
> +
> +       return false;
> +}
> +
>  static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
>  {
>         struct mem_cgroup *target_memcg = sc->target_mem_cgroup;
>         struct mem_cgroup *memcg;
> +       bool is_global_shrink = get_is_global_shrink(sc);
>
>         memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
>         do {
> -               struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
> +               struct lruvec *lruvec;
>                 unsigned long reclaimed;
>                 unsigned long scanned;
>
> +               if (is_global_shrink &&
> +                       memcg->global_shrink_priority < sc->priority)
> +                       continue;
> +
> +               lruvec = mem_cgroup_lruvec(memcg, pgdat);
> +
>                 switch (mem_cgroup_protected(target_memcg, memcg)) {
>                 case MEMCG_PROT_MIN:
>                         /*
> @@ -2682,11 +2698,21 @@ static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
>                 reclaimed = sc->nr_reclaimed;
>                 scanned = sc->nr_scanned;
>
> +               if (is_global_shrink &&
> +                       memcg->global_shrink_priority != DEF_PRIORITY)
> +                       sc->priority += DEF_PRIORITY
> +                                       - memcg->global_shrink_priority;
> +

For example.
In this case this memcg can't do full scan.
This behavior is similar with a hard protect(memroy.min), which may
cause unexpected OOM under memory pressure.

Pls. correct me if I misunderstand you.

>                 shrink_lruvec(lruvec, sc);
>
>                 shrink_slab(sc->gfp_mask, pgdat->node_id, memcg,
>                             sc->priority);
>
> +               if (is_global_shrink &&
> +                       memcg->global_shrink_priority != DEF_PRIORITY)
> +                       sc->priority -= DEF_PRIORITY
> +                                       - memcg->global_shrink_priority;
> +
>                 /* Record the group's reclaim efficiency */
>                 vmpressure(sc->gfp_mask, memcg, false,
>                            sc->nr_scanned - scanned,
> @@ -3395,11 +3421,18 @@ static void age_active_anon(struct pglist_data *pgdat,
>
>         memcg = mem_cgroup_iter(NULL, NULL, NULL);
>         do {
> +               if (memcg->global_shrink_priority < sc->priority)
> +                       continue;
> +
>                 lruvec = mem_cgroup_lruvec(memcg, pgdat);
> +               /*
> +                * Not set sc->priority according even if this is
> +                * a global shrink because nr_to_scan is set to
> +                * SWAP_CLUSTER_MAX and there is not other part use it.
> +                */
>                 shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
>                                    sc, LRU_ACTIVE_ANON);
> -               memcg = mem_cgroup_iter(NULL, memcg, NULL);
> -       } while (memcg);
> +       } while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)));
>  }
>
>  static bool pgdat_watermark_boosted(pg_data_t *pgdat, int classzone_idx)
> --
> 2.7.4
>
>

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[Chris Down]

Hi Hui,

In general cgroup v1 is in maintenance mode -- that is, excepting specific 
bugfixes, we don't plan to add new features.

Hui Zhu writes:
>Currently, memcg has some config to limit memory usage and config
>the shrink behavior.
>In the memory-constrained environment, put different priority tasks
>into different cgroups with different memory limits to protect the
>performance of the high priority tasks.  Because the global memory
>shrink will affect the performance of all tasks.  The memory limit
>cgroup can make shrink happen inside the cgroup.  Then it can decrease
>the memory shrink of the high priority task to protect its performance.
>
>But the memory footprint of the task is not static.  It will change as
>the working pressure changes.  And the version changes will affect it too.
>Then set the appropriate memory limit to decrease the global memory shrink
>is a difficult job and lead to wasted memory or performance loss sometimes.
>
>This commit adds global shrink priority to memcg to try to handle this
>problem.

I have significant concerns with exposing scan priority to userspace. This is 
an incredibly difficult metric for users to reason about since it's a reclaim 
implementation feature and would add to an already confusing and fragmented API 
in v1.

Have you considered using memory protection (memory.low, memory.min) for this 
instead? It sounds like it can achieve the results you want, in that it allows 
you to direct and prioritise reclaim in a way that allows for ballparking (ie. 
it is compatible with applications with variable memory footprints).

Thanks,

Chris

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[teawater]

> 在 2019年12月18日，22:09，Chris Down <chris@chrisdown.name> 写道：
> 
> Hi Hui,
> 
> In general cgroup v1 is in maintenance mode -- that is, excepting specific bugfixes, we don't plan to add new features.

Hi Chris,


Thanks for your review.

I will move to v2.

> 
> Hui Zhu writes:
>> Currently, memcg has some config to limit memory usage and config
>> the shrink behavior.
>> In the memory-constrained environment, put different priority tasks
>> into different cgroups with different memory limits to protect the
>> performance of the high priority tasks.  Because the global memory
>> shrink will affect the performance of all tasks.  The memory limit
>> cgroup can make shrink happen inside the cgroup.  Then it can decrease
>> the memory shrink of the high priority task to protect its performance.
>> 
>> But the memory footprint of the task is not static.  It will change as
>> the working pressure changes.  And the version changes will affect it too.
>> Then set the appropriate memory limit to decrease the global memory shrink
>> is a difficult job and lead to wasted memory or performance loss sometimes.
>> 
>> This commit adds global shrink priority to memcg to try to handle this
>> problem.
> 
> I have significant concerns with exposing scan priority to userspace. This is an incredibly difficult metric for users to reason about since it's a reclaim implementation feature and would add to an already confusing and fragmented API in v1.
> 
> Have you considered using memory protection (memory.low, memory.min) for this instead? It sounds like it can achieve the results you want, in that it allows you to direct and prioritise reclaim in a way that allows for ballparking (ie. it is compatible with applications with variable memory footprints).
> 

Memory.min, low, high can affect the global shrink behavior.  They can help task keep some pages to help protect performance.

But what I want is the low priority tasks (the tasks that performance is not very important) do more shrink first.  And when low priority tasks doesn’t have enough pages to be dropped and system need more free page, shrink the high priority task’s pages.  Because at this time, system’s stable is more important than the performance of priority task.
With memory.min and memory.low, I have no idea to config them to support this.  That is why I add global shrink priority.

Thanks,
Hui

> Thanks,
> 
> Chris

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[teawater]

> 在 2019年12月18日，18:47，Yafang Shao <laoar.shao@gmail.com> 写道：
> 
> On Wed, Dec 18, 2019 at 5:44 PM Hui Zhu <teawaterz@linux.alibaba.com> wrote:
>> 
>> Currently, memcg has some config to limit memory usage and config
>> the shrink behavior.
>> In the memory-constrained environment, put different priority tasks
>> into different cgroups with different memory limits to protect the
>> performance of the high priority tasks.  Because the global memory
>> shrink will affect the performance of all tasks.  The memory limit
>> cgroup can make shrink happen inside the cgroup.  Then it can decrease
>> the memory shrink of the high priority task to protect its performance.
>> 
>> But the memory footprint of the task is not static.  It will change as
>> the working pressure changes.  And the version changes will affect it too.
>> Then set the appropriate memory limit to decrease the global memory shrink
>> is a difficult job and lead to wasted memory or performance loss sometimes.
>> 
>> This commit adds global shrink priority to memcg to try to handle this
>> problem.
>> The default global shrink priority of each cgroup is DEF_PRIORITY.
>> Its behavior in global shrink is not changed.
>> And when global shrink priority of a cgroup is smaller than DEF_PRIORITY,
>> its memory will be shrink when memcg->global_shrink_priority greater than
>> or equal to sc->priority.
>> 
> 
> Just a kind reminder that sc->priority is really propotional, rather
> than priority.
> The relcaimer scans (total_size >> priority) pages at once.
> If the relcaimer can't relaim enough memory, it will decrease
> sc->priority and scan MEMCGs again until the sc->pirority drops to 0.
> (sc->priority is really a misleading wording. )
> So comparing the memcg priority with  sc->priority may cause unexpected issues.
> 
>> The following is an example to use global shrink priority in a VM that
>> has 2 CPUs, 1G memory and 4G swap:
>> # These are test shells that call usemem that get from
>> # https://git.kernel.org/pub/scm/linux/kernel/git/wfg/vm-scalability.git
>> cat 1.sh
>> sleep 9999
>> # -s 3600: Sleep 3600 seconds after test complete then usemem will
>> # not release the memory at once.
>> # -Z:  read memory again after access the memory.
>> # The first time access memory need shrink memory to allocate page.
>> # Then the access speed of high priority will not increase a lot.
>> # The read again speed of high priority will increase.
>> # $((850 * 1024 * 1024 + 8)): Different sizes are used to distinguish
>> # the results of the two tests.
>> usemem -s 3600 -Z -a -n 1 $((850 * 1024 * 1024 + 8))
>> cat 2.sh
>> sleep 9999
>> usemem -s 3600 -Z -a -n 1 $((850 * 1024 * 1024))
>> 
>> # Setup swap
>> swapon /swapfile
>> # Setup 2 cgroups
>> mkdir /sys/fs/cgroup/memory/t1/
>> mkdir /sys/fs/cgroup/memory/t2/
>> 
>> # Run tests with same global shrink priority
>> cat /sys/fs/cgroup/memory/t1/memory.global_shrink_priority
>> 12
>> cat /sys/fs/cgroup/memory/t2/memory.global_shrink_priority
>> 12
>> echo $$ > /sys/fs/cgroup/memory/t1/cgroup.procs
>> sh 1.sh &
>> echo $$ > /sys/fs/cgroup/memory/t2/cgroup.procs
>> sh 2.sh &
>> echo $$ > /sys/fs/cgroup/memory/cgroup.procs
>> killall sleep
>> # This the test results
>> 1002700800 bytes / 2360359 usecs = 414852 KB/s
>> 1002700809 bytes / 2676181 usecs = 365894 KB/s
>> read again 891289600 bytes / 13515142 usecs = 64401 KB/s
>> read again 891289608 bytes / 13252268 usecs = 65679 KB/s
>> killall usemem
>> 
>> # Run tests with 12 and 8
>> cat /sys/fs/cgroup/memory/t1/memory.global_shrink_priority
>> 12
>> echo 8 > /sys/fs/cgroup/memory/t2/memory.global_shrink_priority
>> echo $$ > /sys/fs/cgroup/memory/t1/cgroup.procs
>> sh 1.sh &
>> echo $$ > /sys/fs/cgroup/memory/t2/cgroup.procs
>> sh 2.sh &
>> echo $$ > /sys/fs/cgroup/memory/cgroup.procs
>> killall sleep
>> # This the test results
>> 1002700800 bytes / 1809056 usecs = 541276 KB/s
>> 1002700809 bytes / 2184337 usecs = 448282 KB/s
>> read again 891289600 bytes / 6666224 usecs = 130568 KB/s
>> read again 891289608 bytes / 9171440 usecs = 94903 KB/s
>> killall usemem
>> 
>> # This is the test results of 12 and 6
>> 1002700800 bytes / 1827914 usecs = 535692 KB/s
>> 1002700809 bytes / 2135124 usecs = 458615 KB/s
>> read again 891289600 bytes / 1498419 usecs = 580878 KB/s
>> read again 891289608 bytes / 7328362 usecs = 118771 KB/s
>> 
>> Signed-off-by: Hui Zhu <teawaterz@linux.alibaba.com>
>> ---
>> include/linux/memcontrol.h |  2 ++
>> mm/memcontrol.c            | 32 ++++++++++++++++++++++++++++++++
>> mm/vmscan.c                | 39 ++++++++++++++++++++++++++++++++++++---
>> 3 files changed, 70 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index a7a0a1a5..8ad2437 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -244,6 +244,8 @@ struct mem_cgroup {
>>        /* OOM-Killer disable */
>>        int             oom_kill_disable;
>> 
>> +       s8 global_shrink_priority;
>> +
>>        /* memory.events and memory.events.local */
>>        struct cgroup_file events_file;
>>        struct cgroup_file events_local_file;
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index c5b5f74..39fdc84 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -4646,6 +4646,32 @@ static ssize_t memcg_write_event_control(struct kernfs_open_file *of,
>>        return ret;
>> }
>> 
>> +static ssize_t mem_global_shrink_priority_write(struct kernfs_open_file *of,
>> +                               char *buf, size_t nbytes, loff_t off)
>> +{
>> +       struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
>> +       s8 val;
>> +       int ret;
>> +
>> +       ret = kstrtos8(buf, 0, &val);
>> +       if (ret < 0)
>> +               return ret;
>> +       if (val > DEF_PRIORITY)
>> +               return -EINVAL;
>> +
>> +       memcg->global_shrink_priority = val;
>> +
>> +       return nbytes;
>> +}
>> +
>> +static s64 mem_global_shrink_priority_read(struct cgroup_subsys_state *css,
>> +                                       struct cftype *cft)
>> +{
>> +       struct mem_cgroup *memcg = mem_cgroup_from_css(css);
>> +
>> +       return memcg->global_shrink_priority;
>> +}
>> +
>> static struct cftype mem_cgroup_legacy_files[] = {
>>        {
>>                .name = "usage_in_bytes",
>> @@ -4774,6 +4800,11 @@ static struct cftype mem_cgroup_legacy_files[] = {
>>                .write = mem_cgroup_reset,
>>                .read_u64 = mem_cgroup_read_u64,
>>        },
>> +       {
>> +               .name = "global_shrink_priority",
>> +               .write = mem_global_shrink_priority_write,
>> +               .read_s64 = mem_global_shrink_priority_read,
>> +       },
>>        { },    /* terminate */
>> };
>> 
>> @@ -4996,6 +5027,7 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
>> 
>>        memcg->high = PAGE_COUNTER_MAX;
>>        memcg->soft_limit = PAGE_COUNTER_MAX;
>> +       memcg->global_shrink_priority = DEF_PRIORITY;
>>        if (parent) {
>>                memcg->swappiness = mem_cgroup_swappiness(parent);
>>                memcg->oom_kill_disable = parent->oom_kill_disable;
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 74e8edc..5e11d45 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -2637,17 +2637,33 @@ static inline bool should_continue_reclaim(struct pglist_data *pgdat,
>>        return inactive_lru_pages > pages_for_compaction;
>> }
>> 
>> +static bool get_is_global_shrink(struct scan_control *sc)
>> +{
>> +       if (!sc->target_mem_cgroup ||
>> +               mem_cgroup_is_root(sc->target_mem_cgroup))
>> +               return true;
>> +
>> +       return false;
>> +}
>> +
>> static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
>> {
>>        struct mem_cgroup *target_memcg = sc->target_mem_cgroup;
>>        struct mem_cgroup *memcg;
>> +       bool is_global_shrink = get_is_global_shrink(sc);
>> 
>>        memcg = mem_cgroup_iter(target_memcg, NULL, NULL);
>>        do {
>> -               struct lruvec *lruvec = mem_cgroup_lruvec(memcg, pgdat);
>> +               struct lruvec *lruvec;
>>                unsigned long reclaimed;
>>                unsigned long scanned;
>> 
>> +               if (is_global_shrink &&
>> +                       memcg->global_shrink_priority < sc->priority)
>> +                       continue;
>> +
>> +               lruvec = mem_cgroup_lruvec(memcg, pgdat);
>> +
>>                switch (mem_cgroup_protected(target_memcg, memcg)) {
>>                case MEMCG_PROT_MIN:
>>                        /*
>> @@ -2682,11 +2698,21 @@ static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
>>                reclaimed = sc->nr_reclaimed;
>>                scanned = sc->nr_scanned;
>> 
>> +               if (is_global_shrink &&
>> +                       memcg->global_shrink_priority != DEF_PRIORITY)
>> +                       sc->priority += DEF_PRIORITY
>> +                                       - memcg->global_shrink_priority;
>> +
> 
> For example.
> In this case this memcg can't do full scan.
> This behavior is similar with a hard protect(memroy.min), which may
> cause unexpected OOM under memory pressure.
> 
> Pls. correct me if I misunderstand you.

Thanks and agree with you.
Low priority task should do more shrink if the high priority task is ignored.

Best,
Hui

> 
>>                shrink_lruvec(lruvec, sc);
>> 
>>                shrink_slab(sc->gfp_mask, pgdat->node_id, memcg,
>>                            sc->priority);
>> 
>> +               if (is_global_shrink &&
>> +                       memcg->global_shrink_priority != DEF_PRIORITY)
>> +                       sc->priority -= DEF_PRIORITY
>> +                                       - memcg->global_shrink_priority;
>> +
>>                /* Record the group's reclaim efficiency */
>>                vmpressure(sc->gfp_mask, memcg, false,
>>                           sc->nr_scanned - scanned,
>> @@ -3395,11 +3421,18 @@ static void age_active_anon(struct pglist_data *pgdat,
>> 
>>        memcg = mem_cgroup_iter(NULL, NULL, NULL);
>>        do {
>> +               if (memcg->global_shrink_priority < sc->priority)
>> +                       continue;
>> +
>>                lruvec = mem_cgroup_lruvec(memcg, pgdat);
>> +               /*
>> +                * Not set sc->priority according even if this is
>> +                * a global shrink because nr_to_scan is set to
>> +                * SWAP_CLUSTER_MAX and there is not other part use it.
>> +                */
>>                shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
>>                                   sc, LRU_ACTIVE_ANON);
>> -               memcg = mem_cgroup_iter(NULL, memcg, NULL);
>> -       } while (memcg);
>> +       } while ((memcg = mem_cgroup_iter(NULL, memcg, NULL)));
>> }
>> 
>> static bool pgdat_watermark_boosted(pg_data_t *pgdat, int classzone_idx)
>> --
>> 2.7.4

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[Chris Down]

Hi Hui,

teawater writes:
>Memory.min, low, high can affect the global shrink behavior.  They can help 
>task keep some pages to help protect performance.
>
>But what I want is the low priority tasks (the tasks that performance is not 
>very important) do more shrink first.  And when low priority tasks doesn’t 
>have enough pages to be dropped and system need more free page, shrink the 
>high priority task’s pages.  Because at this time, system’s stable is more 
>important than the performance of priority task.
>With memory.min and memory.low, I have no idea to config them to support this.  
>That is why I add global shrink priority.

For sure, that's what I'm suggesting you use memory.{min,low} for -- you define 
some subset of the cgroup hierarchy as "protected", and then you bias reclaim 
away from protected cgroups (and thus *towards* unprotected cgroups) by biasing 
the size of LRU scanning. See my patch that went into 5.4 and the examples in 
the commit message:

     commit 9783aa9917f8ae24759e67bf882f1aba32fe4ea1
     Author: Chris Down <chris@chrisdown.name>
     Date:   Sun Oct 6 17:58:32 2019 -0700

         mm, memcg: proportional memory.{low,min} reclaim

You can see how we're using memory.{low,min} to achieve this in this case 
study[0]. It's not exactly equivalent technically to your solution, but the end 
goals are similar.

Thanks,

Chris

0: https://facebookmicrosites.github.io/cgroup2/docs/overview.html#case-study-the-fbtax2-project

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[teawater]

> 在 2019年12月19日，19:26，Chris Down <chris@chrisdown.name> 写道：
> 
> Hi Hui,
> 
> teawater writes:
>> Memory.min, low, high can affect the global shrink behavior.  They can help task keep some pages to help protect performance.
>> 
>> But what I want is the low priority tasks (the tasks that performance is not very important) do more shrink first.  And when low priority tasks doesn’t have enough pages to be dropped and system need more free page, shrink the high priority task’s pages.  Because at this time, system’s stable is more important than the performance of priority task.
>> With memory.min and memory.low, I have no idea to config them to support this.  That is why I add global shrink priority.
> 
> For sure, that's what I'm suggesting you use memory.{min,low} for -- you define some subset of the cgroup hierarchy as "protected", and then you bias reclaim away from protected cgroups (and thus *towards* unprotected cgroups) by biasing the size of LRU scanning. See my patch that went into 5.4 and the examples in the commit message:
> 
>    commit 9783aa9917f8ae24759e67bf882f1aba32fe4ea1
>    Author: Chris Down <chris@chrisdown.name>
>    Date:   Sun Oct 6 17:58:32 2019 -0700
> 
>        mm, memcg: proportional memory.{low,min} reclaim
> 
> You can see how we're using memory.{low,min} to achieve this in this case study[0]. It's not exactly equivalent technically to your solution, but the end goals are similar.
> 
> Thanks,
> 
> Chris
> 
> 0: https://facebookmicrosites.github.io/cgroup2/docs/overview.html#case-study-the-fbtax2-project

Hi Chris,

Really appreciate for your help.  I will try to use it handle my problem.

Best,
Hui

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[teawater]

> 在 2019年12月19日，19:26，Chris Down <chris@chrisdown.name> 写道：
> 
> Hi Hui,
> 
> teawater writes:
>> Memory.min, low, high can affect the global shrink behavior.  They can help task keep some pages to help protect performance.
>> 
>> But what I want is the low priority tasks (the tasks that performance is not very important) do more shrink first.  And when low priority tasks doesn’t have enough pages to be dropped and system need more free page, shrink the high priority task’s pages.  Because at this time, system’s stable is more important than the performance of priority task.
>> With memory.min and memory.low, I have no idea to config them to support this.  That is why I add global shrink priority.
> 
> For sure, that's what I'm suggesting you use memory.{min,low} for -- you define some subset of the cgroup hierarchy as "protected", and then you bias reclaim away from protected cgroups (and thus *towards* unprotected cgroups) by biasing the size of LRU scanning. See my patch that went into 5.4 and the examples in the commit message:
> 
>    commit 9783aa9917f8ae24759e67bf882f1aba32fe4ea1
>    Author: Chris Down <chris@chrisdown.name>
>    Date:   Sun Oct 6 17:58:32 2019 -0700
> 
>        mm, memcg: proportional memory.{low,min} reclaim
> 
> You can see how we're using memory.{low,min} to achieve this in this case study[0]. It's not exactly equivalent technically to your solution, but the end goals are similar.
> 
> Thanks,
> 
> Chris
> 
> 0: https://facebookmicrosites.github.io/cgroup2/docs/overview.html#case-study-the-fbtax2-project

Hi Chris,

I have another idea about global shrink priority.

In the memory-constrained and complex multitasking environment such as an Android system may require more complex performance priority.
For example, the tasks of app in the font, they need high priority because low priority will affect the user experience at once.
The tasks of app in background should have lower priority than the first one.  And sometime, each app should have different priority.  Because some apps are frequently used.  They should have high priority than other background apps.
The daemons should have lower priority than background apps.  Because most of them will not affect the user experience.

Do you think this environment is suitable for global shrink priority.

Best,
Hui

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[Chris Down]

Hi Hui,

teawater writes:
>In the memory-constrained and complex multitasking environment such as an Android system may require more complex performance priority.
>For example, the tasks of app in the font, they need high priority because low priority will affect the user experience at once.
>The tasks of app in background should have lower priority than the first one.  And sometime, each app should have different priority.  Because some apps are frequently used.  They should have high priority than other background apps.
>The daemons should have lower priority than background apps.  Because most of them will not affect the user experience.

In general I don't think it's meaningful to speculate about whether it would 
help or not without data and evidence gathering. It would really depend on how 
the system is composed overall. Is this a real problem you're seeing, or just 
something hypothetical?

If there is a real case to discuss, we can certainly discuss it. That said, at 
the very least I think the API needs to be easier to reason about rather than 
just exposing mm internals, and there needs to be a demonstration that it 
solves a real problem and existing controls are insufficient :-)

Thanks,

Chris

Re: [PATCH] mm: vmscan: memcg: Add global shrink priority

[teawater]

> 在 2019年12月29日，22:02，Chris Down <chris@chrisdown.name> 写道：
> 
> Hi Hui,
> 
> teawater writes:
>> In the memory-constrained and complex multitasking environment such as an Android system may require more complex performance priority.
>> For example, the tasks of app in the font, they need high priority because low priority will affect the user experience at once.
>> The tasks of app in background should have lower priority than the first one.  And sometime, each app should have different priority.  Because some apps are frequently used.  They should have high priority than other background apps.
>> The daemons should have lower priority than background apps.  Because most of them will not affect the user experience.
> 
> In general I don't think it's meaningful to speculate about whether it would help or not without data and evidence gathering. It would really depend on how the system is composed overall. Is this a real problem you're seeing, or just something hypothetical?
> 
> If there is a real case to discuss, we can certainly discuss it. That said, at the very least I think the API needs to be easier to reason about rather than just exposing mm internals, and there needs to be a demonstration that it solves a real problem and existing controls are insufficient :-)
> 
> Thanks,
> 
> Chris

Thanks!  Agree with you.

Best,
Hui