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@xxxxxxxxx> wrote:
>
> On Thu, 20 Aug 2026 15:18:39 -0700 Joanne Koong <joannelkoong@xxxxxxxxx> wrote:
>
> > On Tue, Aug 18, 2026 at 11:35 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
> > >
> > > On Mon, Aug 17, 2026 at 6:53 PM Song Hu <husong@xxxxxxxxxx> 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@xxxxxxxxxx/T/#m5ed6ff8e64eeb909ff3c92bde5105406b1ddaf10