[Devel] [PATCH vz9] mm/memcontrol: fix cache counter imbalance in mem_cgroup_replace_folio()

[Pavel Tikhomirov]

On 7/24/25 00:31, Konstantin Khorenko wrote:
> The function was checking cache flag on the new folio instead of the old one,
> causing cache counter imbalance when replacing cache pages.
> The new folio is freshly allocated and never has the cache flag set, so
> we need to copy the flag from the old folio and charge the cache counter
> accordingly.
> 
> This fixes potential cache accounting issues during page cache replacement
> operations like page migration between NUMA nodes.
> 
> Fixes: 763a087e29d11 ("mm: Memory cgroup page cache limit")
> https://virtuozzo.atlassian.net/browse/VSTOR-111756
> 
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> 
> Feature: mm: Memory cgroup page cache limit
> ---
>   mm/memcontrol.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index cefb72234fda5..af70959f00c8e 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -8292,6 +8292,12 @@ void mem_cgroup_migrate(struct folio *old, struct folio *new)
>   			page_counter_charge(&memcg->memsw, nr_pages);
>   	}
>   
> +	if (folio_memcg_cache(old)) {
> +		WRITE_ONCE(folio->memcg_data,

We don't seem to have "folio" variable here on vz9 (at least I checked 
in rh9-5.14.0-427.77.1.vz9.86.2), probably meant to be "new".

> +			   READ_ONCE(folio->memcg_data) | MEMCG_DATA_PGCACHE);
> +		page_counter_charge(&memcg->cache, nr_pages);
> +	}
> +
>   	/*
>   	 * finist explained the idea behind adding a WARN_ON() here:
>   	 * - we do not want to check flags correctness on each flag change
> @@ -8312,9 +8318,6 @@ void mem_cgroup_migrate(struct folio *old, struct folio *new)
>   		 folio_test_swapbacked(new)   || folio_test_swapcache(new) ||
>   		 folio_test_mappedtodisk(new) || folio_test_ksm(new)));
>   
> -	if (folio_memcg_cache(new))
> -		page_counter_charge(&memcg->cache, nr_pages);
> -
>   	css_get(&memcg->css);
>   	commit_charge(new, memcg);

This commit_charge is probably making all our efforts vain, as it resets 
flags in new->memcg_data by putting memory cgroup pointer there. E.g. in 
charge_memcg we first do commit_charge and only then add our flag to it.

I believe we must put WRITE_ONCE after commit_charge, same for our 
"WARN_ON(folio_memcg_cache(new)...".

>   

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.