[rh7,v4] mm/memcg: fix cache growth above cache.limit_in_bytes

Submitted by Andrey Ryabinin on July 30, 2020, 3:02 p.m.

Details

Message ID 20200730150238.31052-1-aryabinin@virtuozzo.com
State New
Series "mm/memcg: fix cache growth above cache.limit_in_bytes"
Headers show

Commit Message

Andrey Ryabinin July 30, 2020, 3:02 p.m.
Exceeding cache above cache.limit_in_bytes schedules high_work_func()
which tries to reclaim 32 pages. If cache generated fast enough or it allows
cgroup to steadily grow above cache.limit_in_bytes because we don't reclaim
enough. Try to reclaim exceeded amount of cache instead.

https://jira.sw.ru/browse/PSBM-106384
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---

 - Changes since v1: add bug link to changelog
 - Changes since v2: Fix cache_overused check (We should check if it's positive).
    Made this stupid bug during cleanup, patch was tested without bogus cleanup,
    so it shoud work.
 - Chnages since v3: Compilation fixes, properly tested now.

 mm/memcontrol.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3cf200f506c3..16cbd451a588 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3080,12 +3080,16 @@  static void reclaim_high(struct mem_cgroup *memcg,
 {
 
 	do {
+		long cache_overused;
+
 		if (page_counter_read(&memcg->memory) > memcg->high)
 			try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask, 0);
 
-		if (page_counter_read(&memcg->cache) > memcg->cache.limit)
-			try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask,
-						MEM_CGROUP_RECLAIM_NOSWAP);
+		cache_overused = page_counter_read(&memcg->cache) -
+			memcg->cache.limit;
+		if (cache_overused > 0)
+			try_to_free_mem_cgroup_pages(memcg, cache_overused,
+					gfp_mask, MEM_CGROUP_RECLAIM_NOSWAP);
 
 	} while ((memcg = parent_mem_cgroup(memcg)));
 }

Comments

Evgenii Shatokhin July 30, 2020, 3:52 p.m.
Hi,

On 30.07.2020 18:02, Andrey Ryabinin wrote:
> Exceeding cache above cache.limit_in_bytes schedules high_work_func()
> which tries to reclaim 32 pages. If cache generated fast enough or it allows
> cgroup to steadily grow above cache.limit_in_bytes because we don't reclaim
> enough. Try to reclaim exceeded amount of cache instead.
> 
> https://jira.sw.ru/browse/PSBM-106384
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
> 
>   - Changes since v1: add bug link to changelog
>   - Changes since v2: Fix cache_overused check (We should check if it's positive).
>      Made this stupid bug during cleanup, patch was tested without bogus cleanup,
>      so it shoud work.
>   - Chnages since v3: Compilation fixes, properly tested now.
> 
>   mm/memcontrol.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 3cf200f506c3..16cbd451a588 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3080,12 +3080,16 @@ static void reclaim_high(struct mem_cgroup *memcg,
>   {
>   
>   	do {
> +		long cache_overused;
> +
>   		if (page_counter_read(&memcg->memory) > memcg->high)
>   			try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask, 0);
>   
> -		if (page_counter_read(&memcg->cache) > memcg->cache.limit)
> -			try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask,
> -						MEM_CGROUP_RECLAIM_NOSWAP);
> +		cache_overused = page_counter_read(&memcg->cache) -
> +			memcg->cache.limit;

If cache_overused is less than 32 pages, the kernel would try to reclaim 
less than before the patch. It it OK, or should it try to reclaim at 
least 32 pages?

> +		if (cache_overused > 0)
> +			try_to_free_mem_cgroup_pages(memcg, cache_overused,
> +					gfp_mask, MEM_CGROUP_RECLAIM_NOSWAP);
>   
>   	} while ((memcg = parent_mem_cgroup(memcg)));
>   }
>
Andrey Ryabinin July 30, 2020, 4:17 p.m.
On 7/30/20 6:52 PM, Evgenii Shatokhin wrote:
> Hi,
> 
> On 30.07.2020 18:02, Andrey Ryabinin wrote:
>> Exceeding cache above cache.limit_in_bytes schedules high_work_func()
>> which tries to reclaim 32 pages. If cache generated fast enough or it allows
>> cgroup to steadily grow above cache.limit_in_bytes because we don't reclaim
>> enough. Try to reclaim exceeded amount of cache instead.
>>
>> https://jira.sw.ru/browse/PSBM-106384
>> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
>> ---
>>
>>   - Changes since v1: add bug link to changelog
>>   - Changes since v2: Fix cache_overused check (We should check if it's positive).
>>      Made this stupid bug during cleanup, patch was tested without bogus cleanup,
>>      so it shoud work.
>>   - Chnages since v3: Compilation fixes, properly tested now.
>>
>>   mm/memcontrol.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 3cf200f506c3..16cbd451a588 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -3080,12 +3080,16 @@ static void reclaim_high(struct mem_cgroup *memcg,
>>   {
>>         do {
>> +        long cache_overused;
>> +
>>           if (page_counter_read(&memcg->memory) > memcg->high)
>>               try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask, 0);
>>   -        if (page_counter_read(&memcg->cache) > memcg->cache.limit)
>> -            try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask,
>> -                        MEM_CGROUP_RECLAIM_NOSWAP);
>> +        cache_overused = page_counter_read(&memcg->cache) -
>> +            memcg->cache.limit;
> 
> If cache_overused is less than 32 pages, the kernel would try to reclaim less than before the patch. It it OK, or should it try to reclaim at least 32 pages?

It's ok, try_to_free_mem_cgroup_pages will increase it:

unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *memcg,
					   unsigned long nr_pages,
					   gfp_t gfp_mask,
					   int flags)

....
		.nr_to_reclaim = max(nr_pages, SWAP_CLUSTER_MAX),