Re: [PATCH] mm: memcg: use ratelimited stats flush in obj_cgroup_may_zswap()

[Song Hu]

在 2026/8/18 00:04, Shakeel Butt 写道:
> On Mon, Aug 17, 2026 at 09:18:43PM +0800, Song Hu wrote:
>> obj_cgroup_may_zswap() runs on every folio swapped out through
>> zswap.  For each ancestor with a non-max zswap.max, it flushes the
>> cgroup rstat hierarchy synchronously with force=true, which skips
>> the ratelimit inside __mem_cgroup_flush_stats().  In a swap storm
>> with zswap.max configured, a container takes the global rstat lock
>> on every swapped-out folio.
> 
> Any reason you are limiting zswap through zswap.max?
> 

Mostly fairness on a shared pool: zswap.max_pool_percent is global
only, so on a multi-tenant host one cgroup's cold anonymous memory
can soak the pool and crowd out the others.  zswap.max is the only
per-cgroup control over that share; memory.max bounds the total
footprint, not the share of the pool.

>>
>> zswap_shrinker_count() had the same pattern and switched to
>> mem_cgroup_flush_stats_ratelimited() in commit ea80da363a1f
>> ("mm/zswap: use ratelimited stats flush in zswap_shrinker_count()"),
>> where the same flush on the shrinker side showed up at 2.88% of
>> kernel cycles under osq_lock on a 96-core machine.
>>
>> Measured on a KVM guest with a swap storm under a cgroup with
>> zswap.max set: obj_cgroup_may_zswap() was entered 198,977 times
>> before the patch and 198,968 times after, while
>> __mem_cgroup_flush_stats() was entered 281,017 times before and
>> 80,445 times after.  The removed 200,572 flushes match the store
>> attempt count almost exactly; the remainder comes from other stats
>> readers in the swap path.
> 
> This is a known issue. Using ratelimited interface also comes with a drawback
> that the kernel may react on stale information and the consequences might be
> unneeded oom-kills.
> 
> There was orthogonal discussion on moving zswap limit enforcement away from
> rstat. Yosry, any updates on that?
> 

Fair point.  The direction I had considered is the benign one -
stale low after a burst of stores overshoots admission for a cycle.
But after a burst of writebacks the folded value can be stale high
too, and premature rejection with zswap.writeback off or swap.max
tight ends in a memcg OOM kill that accurate stats would have
avoided.  may_zswap() is admission control rather than a hint, so
the guarantee matters more here than for the shrinker side
of ea80da363a1f.

If moving the limit enforcement off rstat is happening, that
supersedes this patch and I am happy to drop it.  Otherwise it can
serve as a stopgap for the per-folio lock cost - your call, and
curious where Yosry's discussion stands.

Thanks,
Song

>>
>> The stats can now be up to one flusher cycle stale, so zswap.max
>> admission can overshoot for one cycle in a storm; the overshoot is
>> corrected as soon as the next flush lands and later stores see it,
>> the same tradeoff the shrinker side made.
>>
>> Fixes: f4840ccfca25 ("zswap: memcg accounting")
>> Signed-off-by: Song Hu <husong@xxxxxxxxxx>
>> ---
>>  mm/memcontrol.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 17da1f43b7d3..7a8f689055c6 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -6000,8 +6000,7 @@ bool obj_cgroup_may_zswap(struct obj_cgroup *objcg)
>>  			break;
>>  		}
>>  
>> -		/* Force flush to get accurate stats for charging */
>> -		__mem_cgroup_flush_stats(memcg, true);
>> +		mem_cgroup_flush_stats_ratelimited(memcg);
>>  		pages = memcg_page_state(memcg, MEMCG_ZSWAP_B) / PAGE_SIZE;
>>  		if (pages < max)
>>  			continue;
>> -- 
>> 2.43.0
>>