Re: [PATCH] mm/memcg: clear folio memcg after changing per memcg stats

[Bingfang Guo]

Hi, Andrew.

> On Sep 1, 2026, at 10:11, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> On Mon, 31 Aug 2026 11:32:43 +0800 Bingfang Guo via B4 Relay <devnull+bingfangguo.tencent.com@xxxxxxxxxx> wrote:
> 
>> From: Bingfang Guo <bingfangguo@xxxxxxxxxxx>
>> 
>> __memcg1_swapout() transfers the memsw charge of a folio to its swap
>> entry and clears folio->memcg_data as part of that.  In the vmscan
>> swapout path it runs before __swap_cache_del_folio(), which then
>> decrements the swapcache stats through lruvec_stat_mod_folio().  Since
>> folio->memcg_data has already been cleared, folio_memcg() returns NULL
>> and the NR_SWAPCACHE decrement only updates the node-level counter
>> instead of the memcg's lruvec, leaking the per-memcg swapcache count.
>> 
>> Move the __memcg1_swapout() call into __swap_cache_del_folio(), after
>> the NR_FILE_PAGES and NR_SWAPCACHE updates but before
>> __swap_cache_do_del_folio() removes the folio from the swap cache.  This
>> keeps the stats attributed to the folio's memcg while still recording
>> the swap cgroup with a valid folio->swap.  Add a swapout parameter so
>> the plain swap_cache_del_folio() path is left unchanged.
> 
> As always, the most important thing to tell us when fixing a bug is
> "what are the userspace-visible runtime effects of this bug".  Please
> update your prompts to always tell us this, in the first paragraph.

Yes, of course. I will note that in the future.

> Clearly the immediate effect is messed up is messed up stats
> (memory.stat?).  But what are the subsequent runtime effects of this?

Although I don’t really know any user space program relying on this
value, I noticed this when testing another patch set by swapping pages
in and out and saw extremely high swapcached count in the per memcg
level memory.stat. It seems that the counter never gets decreased so the
value is rather useless and confusing to users trying to analyze problems
and read this. So I think fixing this can be helpful.

I will include this message in the patch later to make it clear.

> 
>> Fixes: b197d41462c20 ("mm/memcg, swap: store cgroup id in cluster table directly")
> 
> Quite recent.
> 
>> The problem is reproducible using the following script and program:
> 
> OK, thanks.  I agree with putting this info below the "---".  It's
> probably too detailed for the changelog - curious people can find it by
> following the Link:
> 
> Anyway.  Sashiko points out that a CONFIG_SWAP=n stub wasn't updated
> (this happens often):
> 
> https://sashiko.dev/#/patchset/20260831-memcg-swapcache-stats-fix-v1-1-1c0819ebdb86@xxxxxxxxxxx

Sure. I will fix this in the next version.

> 
> so I'll take no action at this time.  Please do update the changelog to
> include the runtime impact and I'd like reviewers to suggest whether we
> should backport.

Thanks for your reviewing and suggestions!


Regards,
Bingfang