Re: [PATCH] memcg: Set memory min, low, high values along with max

[Michal Hocko]

On Fri 31-03-23 03:22:32, Shaun Tancheff wrote:
> From: Shaun Tancheff <shaun@xxxxxxxxxxxx>
> 
> memcg-v1 does not expose memory min, low, and high.
> 
> These values should to be set to reasonable non-zero values
> when max is set.
> 
> This patch sets them to 10%, 20% and 80% respective to max.
> 
> This fixes an issue with memory pressure with file systems
> do an unbounded high rate of I/O hitting oom.

Cgroup v1 has some inherent issues with the dirty data throttling and
that is a well known problem. Especially when those FS allocations are
GFP_NOFS where we cannot throttle direct in the memory reclaim path (see
shrink_folio_list). There are some hacks^Wworkarounds to help out, e.g.
81a70c21d917 ("mm/cgroup/reclaim: fix dirty pages throttling on cgroup
v1") but the issue is inherent to how v1 operates so nothing will work
100%. A more appropriate way forward is to use v2 instead.

But maybe you are hitting a different problem so please tell us more.

> Signed-off-by: Shaun Tancheff <shaun.tancheff@xxxxxxx>

min, low and high are v2 concepts and v1 cannot/won't really use them.
Besides that hard coded policies do not belong to the kernel. So no to
this patch. We definitely want to hear more about the underlying problem
and see what we can do about that.

Nacked-by: Michal Hocko <mhocko@xxxxxxxx>

> ---
>  mm/memcontrol.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 2eee092f8f11..0f5918d9dd2a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3491,6 +3491,15 @@ static int mem_cgroup_resize_max(struct mem_cgroup *memcg,
>  		if (max > counter->max)
>  			enlarge = true;
>  		ret = page_counter_set_max(counter, max);
> +		if (!ret && !memsw) {
> +			unsigned long min = (max / 10) + 1;
> +			unsigned long low = min * 2;
> +			unsigned long high = max - low;
> +
> +			page_counter_set_min(counter, min);
> +			page_counter_set_low(counter, low);
> +			page_counter_set_high(counter, high);
> +		}
>  		mutex_unlock(&memcg_max_mutex);
>  
>  		if (!ret)
> -- 
> 2.34.1

-- 
Michal Hocko
SUSE Labs