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

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

[Song Hu]

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.

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.

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@kylinos.cn>
---
 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

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

[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?

> 
> 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?

> 
> 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@kylinos.cn>
> ---
>  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
> 

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@kylinos.cn>
>> ---
>>  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
>>

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

[Yosry Ahmed]

On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@kylinos.cn> wrote:
>
>
>
> 在 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?

I am not actively looking into that, but Joanne was looking into
AFAICT. I will respond to the thread there and CC Song as well.

> >
>
> 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.

Not sure how much progress is being made there, but I would really
like to stop the proliferation of ratelimited flushing, especially in
this case where it can cause OOM kills.

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

[Joanne Koong]

On Tue, Aug 18, 2026 at 11:35 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@kylinos.cn> wrote:
> >
> >
> >
> > 在 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?
>
> I am not actively looking into that, but Joanne was looking into
> AFAICT. I will respond to the thread there and CC Song as well.
>

For this zswap stat, I ran some benchmarks comparing 4 approaches
(switchable behind a runtime knob [1]):
a) rstat + forced flush       (baseline aka what the tree does today)
b) rstat + ratelimited         (Song's proposal)
c) hierarchical per-CPU   (Yosry's idea from [2])
d) page counters              (following what all the other memcg limits do)

For the setup, the benchmark creates a cgroup chain at depth X with
memory.max set to 1G on the leaf and memory.zswap.max set to 512M on
every level, and spins up 20 processes there that each allocate 100
MiB, fault it in, and touch every page four more times. With that 2000
MiB against the 1 GiB memory.max, it triggers reclaim continuously and
makes the swap traffic go through zswap. The machine I ran this on had
80 CPUs.

I also ran it with no memory.zswap.max set (ie no reads triggered,
only update path runs) - as I understand it, this is the configuration
that is more often used in practice.

These are the results I saw:

kernel cpu time (in ns) per zswap store, zswap.max set
                      a)                     b)            c)                d)
depth 1        567,116          35,604     35,841     34,995
depth 2      1,082,007          35,507     37,803     36,209
depth 4      2,153,329          37,591     40,117     39,893
depth 8      4,211,692          34,963     41,070     44,211
depth 32    15,728,220       53,575     94,685     65,946

kernel cpu time (in ns) per zswap store, no zswap.max (update path only):
                        a)                  b)              c)            d)
depth 1         34,787          34,062     34,930     35,630
depth 2         34,913          35,061     36,404     36,360
depth 4         36,422          36,809     36,922     37,483
depth 8         42,177          34,440     36,679     40,793
depth 32        55,309          55,321     57,671     59,190

c) and d) are for the most part pretty comparable to b) without
introducing the staleness problem of b). Between c) and d), I think d)
ends up outperforming c) as the # of cpus and depth gets larger.

I'm seeing that all the other memory limits (eg memory.swap.max,
memory.max, etc) are already using page counters. Is there a reason
the zswap stat can't? If not, does it make sense for the zswap stat to
switch over to using page counters?

Thanks,
Joanne

[1] https://github.com/joannekoong/linux/commit/9092e0057f038ffca43ae3f35faf58c33b1d7c9b
[2] https://lore.kernel.org/linux-fsdevel/anpsLy_pCanocgS1@google.com/

> > >
> >
> > 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.
>
> Not sure how much progress is being made there, but I would really
> like to stop the proliferation of ratelimited flushing, especially in
> this case where it can cause OOM kills.

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

[Johannes Weiner]

On Thu, Aug 20, 2026 at 03:18:39PM -0700, Joanne Koong wrote:
> On Tue, Aug 18, 2026 at 11:35 AM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@kylinos.cn> wrote:
> > >
> > >
> > >
> > > 在 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?
> >
> > I am not actively looking into that, but Joanne was looking into
> > AFAICT. I will respond to the thread there and CC Song as well.
> >
> 
> For this zswap stat, I ran some benchmarks comparing 4 approaches
> (switchable behind a runtime knob [1]):
> a) rstat + forced flush       (baseline aka what the tree does today)
> b) rstat + ratelimited         (Song's proposal)
> c) hierarchical per-CPU   (Yosry's idea from [2])
> d) page counters              (following what all the other memcg limits do)
> 
> For the setup, the benchmark creates a cgroup chain at depth X with
> memory.max set to 1G on the leaf and memory.zswap.max set to 512M on
> every level, and spins up 20 processes there that each allocate 100
> MiB, fault it in, and touch every page four more times. With that 2000
> MiB against the 1 GiB memory.max, it triggers reclaim continuously and
> makes the swap traffic go through zswap. The machine I ran this on had
> 80 CPUs.
> 
> I also ran it with no memory.zswap.max set (ie no reads triggered,
> only update path runs) - as I understand it, this is the configuration
> that is more often used in practice.
> 
> These are the results I saw:
> 
> kernel cpu time (in ns) per zswap store, zswap.max set
>                       a)                     b)            c)                d)
> depth 1        567,116          35,604     35,841     34,995
> depth 2      1,082,007          35,507     37,803     36,209
> depth 4      2,153,329          37,591     40,117     39,893
> depth 8      4,211,692          34,963     41,070     44,211
> depth 32    15,728,220       53,575     94,685     65,946
> 
> kernel cpu time (in ns) per zswap store, no zswap.max (update path only):
>                         a)                  b)              c)            d)
> depth 1         34,787          34,062     34,930     35,630
> depth 2         34,913          35,061     36,404     36,360
> depth 4         36,422          36,809     36,922     37,483
> depth 8         42,177          34,440     36,679     40,793
> depth 32        55,309          55,321     57,671     59,190
> 
> c) and d) are for the most part pretty comparable to b) without
> introducing the staleness problem of b). Between c) and d), I think d)
> ends up outperforming c) as the # of cpus and depth gets larger.
> 
> I'm seeing that all the other memory limits (eg memory.swap.max,
> memory.max, etc) are already using page counters. Is there a reason
> the zswap stat can't? If not, does it make sense for the zswap stat to
> switch over to using page counters?

Thanks for running these.

I used the vmstat counter on the assumption that setting zswap.max is
rare and the counter is maintained anyway for memory.stat.

But I never actually tested it. The assumption was that surely walking
ancestors on a quick if (max == PAGE_COUNTER_MAX) continue would be
much cheaper than page counter atomics at every level. And so I'm
surprised by your results.

But that was the sole reason. If it doesn't stand up to benchmarking,
no objection to streamlining the control to a standard implementation.

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

[Yosry Ahmed]

On Fri, Aug 21, 2026 at 10:52 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
>
> On Thu, Aug 20, 2026 at 03:18:39PM -0700, Joanne Koong wrote:
> > On Tue, Aug 18, 2026 at 11:35 AM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
> > > On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@kylinos.cn> wrote:
> > > >
> > > >
> > > >
> > > > 在 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?
> > >
> > > I am not actively looking into that, but Joanne was looking into
> > > AFAICT. I will respond to the thread there and CC Song as well.
> > >
> >
> > For this zswap stat, I ran some benchmarks comparing 4 approaches
> > (switchable behind a runtime knob [1]):
> > a) rstat + forced flush       (baseline aka what the tree does today)
> > b) rstat + ratelimited         (Song's proposal)
> > c) hierarchical per-CPU   (Yosry's idea from [2])
> > d) page counters              (following what all the other memcg limits do)

Thanks for trying this. I actually thought about page counters but
quickly dismissed it because zswap needs sub-page charging. I see you
are using the page counters here as byte counters tho :)

It's probably fine, I think the risk of overflow is low (at least on 64-bit).

> >
> > For the setup, the benchmark creates a cgroup chain at depth X with
> > memory.max set to 1G on the leaf and memory.zswap.max set to 512M on
> > every level, and spins up 20 processes there that each allocate 100
> > MiB, fault it in, and touch every page four more times. With that 2000
> > MiB against the 1 GiB memory.max, it triggers reclaim continuously and
> > makes the swap traffic go through zswap. The machine I ran this on had
> > 80 CPUs.
> >
> > I also ran it with no memory.zswap.max set (ie no reads triggered,
> > only update path runs) - as I understand it, this is the configuration
> > that is more often used in practice.
> >
> > These are the results I saw:
> >
> > kernel cpu time (in ns) per zswap store, zswap.max set
> >                       a)                     b)            c)                d)
> > depth 1        567,116          35,604     35,841     34,995
> > depth 2      1,082,007          35,507     37,803     36,209
> > depth 4      2,153,329          37,591     40,117     39,893
> > depth 8      4,211,692          34,963     41,070     44,211
> > depth 32    15,728,220       53,575     94,685     65,946
> >
> > kernel cpu time (in ns) per zswap store, no zswap.max (update path only):
> >                         a)                  b)              c)            d)
> > depth 1         34,787          34,062     34,930     35,630
> > depth 2         34,913          35,061     36,404     36,360
> > depth 4         36,422          36,809     36,922     37,483
> > depth 8         42,177          34,440     36,679     40,793
> > depth 32        55,309          55,321     57,671     59,190
> >
> > c) and d) are for the most part pretty comparable to b) without
> > introducing the staleness problem of b). Between c) and d), I think d)
> > ends up outperforming c) as the # of cpus and depth gets larger.

The main advantage of (c) to me is that we can probably update ~all
memcg stats to use this scheme, or at least the problematic ones, it
should be generic enough. Also, I have a concern about (d), see below.

> >
> > I'm seeing that all the other memory limits (eg memory.swap.max,
> > memory.max, etc) are already using page counters. Is there a reason
> > the zswap stat can't? If not, does it make sense for the zswap stat to
> > switch over to using page counters?
>
> Thanks for running these.
>
> I used the vmstat counter on the assumption that setting zswap.max is
> rare and the counter is maintained anyway for memory.stat.
>
> But I never actually tested it. The assumption was that surely walking
> ancestors on a quick if (max == PAGE_COUNTER_MAX) continue would be
> much cheaper than page counter atomics at every level. And so I'm
> surprised by your results.

+1.

I previously did an experiment with per-cgroup atomics (should be the
same as page counters), and it scaled more poorly than the numbers you
have here. I was running tests in a VM on an AMD Turin CPU, and I
think I tried 10, 20, and 50 processes, so maybe I pushed it to the
limit. I remember seeing a large regression with 50 processes. I used
bpftrace to measure the latency of zswap loads and zswap stores.

Would you be able to also collect numbers with >20 processes and with
zswap loads? I think latency of zswap loads is more critical because
it's usually in the fault path. One other thing is, you need to be
careful with zswap loads because a miss will be really fast, so they
will pull the average latency down. Ideally you'd only measure zswap
load hits. It would also be useful to see the latency at the tail
(e.g. p90, p95, p99), as people usually care a lot about page fault
latency at the tail, not just the average.

Sorry if I am asking too much :)

Honestly, I am not sure if >20 processes is a practical concern, but
zswap load latency is.

That being said, I generally prefer (c) better because it should scale
with more concurrency/CPUs and should generalize better to other
stats. But I am obviously biased :P

>
> But that was the sole reason. If it doesn't stand up to benchmarking,
> no objection to streamlining the control to a standard implementation.

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

[Joanne Koong]

On Fri, Aug 21, 2026 at 12:28 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Fri, Aug 21, 2026 at 10:52 AM Johannes Weiner <hannes@cmpxchg.org> wrote:
> >
> > On Thu, Aug 20, 2026 at 03:18:39PM -0700, Joanne Koong wrote:
> > > On Tue, Aug 18, 2026 at 11:35 AM Yosry Ahmed <yosry@kernel.org> wrote:
> > > >
> > > > On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@kylinos.cn> wrote:
> > > > >
> > > > >
> > > > >
> > > > > 在 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?
> > > >
> > > > I am not actively looking into that, but Joanne was looking into
> > > > AFAICT. I will respond to the thread there and CC Song as well.
> > > >
> > >
> > > For this zswap stat, I ran some benchmarks comparing 4 approaches
> > > (switchable behind a runtime knob [1]):
> > > a) rstat + forced flush       (baseline aka what the tree does today)
> > > b) rstat + ratelimited         (Song's proposal)
> > > c) hierarchical per-CPU   (Yosry's idea from [2])
> > > d) page counters              (following what all the other memcg limits do)
>
> Thanks for trying this. I actually thought about page counters but
> quickly dismissed it because zswap needs sub-page charging. I see you
> are using the page counters here as byte counters tho :)
>
> It's probably fine, I think the risk of overflow is low (at least on 64-bit).
>
> > >
> > > For the setup, the benchmark creates a cgroup chain at depth X with
> > > memory.max set to 1G on the leaf and memory.zswap.max set to 512M on
> > > every level, and spins up 20 processes there that each allocate 100
> > > MiB, fault it in, and touch every page four more times. With that 2000
> > > MiB against the 1 GiB memory.max, it triggers reclaim continuously and
> > > makes the swap traffic go through zswap. The machine I ran this on had
> > > 80 CPUs.
> > >
> > > I also ran it with no memory.zswap.max set (ie no reads triggered,
> > > only update path runs) - as I understand it, this is the configuration
> > > that is more often used in practice.
> > >
> > > These are the results I saw:
> > >
> > > kernel cpu time (in ns) per zswap store, zswap.max set
> > >                       a)                     b)            c)                d)
> > > depth 1        567,116          35,604     35,841     34,995
> > > depth 2      1,082,007          35,507     37,803     36,209
> > > depth 4      2,153,329          37,591     40,117     39,893
> > > depth 8      4,211,692          34,963     41,070     44,211
> > > depth 32    15,728,220       53,575     94,685     65,946
> > >
> > > kernel cpu time (in ns) per zswap store, no zswap.max (update path only):
> > >                         a)                  b)              c)            d)
> > > depth 1         34,787          34,062     34,930     35,630
> > > depth 2         34,913          35,061     36,404     36,360
> > > depth 4         36,422          36,809     36,922     37,483
> > > depth 8         42,177          34,440     36,679     40,793
> > > depth 32        55,309          55,321     57,671     59,190
> > >
> > > c) and d) are for the most part pretty comparable to b) without
> > > introducing the staleness problem of b). Between c) and d), I think d)
> > > ends up outperforming c) as the # of cpus and depth gets larger.
>
> The main advantage of (c) to me is that we can probably update ~all
> memcg stats to use this scheme, or at least the problematic ones, it
> should be generic enough. Also, I have a concern about (d), see below.
>
> > >
> > > I'm seeing that all the other memory limits (eg memory.swap.max,
> > > memory.max, etc) are already using page counters. Is there a reason
> > > the zswap stat can't? If not, does it make sense for the zswap stat to
> > > switch over to using page counters?
> >
> > Thanks for running these.
> >
> > I used the vmstat counter on the assumption that setting zswap.max is
> > rare and the counter is maintained anyway for memory.stat.
> >
> > But I never actually tested it. The assumption was that surely walking
> > ancestors on a quick if (max == PAGE_COUNTER_MAX) continue would be
> > much cheaper than page counter atomics at every level. And so I'm
> > surprised by your results.

I think your assumptions are correct. In that second table above (no
zswap.max set, only update path runs), d) has worse performance than
the baseline a) (except for depth=8, which was a noisy fluke). Rerun
with 20 reps, I saw similiar-ish results:

depth       a)                       d)                       diff
     1  35051 +/- 116   35635 +/- 149    +584 +/- 189   (+1.7%)
     2  35603 +/- 142   35859 +/- 179    +256 +/- 229   (+0.7%)
     4  36358 +/- 115   37761 +/- 167   +1403 +/- 203   (+3.9%)
     8  39537 +/- 178   41225 +/- 240   +1688 +/- 299   (+4.3%)
   32  52181 +/- 210   59042 +/- 161   +6861 +/- 265  (+13.2%)

I think the atomics do cost roughly what you assumed, but compared to
the overall latency of the zswap path, it's adding hundreds of
nanoseconds to a path that takes tens of microseconds.

>
> +1.
>
> I previously did an experiment with per-cgroup atomics (should be the
> same as page counters), and it scaled more poorly than the numbers you
> have here. I was running tests in a VM on an AMD Turin CPU, and I
> think I tried 10, 20, and 50 processes, so maybe I pushed it to the
> limit. I remember seeing a large regression with 50 processes. I used
> bpftrace to measure the latency of zswap loads and zswap stores.
>
> Would you be able to also collect numbers with >20 processes and with

Beyond 20 workers, I'm seeing that the additional workers basically
just queue, without improving throughput much. I don't think this
bottleneck is related to the accounting method used though. a), c) and
d) all are within 1% of each other at 40 and 80 workers, so all 3
approaches are bottlenecked by this.

> zswap loads? I think latency of zswap loads is more critical because
> it's usually in the fault path. One other thing is, you need to be
> careful with zswap loads because a miss will be really fast, so they
> will pull the average latency down. Ideally you'd only measure zswap
> load hits. It would also be useful to see the latency at the tail
> (e.g. p90, p95, p99), as people usually care a lot about page fault
> latency at the tail, not just the average.
>
> Sorry if I am asking too much :)
>
> Honestly, I am not sure if >20 processes is a practical concern, but
> zswap load latency is.

Ah, thanks for pointing out the zswap load path and its relation to
faults. These are the results I'm seeing:

zswap_load() latency, (hits only (retval == 0), no zswap.max set, 20
workers on 80 cpus, 5 runs, ns):
                  a)              c)       d)
  depth 2
    p50        6,850      6,700    6,850
    p90       10,000    9,750   10,000
    p95       11,050   10,800   11,000
    p99       13,600   13,350   13,500
    avg       7,269    7,128    7,297
  depth 4
    p50        6,600    6,500    6,700
    p90        9,700    9,650    9,950
    p95       10,800   10,750   11,000
    p99       13,350   13,250   13,550
    avg       7,034    7,029    7,266
  depth 8
    p50        6,000    6,150    6,750
    p90        9,000    9,250    9,900
    p95       10,050   10,300   11,050
    p99       12,500   12,800   13,650
    avg       6,508    6,652    7,273
  depth 32
    p50        5,250    5,400    9,100
    p90        7,900    8,050   12,550
    p95        9,000    9,150   13,800
    p99       11,550   11,700   16,450
    avg       5,786    5,958    9,469

For depths 2 and 4, there's no real difference for d), but depth 8+
shows worse performance. In Meta's fleet, hierarchies of depth 8+ are
common.

>
> That being said, I generally prefer (c) better because it should scale

I now prefer (c) as well. I hadn't realized setting zswap.max is a
rare path until Johannes mentioned it, and with your mention of
zswap_load() sitting in the fault-critical path, I think the benchmark
results show a clear improvement for c) over d) at higher depths.

I was uneasy about the cost of c)'s reads scaling linearly with the #
of cpus on a system (ie for_each_possible_cpu per level), but given
the uncommonness of the zswap.max path, I think that's the right thing
to trade away. Either way, it's a big improvement over the baseline a)
path that currently exists anyways.

> with more concurrency/CPUs and should generalize better to other
> stats. But I am obviously biased :P

I'm still investigating the writeback and vmscan cases. For writeback,
using (c) is more complicated since NR_FILE_DIRTY and NR_WRITEBACK are
node stats. I'm planning to spend time this week running benchmarks
for it.

If for those cases, (c) is viable, then I'll send a patch that adds
(c) as general infrastructure. Otherwise, I'll send out (c) as a zswap
specific patch.

Does this sound good to everyone? If there are any objections, please
let me know.

Thanks,
Joanne

>
> >
> > But that was the sole reason. If it doesn't stand up to benchmarking,
> > no objection to streamlining the control to a standard implementation.

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

[Yosry Ahmed]

> > > > For this zswap stat, I ran some benchmarks comparing 4 approaches
> > > > (switchable behind a runtime knob [1]):
> > > > a) rstat + forced flush       (baseline aka what the tree does today)
> > > > b) rstat + ratelimited         (Song's proposal)
> > > > c) hierarchical per-CPU   (Yosry's idea from [2])
> > > > d) page counters              (following what all the other memcg limits do)
> >
> > Thanks for trying this. I actually thought about page counters but
> > quickly dismissed it because zswap needs sub-page charging. I see you
> > are using the page counters here as byte counters tho :)
> >
> > It's probably fine, I think the risk of overflow is low (at least on 64-bit).
> >
> > > >
> > > > For the setup, the benchmark creates a cgroup chain at depth X with
> > > > memory.max set to 1G on the leaf and memory.zswap.max set to 512M on
> > > > every level, and spins up 20 processes there that each allocate 100
> > > > MiB, fault it in, and touch every page four more times. With that 2000
> > > > MiB against the 1 GiB memory.max, it triggers reclaim continuously and
> > > > makes the swap traffic go through zswap. The machine I ran this on had
> > > > 80 CPUs.
> > > >
> > > > I also ran it with no memory.zswap.max set (ie no reads triggered,
> > > > only update path runs) - as I understand it, this is the configuration
> > > > that is more often used in practice.
> > > >
> > > > These are the results I saw:
> > > >
> > > > kernel cpu time (in ns) per zswap store, zswap.max set
> > > >                       a)                     b)            c)                d)
> > > > depth 1        567,116          35,604     35,841     34,995
> > > > depth 2      1,082,007          35,507     37,803     36,209
> > > > depth 4      2,153,329          37,591     40,117     39,893
> > > > depth 8      4,211,692          34,963     41,070     44,211
> > > > depth 32    15,728,220       53,575     94,685     65,946
> > > >
> > > > kernel cpu time (in ns) per zswap store, no zswap.max (update path only):
> > > >                         a)                  b)              c)            d)
> > > > depth 1         34,787          34,062     34,930     35,630
> > > > depth 2         34,913          35,061     36,404     36,360
> > > > depth 4         36,422          36,809     36,922     37,483
> > > > depth 8         42,177          34,440     36,679     40,793
> > > > depth 32        55,309          55,321     57,671     59,190
> > > >
> > > > c) and d) are for the most part pretty comparable to b) without
> > > > introducing the staleness problem of b). Between c) and d), I think d)
> > > > ends up outperforming c) as the # of cpus and depth gets larger.
> >
> > The main advantage of (c) to me is that we can probably update ~all
> > memcg stats to use this scheme, or at least the problematic ones, it
> > should be generic enough. Also, I have a concern about (d), see below.
> >
> > > >
> > > > I'm seeing that all the other memory limits (eg memory.swap.max,
> > > > memory.max, etc) are already using page counters. Is there a reason
> > > > the zswap stat can't? If not, does it make sense for the zswap stat to
> > > > switch over to using page counters?
> > >
> > > Thanks for running these.
> > >
> > > I used the vmstat counter on the assumption that setting zswap.max is
> > > rare and the counter is maintained anyway for memory.stat.
> > >
> > > But I never actually tested it. The assumption was that surely walking
> > > ancestors on a quick if (max == PAGE_COUNTER_MAX) continue would be
> > > much cheaper than page counter atomics at every level. And so I'm
> > > surprised by your results.
>
> I think your assumptions are correct. In that second table above (no
> zswap.max set, only update path runs), d) has worse performance than
> the baseline a) (except for depth=8, which was a noisy fluke). Rerun
> with 20 reps, I saw similiar-ish results:
>
> depth       a)                       d)                       diff
>      1  35051 +/- 116   35635 +/- 149    +584 +/- 189   (+1.7%)
>      2  35603 +/- 142   35859 +/- 179    +256 +/- 229   (+0.7%)
>      4  36358 +/- 115   37761 +/- 167   +1403 +/- 203   (+3.9%)
>      8  39537 +/- 178   41225 +/- 240   +1688 +/- 299   (+4.3%)
>    32  52181 +/- 210   59042 +/- 161   +6861 +/- 265  (+13.2%)
>
> I think the atomics do cost roughly what you assumed, but compared to
> the overall latency of the zswap path, it's adding hundreds of
> nanoseconds to a path that takes tens of microseconds.
>
> >
> > +1.
> >
> > I previously did an experiment with per-cgroup atomics (should be the
> > same as page counters), and it scaled more poorly than the numbers you
> > have here. I was running tests in a VM on an AMD Turin CPU, and I
> > think I tried 10, 20, and 50 processes, so maybe I pushed it to the
> > limit. I remember seeing a large regression with 50 processes. I used
> > bpftrace to measure the latency of zswap loads and zswap stores.
> >
> > Would you be able to also collect numbers with >20 processes and with
>
> Beyond 20 workers, I'm seeing that the additional workers basically
> just queue, without improving throughput much. I don't think this
> bottleneck is related to the accounting method used though. a), c) and
> d) all are within 1% of each other at 40 and 80 workers, so all 3
> approaches are bottlenecked by this.
>
> > zswap loads? I think latency of zswap loads is more critical because
> > it's usually in the fault path. One other thing is, you need to be
> > careful with zswap loads because a miss will be really fast, so they
> > will pull the average latency down. Ideally you'd only measure zswap
> > load hits. It would also be useful to see the latency at the tail
> > (e.g. p90, p95, p99), as people usually care a lot about page fault
> > latency at the tail, not just the average.
> >
> > Sorry if I am asking too much :)
> >
> > Honestly, I am not sure if >20 processes is a practical concern, but
> > zswap load latency is.
>
> Ah, thanks for pointing out the zswap load path and its relation to
> faults. These are the results I'm seeing:
>
> zswap_load() latency, (hits only (retval == 0), no zswap.max set, 20
> workers on 80 cpus, 5 runs, ns):
>                   a)              c)       d)
>   depth 2
>     p50        6,850      6,700    6,850
>     p90       10,000    9,750   10,000
>     p95       11,050   10,800   11,000
>     p99       13,600   13,350   13,500
>     avg       7,269    7,128    7,297
>   depth 4
>     p50        6,600    6,500    6,700
>     p90        9,700    9,650    9,950
>     p95       10,800   10,750   11,000
>     p99       13,350   13,250   13,550
>     avg       7,034    7,029    7,266
>   depth 8
>     p50        6,000    6,150    6,750
>     p90        9,000    9,250    9,900
>     p95       10,050   10,300   11,050
>     p99       12,500   12,800   13,650
>     avg       6,508    6,652    7,273
>   depth 32
>     p50        5,250    5,400    9,100
>     p90        7,900    8,050   12,550
>     p95        9,000    9,150   13,800
>     p99       11,550   11,700   16,450
>     avg       5,786    5,958    9,469
>
> For depths 2 and 4, there's no real difference for d), but depth 8+
> shows worse performance. In Meta's fleet, hierarchies of depth 8+ are
> common.
>
> >
> > That being said, I generally prefer (c) better because it should scale
>
> I now prefer (c) as well. I hadn't realized setting zswap.max is a
> rare path until Johannes mentioned it, and with your mention of
> zswap_load() sitting in the fault-critical path, I think the benchmark
> results show a clear improvement for c) over d) at higher depths.
>
> I was uneasy about the cost of c)'s reads scaling linearly with the #
> of cpus on a system (ie for_each_possible_cpu per level), but given
> the uncommonness of the zswap.max path, I think that's the right thing
> to trade away. Either way, it's a big improvement over the baseline a)
> path that currently exists anyways.
>
> > with more concurrency/CPUs and should generalize better to other
> > stats. But I am obviously biased :P
>
> I'm still investigating the writeback and vmscan cases. For writeback,
> using (c) is more complicated since NR_FILE_DIRTY and NR_WRITEBACK are
> node stats. I'm planning to spend time this week running benchmarks
> for it.
>
> If for those cases, (c) is viable, then I'll send a patch that adds
> (c) as general infrastructure. Otherwise, I'll send out (c) as a zswap
> specific patch.
>
> Does this sound good to everyone? If there are any objections, please
> let me know.

If (c) is holding up for writeback and vmscan, I would question a more
radical approach of tearing apart the rstat framework and using it
across the board. That is obviously a heavier lift and more
controversial, but if we can get away with it, I think it will
simplify things greatly and honestly rstat has been causing a lot of
trouble in the last few years.

But this can be done incrementally too, we can start by separating out
the problematic stats to use the new update/flushing scheme, and then
do the larger overhaul when it holds up.

Either way, thanks a lot for all the work you're doing here.

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

[Joshua Hahn]

On Thu, 20 Aug 2026 15:18:39 -0700 Joanne Koong <joannelkoong@gmail.com> wrote:

> On Tue, Aug 18, 2026 at 11:35 AM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@kylinos.cn> wrote:
> > >
> > >
> > >
> > > 在 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?
> >
> > I am not actively looking into that, but Joanne was looking into
> > AFAICT. I will respond to the thread there and CC Song as well.
> >
> 
> For this zswap stat, I ran some benchmarks comparing 4 approaches
> (switchable behind a runtime knob [1]):
> a) rstat + forced flush       (baseline aka what the tree does today)
> b) rstat + ratelimited         (Song's proposal)
> c) hierarchical per-CPU   (Yosry's idea from [2])
> d) page counters              (following what all the other memcg limits do)

Hi Joanne,

I hope you are doing well! Thank you for running all these experiments.
I'm not biased in any direction but I was curious what you did for the
page counter implementation. The reason I am asking is because I am
working on making page counters more scalable / performant by pushing
the memcg stock down to the page counter level [3]. I wonder if the
caching that stock provides can give (d) an upper hand over (c).

This is just an idea though, and I think this would depend on how your
(d) implementation looks like.

Sorry that I joined in this conversation late : -(

Thanks again, Joanne!
Joshua

[3] https://lore.kernel.org/all/20260623180124.868655-1-joshua.hahnjy@gmail.com/
(It's been a while since v4, I got a little sidetracked working on the
tiered memcg series. I have a v5 ready to go, just waiting on some
running some tests before sending it out.)

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

[Joanne Koong]

On Mon, Aug 24, 2026 at 2:16 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> > > > > For this zswap stat, I ran some benchmarks comparing 4 approaches
> > > > > (switchable behind a runtime knob [1]):
> > > > > a) rstat + forced flush       (baseline aka what the tree does today)
> > > > > b) rstat + ratelimited         (Song's proposal)
> > > > > c) hierarchical per-CPU   (Yosry's idea from [2])
> > > > > d) page counters              (following what all the other memcg limits do)
> > >
> > > Thanks for trying this. I actually thought about page counters but
> > > quickly dismissed it because zswap needs sub-page charging. I see you
> > > are using the page counters here as byte counters tho :)
> > >
> > > It's probably fine, I think the risk of overflow is low (at least on 64-bit).
> > >
> > > > >
> > > > > For the setup, the benchmark creates a cgroup chain at depth X with
> > > > > memory.max set to 1G on the leaf and memory.zswap.max set to 512M on
> > > > > every level, and spins up 20 processes there that each allocate 100
> > > > > MiB, fault it in, and touch every page four more times. With that 2000
> > > > > MiB against the 1 GiB memory.max, it triggers reclaim continuously and
> > > > > makes the swap traffic go through zswap. The machine I ran this on had
> > > > > 80 CPUs.
> > > > >
> > > > > I also ran it with no memory.zswap.max set (ie no reads triggered,
> > > > > only update path runs) - as I understand it, this is the configuration
> > > > > that is more often used in practice.
> > > > >
> > > > > These are the results I saw:
> > > > >
> > > > > kernel cpu time (in ns) per zswap store, zswap.max set
> > > > >                       a)                     b)            c)                d)
> > > > > depth 1        567,116          35,604     35,841     34,995
> > > > > depth 2      1,082,007          35,507     37,803     36,209
> > > > > depth 4      2,153,329          37,591     40,117     39,893
> > > > > depth 8      4,211,692          34,963     41,070     44,211
> > > > > depth 32    15,728,220       53,575     94,685     65,946
> > > > >
> > > > > kernel cpu time (in ns) per zswap store, no zswap.max (update path only):
> > > > >                         a)                  b)              c)            d)
> > > > > depth 1         34,787          34,062     34,930     35,630
> > > > > depth 2         34,913          35,061     36,404     36,360
> > > > > depth 4         36,422          36,809     36,922     37,483
> > > > > depth 8         42,177          34,440     36,679     40,793
> > > > > depth 32        55,309          55,321     57,671     59,190
> > > > >
> > > > > c) and d) are for the most part pretty comparable to b) without
> > > > > introducing the staleness problem of b). Between c) and d), I think d)
> > > > > ends up outperforming c) as the # of cpus and depth gets larger.
> > >
> > > The main advantage of (c) to me is that we can probably update ~all
> > > memcg stats to use this scheme, or at least the problematic ones, it
> > > should be generic enough. Also, I have a concern about (d), see below.
> > >
> > > > >
> > > > > I'm seeing that all the other memory limits (eg memory.swap.max,
> > > > > memory.max, etc) are already using page counters. Is there a reason
> > > > > the zswap stat can't? If not, does it make sense for the zswap stat to
> > > > > switch over to using page counters?
> > > >
> > > > Thanks for running these.
> > > >
> > > > I used the vmstat counter on the assumption that setting zswap.max is
> > > > rare and the counter is maintained anyway for memory.stat.
> > > >
> > > > But I never actually tested it. The assumption was that surely walking
> > > > ancestors on a quick if (max == PAGE_COUNTER_MAX) continue would be
> > > > much cheaper than page counter atomics at every level. And so I'm
> > > > surprised by your results.
> >
> > I think your assumptions are correct. In that second table above (no
> > zswap.max set, only update path runs), d) has worse performance than
> > the baseline a) (except for depth=8, which was a noisy fluke). Rerun
> > with 20 reps, I saw similiar-ish results:
> >
> > depth       a)                       d)                       diff
> >      1  35051 +/- 116   35635 +/- 149    +584 +/- 189   (+1.7%)
> >      2  35603 +/- 142   35859 +/- 179    +256 +/- 229   (+0.7%)
> >      4  36358 +/- 115   37761 +/- 167   +1403 +/- 203   (+3.9%)
> >      8  39537 +/- 178   41225 +/- 240   +1688 +/- 299   (+4.3%)
> >    32  52181 +/- 210   59042 +/- 161   +6861 +/- 265  (+13.2%)
> >
> > I think the atomics do cost roughly what you assumed, but compared to
> > the overall latency of the zswap path, it's adding hundreds of
> > nanoseconds to a path that takes tens of microseconds.
> >
> > >
> > > +1.
> > >
> > > I previously did an experiment with per-cgroup atomics (should be the
> > > same as page counters), and it scaled more poorly than the numbers you
> > > have here. I was running tests in a VM on an AMD Turin CPU, and I
> > > think I tried 10, 20, and 50 processes, so maybe I pushed it to the
> > > limit. I remember seeing a large regression with 50 processes. I used
> > > bpftrace to measure the latency of zswap loads and zswap stores.
> > >
> > > Would you be able to also collect numbers with >20 processes and with
> >
> > Beyond 20 workers, I'm seeing that the additional workers basically
> > just queue, without improving throughput much. I don't think this
> > bottleneck is related to the accounting method used though. a), c) and
> > d) all are within 1% of each other at 40 and 80 workers, so all 3
> > approaches are bottlenecked by this.
> >
> > > zswap loads? I think latency of zswap loads is more critical because
> > > it's usually in the fault path. One other thing is, you need to be
> > > careful with zswap loads because a miss will be really fast, so they
> > > will pull the average latency down. Ideally you'd only measure zswap
> > > load hits. It would also be useful to see the latency at the tail
> > > (e.g. p90, p95, p99), as people usually care a lot about page fault
> > > latency at the tail, not just the average.
> > >
> > > Sorry if I am asking too much :)
> > >
> > > Honestly, I am not sure if >20 processes is a practical concern, but
> > > zswap load latency is.
> >
> > Ah, thanks for pointing out the zswap load path and its relation to
> > faults. These are the results I'm seeing:
> >
> > zswap_load() latency, (hits only (retval == 0), no zswap.max set, 20
> > workers on 80 cpus, 5 runs, ns):
> >                   a)              c)       d)
> >   depth 2
> >     p50        6,850      6,700    6,850
> >     p90       10,000    9,750   10,000
> >     p95       11,050   10,800   11,000
> >     p99       13,600   13,350   13,500
> >     avg       7,269    7,128    7,297
> >   depth 4
> >     p50        6,600    6,500    6,700
> >     p90        9,700    9,650    9,950
> >     p95       10,800   10,750   11,000
> >     p99       13,350   13,250   13,550
> >     avg       7,034    7,029    7,266
> >   depth 8
> >     p50        6,000    6,150    6,750
> >     p90        9,000    9,250    9,900
> >     p95       10,050   10,300   11,050
> >     p99       12,500   12,800   13,650
> >     avg       6,508    6,652    7,273
> >   depth 32
> >     p50        5,250    5,400    9,100
> >     p90        7,900    8,050   12,550
> >     p95        9,000    9,150   13,800
> >     p99       11,550   11,700   16,450
> >     avg       5,786    5,958    9,469
> >
> > For depths 2 and 4, there's no real difference for d), but depth 8+
> > shows worse performance. In Meta's fleet, hierarchies of depth 8+ are
> > common.
> >
> > >
> > > That being said, I generally prefer (c) better because it should scale
> >
> > I now prefer (c) as well. I hadn't realized setting zswap.max is a
> > rare path until Johannes mentioned it, and with your mention of
> > zswap_load() sitting in the fault-critical path, I think the benchmark
> > results show a clear improvement for c) over d) at higher depths.
> >
> > I was uneasy about the cost of c)'s reads scaling linearly with the #
> > of cpus on a system (ie for_each_possible_cpu per level), but given
> > the uncommonness of the zswap.max path, I think that's the right thing
> > to trade away. Either way, it's a big improvement over the baseline a)
> > path that currently exists anyways.
> >
> > > with more concurrency/CPUs and should generalize better to other
> > > stats. But I am obviously biased :P
> >
> > I'm still investigating the writeback and vmscan cases. For writeback,
> > using (c) is more complicated since NR_FILE_DIRTY and NR_WRITEBACK are
> > node stats. I'm planning to spend time this week running benchmarks
> > for it.
> >
> > If for those cases, (c) is viable, then I'll send a patch that adds
> > (c) as general infrastructure. Otherwise, I'll send out (c) as a zswap
> > specific patch.
> >
> > Does this sound good to everyone? If there are any objections, please
> > let me know.
>
> If (c) is holding up for writeback and vmscan, I would question a more
> radical approach of tearing apart the rstat framework and using it
> across the board. That is obviously a heavier lift and more
> controversial, but if we can get away with it, I think it will
> simplify things greatly and honestly rstat has been causing a lot of
> trouble in the last few years.
>
> But this can be done incrementally too, we can start by separating out
> the problematic stats to use the new update/flushing scheme, and then
> do the larger overhaul when it holds up.
>
> Either way, thanks a lot for all the work you're doing here.

Thanks for all your guidance with this memcg stats work, Yosry! I appreciate it.

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

[Joanne Koong]

On Mon, Aug 24, 2026 at 2:31 PM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
>
> On Thu, 20 Aug 2026 15:18:39 -0700 Joanne Koong <joannelkoong@gmail.com> wrote:
>
> > On Tue, Aug 18, 2026 at 11:35 AM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
> > > On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@kylinos.cn> wrote:
> > > >
> > > >
> > > >
> > > > 在 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?
> > >
> > > I am not actively looking into that, but Joanne was looking into
> > > AFAICT. I will respond to the thread there and CC Song as well.
> > >
> >
> > For this zswap stat, I ran some benchmarks comparing 4 approaches
> > (switchable behind a runtime knob [1]):
> > a) rstat + forced flush       (baseline aka what the tree does today)
> > b) rstat + ratelimited         (Song's proposal)
> > c) hierarchical per-CPU   (Yosry's idea from [2])
> > d) page counters              (following what all the other memcg limits do)
>
> Hi Joanne,

Hi Joshua!

>
> I hope you are doing well! Thank you for running all these experiments.
> I'm not biased in any direction but I was curious what you did for the
> page counter implementation. The reason I am asking is because I am

Thanks for bringing my attention to your page counter stock series.
The implementation of page counters I used for my benchmarking [1] was
unstocked, where it incurs an atomic cost per charge / uncharge
instead of doing any sort of batching/caching.

> working on making page counters more scalable / performant by pushing
> the memcg stock down to the page counter level [3]. I wonder if the
> caching that stock provides can give (d) an upper hand over (c).
>

I think caching the stock would definitely help the update cost of (d)
as you point out, but unfortunately for this zswap case, the nr_cpus *
# descendants * 64 max over-reporting the caching would introduce to
the zswap stat could lead to overly conservative rejections of stores
that should have been allowed and/or OOM killings for cgroups where
zswap writeback is disabled. Thanks for bringing this idea up though -
I had a similar idea to this with batching the per-cpu counter [2],
until Johannes graciously directed me to commit 2d146aa3aa842 (mm:
memcontrol: switch to rstat), which had switched from per-cpu batching
to rstat precisely because the nr_cpus * # descendants * # batch pages
error bound was getting problematic.

Thanks,
Joanne

[1] https://github.com/joannekoong/linux/commit/9092e0057f038ffca43ae3f35faf58c33b1d7c9b
[2] https://lore.kernel.org/linux-fsdevel/anpsLy_pCanocgS1@google.com/T/#m5ed6ff8e64eeb909ff3c92bde5105406b1ddaf10

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

[Shakeel Butt]

On Mon, Aug 24, 2026 at 02:16:40PM -0700, Yosry Ahmed wrote:
> > > > > For this zswap stat, I ran some benchmarks comparing 4 approaches
> > > > > (switchable behind a runtime knob [1]):
> > > > > a) rstat + forced flush       (baseline aka what the tree does today)
> > > > > b) rstat + ratelimited         (Song's proposal)
> > > > > c) hierarchical per-CPU   (Yosry's idea from [2])
> > > > > d) page counters              (following what all the other memcg limits do)
> > >
> > > Thanks for trying this. I actually thought about page counters but
> > > quickly dismissed it because zswap needs sub-page charging. I see you
> > > are using the page counters here as byte counters tho :)
> > >
> > > It's probably fine, I think the risk of overflow is low (at least on 64-bit).
> > >
> > > > >
> > > > > For the setup, the benchmark creates a cgroup chain at depth X with
> > > > > memory.max set to 1G on the leaf and memory.zswap.max set to 512M on
> > > > > every level, and spins up 20 processes there that each allocate 100
> > > > > MiB, fault it in, and touch every page four more times. With that 2000
> > > > > MiB against the 1 GiB memory.max, it triggers reclaim continuously and
> > > > > makes the swap traffic go through zswap. The machine I ran this on had
> > > > > 80 CPUs.
> > > > >
> > > > > I also ran it with no memory.zswap.max set (ie no reads triggered,
> > > > > only update path runs) - as I understand it, this is the configuration
> > > > > that is more often used in practice.
> > > > >
> > > > > These are the results I saw:
> > > > >
> > > > > kernel cpu time (in ns) per zswap store, zswap.max set
> > > > >                       a)                     b)            c)                d)
> > > > > depth 1        567,116          35,604     35,841     34,995
> > > > > depth 2      1,082,007          35,507     37,803     36,209
> > > > > depth 4      2,153,329          37,591     40,117     39,893
> > > > > depth 8      4,211,692          34,963     41,070     44,211
> > > > > depth 32    15,728,220       53,575     94,685     65,946
> > > > >
> > > > > kernel cpu time (in ns) per zswap store, no zswap.max (update path only):
> > > > >                         a)                  b)              c)            d)
> > > > > depth 1         34,787          34,062     34,930     35,630
> > > > > depth 2         34,913          35,061     36,404     36,360
> > > > > depth 4         36,422          36,809     36,922     37,483
> > > > > depth 8         42,177          34,440     36,679     40,793
> > > > > depth 32        55,309          55,321     57,671     59,190
> > > > >
> > > > > c) and d) are for the most part pretty comparable to b) without
> > > > > introducing the staleness problem of b). Between c) and d), I think d)
> > > > > ends up outperforming c) as the # of cpus and depth gets larger.
> > >
> > > The main advantage of (c) to me is that we can probably update ~all
> > > memcg stats to use this scheme, or at least the problematic ones, it
> > > should be generic enough. Also, I have a concern about (d), see below.
> > >
> > > > >
> > > > > I'm seeing that all the other memory limits (eg memory.swap.max,
> > > > > memory.max, etc) are already using page counters. Is there a reason
> > > > > the zswap stat can't? If not, does it make sense for the zswap stat to
> > > > > switch over to using page counters?
> > > >
> > > > Thanks for running these.
> > > >
> > > > I used the vmstat counter on the assumption that setting zswap.max is
> > > > rare and the counter is maintained anyway for memory.stat.
> > > >
> > > > But I never actually tested it. The assumption was that surely walking
> > > > ancestors on a quick if (max == PAGE_COUNTER_MAX) continue would be
> > > > much cheaper than page counter atomics at every level. And so I'm
> > > > surprised by your results.
> >
> > I think your assumptions are correct. In that second table above (no
> > zswap.max set, only update path runs), d) has worse performance than
> > the baseline a) (except for depth=8, which was a noisy fluke). Rerun
> > with 20 reps, I saw similiar-ish results:
> >
> > depth       a)                       d)                       diff
> >      1  35051 +/- 116   35635 +/- 149    +584 +/- 189   (+1.7%)
> >      2  35603 +/- 142   35859 +/- 179    +256 +/- 229   (+0.7%)
> >      4  36358 +/- 115   37761 +/- 167   +1403 +/- 203   (+3.9%)
> >      8  39537 +/- 178   41225 +/- 240   +1688 +/- 299   (+4.3%)
> >    32  52181 +/- 210   59042 +/- 161   +6861 +/- 265  (+13.2%)
> >
> > I think the atomics do cost roughly what you assumed, but compared to
> > the overall latency of the zswap path, it's adding hundreds of
> > nanoseconds to a path that takes tens of microseconds.
> >
> > >
> > > +1.
> > >
> > > I previously did an experiment with per-cgroup atomics (should be the
> > > same as page counters), and it scaled more poorly than the numbers you
> > > have here. I was running tests in a VM on an AMD Turin CPU, and I
> > > think I tried 10, 20, and 50 processes, so maybe I pushed it to the
> > > limit. I remember seeing a large regression with 50 processes. I used
> > > bpftrace to measure the latency of zswap loads and zswap stores.
> > >
> > > Would you be able to also collect numbers with >20 processes and with
> >
> > Beyond 20 workers, I'm seeing that the additional workers basically
> > just queue, without improving throughput much. I don't think this
> > bottleneck is related to the accounting method used though. a), c) and
> > d) all are within 1% of each other at 40 and 80 workers, so all 3
> > approaches are bottlenecked by this.
> >
> > > zswap loads? I think latency of zswap loads is more critical because
> > > it's usually in the fault path. One other thing is, you need to be
> > > careful with zswap loads because a miss will be really fast, so they
> > > will pull the average latency down. Ideally you'd only measure zswap
> > > load hits. It would also be useful to see the latency at the tail
> > > (e.g. p90, p95, p99), as people usually care a lot about page fault
> > > latency at the tail, not just the average.
> > >
> > > Sorry if I am asking too much :)
> > >
> > > Honestly, I am not sure if >20 processes is a practical concern, but
> > > zswap load latency is.
> >
> > Ah, thanks for pointing out the zswap load path and its relation to
> > faults. These are the results I'm seeing:
> >
> > zswap_load() latency, (hits only (retval == 0), no zswap.max set, 20
> > workers on 80 cpus, 5 runs, ns):
> >                   a)              c)       d)
> >   depth 2
> >     p50        6,850      6,700    6,850
> >     p90       10,000    9,750   10,000
> >     p95       11,050   10,800   11,000
> >     p99       13,600   13,350   13,500
> >     avg       7,269    7,128    7,297
> >   depth 4
> >     p50        6,600    6,500    6,700
> >     p90        9,700    9,650    9,950
> >     p95       10,800   10,750   11,000
> >     p99       13,350   13,250   13,550
> >     avg       7,034    7,029    7,266
> >   depth 8
> >     p50        6,000    6,150    6,750
> >     p90        9,000    9,250    9,900
> >     p95       10,050   10,300   11,050
> >     p99       12,500   12,800   13,650
> >     avg       6,508    6,652    7,273
> >   depth 32
> >     p50        5,250    5,400    9,100
> >     p90        7,900    8,050   12,550
> >     p95        9,000    9,150   13,800
> >     p99       11,550   11,700   16,450
> >     avg       5,786    5,958    9,469
> >
> > For depths 2 and 4, there's no real difference for d), but depth 8+
> > shows worse performance. In Meta's fleet, hierarchies of depth 8+ are
> > common.
> >
> > >
> > > That being said, I generally prefer (c) better because it should scale
> >
> > I now prefer (c) as well. I hadn't realized setting zswap.max is a
> > rare path until Johannes mentioned it, and with your mention of
> > zswap_load() sitting in the fault-critical path, I think the benchmark
> > results show a clear improvement for c) over d) at higher depths.
> >
> > I was uneasy about the cost of c)'s reads scaling linearly with the #
> > of cpus on a system (ie for_each_possible_cpu per level), but given
> > the uncommonness of the zswap.max path, I think that's the right thing
> > to trade away. Either way, it's a big improvement over the baseline a)
> > path that currently exists anyways.
> >
> > > with more concurrency/CPUs and should generalize better to other
> > > stats. But I am obviously biased :P
> >
> > I'm still investigating the writeback and vmscan cases. For writeback,
> > using (c) is more complicated since NR_FILE_DIRTY and NR_WRITEBACK are
> > node stats. I'm planning to spend time this week running benchmarks
> > for it.
> >
> > If for those cases, (c) is viable, then I'll send a patch that adds
> > (c) as general infrastructure. Otherwise, I'll send out (c) as a zswap
> > specific patch.
> >
> > Does this sound good to everyone? If there are any objections, please
> > let me know.
> 
> If (c) is holding up for writeback and vmscan, I would question a more
> radical approach of tearing apart the rstat framework and using it
> across the board. That is obviously a heavier lift and more
> controversial,

Yes controversial because we used to have similar mechanism which did not work
and we had to move to rstat. Check the commit 42a300353577 and fixes to it.

This worked for zswap stats because their update and consumption are not on
performance critical code paths i.e. these are on the way to compress or
decompress or in reclaim context.

> but if we can get away with it, I think it will
> simplify things greatly and honestly rstat has been causing a lot of
> trouble in the last few years.
> 
> But this can be done incrementally too, we can start by separating out
> the problematic stats to use the new update/flushing scheme,

This I think we do need but for specific stats. I think the stats which have
in-kernel consumers need this. However not all such stats might be fine with
slow update side like zswap. So, we need to evaluate thoroughly.

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

[Yosry Ahmed]

[..]
> > > I now prefer (c) as well. I hadn't realized setting zswap.max is a
> > > rare path until Johannes mentioned it, and with your mention of
> > > zswap_load() sitting in the fault-critical path, I think the benchmark
> > > results show a clear improvement for c) over d) at higher depths.
> > >
> > > I was uneasy about the cost of c)'s reads scaling linearly with the #
> > > of cpus on a system (ie for_each_possible_cpu per level), but given
> > > the uncommonness of the zswap.max path, I think that's the right thing
> > > to trade away. Either way, it's a big improvement over the baseline a)
> > > path that currently exists anyways.
> > >
> > > > with more concurrency/CPUs and should generalize better to other
> > > > stats. But I am obviously biased :P
> > >
> > > I'm still investigating the writeback and vmscan cases. For writeback,
> > > using (c) is more complicated since NR_FILE_DIRTY and NR_WRITEBACK are
> > > node stats. I'm planning to spend time this week running benchmarks
> > > for it.
> > >
> > > If for those cases, (c) is viable, then I'll send a patch that adds
> > > (c) as general infrastructure. Otherwise, I'll send out (c) as a zswap
> > > specific patch.
> > >
> > > Does this sound good to everyone? If there are any objections, please
> > > let me know.
> >
> > If (c) is holding up for writeback and vmscan, I would question a more
> > radical approach of tearing apart the rstat framework and using it
> > across the board. That is obviously a heavier lift and more
> > controversial,
>
> Yes controversial because we used to have similar mechanism which did not work
> and we had to move to rstat. Check the commit 42a300353577 and fixes to it.

IIUC, in that commit, the update side modifies one per-CPU per-cgroup
counters, and when those counters exceed a threshold atomic updates
are performed on all parents, which I think is the slow side. Then,
the read side just reads the potentially stale atomic. That was
problematic because:
(1) The update side can end up performing atomic updates on all parents.
(2) The read side can be inaccurate by up to MEMCG_CHARGE_BATCH *
nr_cpus * nr_children

What I am proposing in (c) is different:

(1) The update side always updates per-CPU per-cgroup counters in all
the parents. It never does atomic operations. The parent iteration is
not cheap, but it is cheaper than atomics for sure and we already do
parent iteration today in some cases in memcg_rstat_updated(). So the
"slow" path should be the same as today.
(2) The read side always iterates per-CPU counters in this cgroup, so
it's always accurate. This might be more expensive on an rstat flush
on average (e.g. if memcg_vmstats_needs_flush() skips the flush), but
it is much more consistent and the tail latency is much better.

>
> This worked for zswap stats because their update and consumption are not on
> performance critical code paths i.e. these are on the way to compress or
> decompress or in reclaim context.

Well, decompression is in the fault path, that's performance critical
to some extent (although not like other stats updated by networking).
Anyway, I think the update path shouldn't regress much with the
approach described in (c).

>
> > but if we can get away with it, I think it will
> > simplify things greatly and honestly rstat has been causing a lot of
> > trouble in the last few years.
> >
> > But this can be done incrementally too, we can start by separating out
> > the problematic stats to use the new update/flushing scheme,
>
> This I think we do need but for specific stats. I think the stats which have
> in-kernel consumers need this. However not all such stats might be fine with
> slow update side like zswap. So, we need to evaluate thoroughly.

I think in the proposed approach (c) the main concern is actually read
latency, not update latency. So if it works for in-kernel consumers it
should definitely work for userspace consumers as well?

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

[Shakeel Butt]

On Wed, Aug 26, 2026 at 02:55:57PM -0700, Yosry Ahmed wrote:
> > >
[...]
> > > If (c) is holding up for writeback and vmscan, I would question a more
> > > radical approach of tearing apart the rstat framework and using it
> > > across the board. That is obviously a heavier lift and more
> > > controversial,
> >
> > Yes controversial because we used to have similar mechanism which did not work
> > and we had to move to rstat. Check the commit 42a300353577 and fixes to it.
> 
> IIUC, in that commit, the update side modifies one per-CPU per-cgroup
> counters, and when those counters exceed a threshold atomic updates
> are performed on all parents, which I think is the slow side. Then,
> the read side just reads the potentially stale atomic. That was
> problematic because:
> (1) The update side can end up performing atomic updates on all parents.
> (2) The read side can be inaccurate by up to MEMCG_CHARGE_BATCH *
> nr_cpus * nr_children
> 
> What I am proposing in (c) is different:
> 
> (1) The update side always updates per-CPU per-cgroup counters in all
> the parents. It never does atomic operations. The parent iteration is
> not cheap, but it is cheaper than atomics for sure and we already do
> parent iteration today in some cases in memcg_rstat_updated().So the
> "slow" path should be the same as today.

Yes but that parent traversal is short circuited when we hit the flush
threshold. At Meta scale, we have observed significant cpus being spent on
memcg_rstat_updated before that.

> (2) The read side always iterates per-CPU counters in this cgroup, so
> it's always accurate. This might be more expensive on an rstat flush
> on average (e.g. if memcg_vmstats_needs_flush() skips the flush), but
> it is much more consistent and the tail latency is much better.
> 

It might be or not. Usually such things are more clear at scale in production
instead of benchmarks.

> >
> > This worked for zswap stats because their update and consumption are not on
> > performance critical code paths i.e. these are on the way to compress or
> > decompress or in reclaim context.
> 
> Well, decompression is in the fault path, that's performance critical
> to some extent (although not like other stats updated by networking).
> Anyway, I think the update path shouldn't regress much with the
> approach described in (c).

Decompression is on the scale of micro-seconds (and I suspect the simple minor
page faults are on nano-seconds scale) and the upward traversal is definitely
much cheaper and will be a noise there.

> 
> >
> > > but if we can get away with it, I think it will
> > > simplify things greatly and honestly rstat has been causing a lot of
> > > trouble in the last few years.
> > >
> > > But this can be done incrementally too, we can start by separating out
> > > the problematic stats to use the new update/flushing scheme,
> >
> > This I think we do need but for specific stats. I think the stats which have
> > in-kernel consumers need this. However not all such stats might be fine with
> > slow update side like zswap. So, we need to evaluate thoroughly.
> 
> I think in the proposed approach (c) the main concern is actually read
> latency, not update latency. So if it works for in-kernel consumers it
> should definitely work for userspace consumers as well?

Here I was mainly talking about consumers which has more strict staleness
requirements and those are mainly kernel consumers. Rstat flushing is definitely
more expensive than this for-each-cpu traversal and most of the time they just
need it for one or very small set of stats.

I am imagining once we have this special mechanism for selected stats, we can
potentially remove ratelimited flush and stats update threshold code.

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

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 4:08 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> On Wed, Aug 26, 2026 at 02:55:57PM -0700, Yosry Ahmed wrote:
> > > >
> [...]
> > > > If (c) is holding up for writeback and vmscan, I would question a more
> > > > radical approach of tearing apart the rstat framework and using it
> > > > across the board. That is obviously a heavier lift and more
> > > > controversial,
> > >
> > > Yes controversial because we used to have similar mechanism which did not work
> > > and we had to move to rstat. Check the commit 42a300353577 and fixes to it.
> >
> > IIUC, in that commit, the update side modifies one per-CPU per-cgroup
> > counters, and when those counters exceed a threshold atomic updates
> > are performed on all parents, which I think is the slow side. Then,
> > the read side just reads the potentially stale atomic. That was
> > problematic because:
> > (1) The update side can end up performing atomic updates on all parents.
> > (2) The read side can be inaccurate by up to MEMCG_CHARGE_BATCH *
> > nr_cpus * nr_children
> >
> > What I am proposing in (c) is different:
> >
> > (1) The update side always updates per-CPU per-cgroup counters in all
> > the parents. It never does atomic operations. The parent iteration is
> > not cheap, but it is cheaper than atomics for sure and we already do
> > parent iteration today in some cases in memcg_rstat_updated().So the
> > "slow" path should be the same as today.
>
> Yes but that parent traversal is short circuited when we hit the flush
> threshold. At Meta scale, we have observed significant cpus being spent on
> memcg_rstat_updated before that.

Interesting. So the upward per-CPU traversal takes time? Perhaps due
to concurrent flushers causing the cachelines to bounce?

>
> > (2) The read side always iterates per-CPU counters in this cgroup, so
> > it's always accurate. This might be more expensive on an rstat flush
> > on average (e.g. if memcg_vmstats_needs_flush() skips the flush), but
> > it is much more consistent and the tail latency is much better.
> >
>
> It might be or not. Usually such things are more clear at scale in production
> instead of benchmarks.

Agreed.

>
> > >
> > > This worked for zswap stats because their update and consumption are not on
> > > performance critical code paths i.e. these are on the way to compress or
> > > decompress or in reclaim context.
> >
> > Well, decompression is in the fault path, that's performance critical
> > to some extent (although not like other stats updated by networking).
> > Anyway, I think the update path shouldn't regress much with the
> > approach described in (c).
>
> Decompression is on the scale of micro-seconds (and I suspect the simple minor
> page faults are on nano-seconds scale) and the upward traversal is definitely
> much cheaper and will be a noise there.
>
> >
> > >
> > > > but if we can get away with it, I think it will
> > > > simplify things greatly and honestly rstat has been causing a lot of
> > > > trouble in the last few years.
> > > >
> > > > But this can be done incrementally too, we can start by separating out
> > > > the problematic stats to use the new update/flushing scheme,
> > >
> > > This I think we do need but for specific stats. I think the stats which have
> > > in-kernel consumers need this. However not all such stats might be fine with
> > > slow update side like zswap. So, we need to evaluate thoroughly.
> >
> > I think in the proposed approach (c) the main concern is actually read
> > latency, not update latency. So if it works for in-kernel consumers it
> > should definitely work for userspace consumers as well?
>
> Here I was mainly talking about consumers which has more strict staleness
> requirements and those are mainly kernel consumers. Rstat flushing is definitely
> more expensive than this for-each-cpu traversal and most of the time they just
> need it for one or very small set of stats.
>
> I am imagining once we have this special mechanism for selected stats, we can
> potentially remove ratelimited flush and stats update threshold code.

Yeah that's the hope. I was basically saying that I am hoping for
more, to transition everything to this flushing scheme if it works
well.

Anyway, as you mentioned, ideally we need production data to proceed.
Joanne, is this something that you are trying to collect by any
chance? :)