memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

Hi Yosry,

On Wed, Jul 22, 2026 at 1:38 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> > > > First let's see if the general solution is doable and simple otherwise we can
> > > > explore use-case specific solutions. In extreme case revert is an options as
> > > > well but I would prefer a more sophisticated ratelimiting (that Jan suggested)
> > > > than a revert.
> > > >
> > >
> > > Sounds good. I'll keep an eye out for the work Yosry's doing on this
> > > and look into hooking it up to writeback if/when his general solution
> > > becomes available.
> >
> > I am not sure if I had in mind would generalize well. For zswap, I was
> > just going to replace rstat with per-memcg atomic counters, and simply
> > walk the cgroup parents in the update path, as the zswap load/store path
> > shouldn't be too hot.
> >
> > However, looking at other in-kernel flushers (including
> > mem_cgroup_wb_stats()), I see they consume other stats like
> > NR_INACTIVE_FILE/NR_ACTIVE_FILE, which are updated in the page
> > allocation path. I am not sure if a cgroup parent walk with atomic
> > updates would fly there.
> >
> > A more problematic one is count_shadow_nodes(), which consumes slab
> > object stats. I think we definitely cannot do atomic updates in the slab
> > allocation path. Although it seems like count_shadow_nodes() is a rough
> > estimate and perhaps we can forgoe using the stats there.
> >
> > If we want a generic solution for in-kernel flushers to improve stats
> > accuracy without killing performance, I think we need a heavier lift to
> > rework rstat or move away from it completely.
>
> (Adding a few more people and mailing lists)
>
> So I experimented with a per-memcg atomics in zswap (see [1] for
> context), and it appears to be too expensive even for updating the
> zswap stats during reclaim / swap faults. So I think zswap will need a
> similar approach to writeback and other in-kernel users.

Thanks for running the experiment.

In a previous thread [1], you mentioned your team was investigating
some premature OOM kills that might be attributed to this same stale
stats issue in the vmscan path. Did that turn out to be the case?
Asking because if so, that might be something we'd hit at Meta too.

>
> One idea is to keep per-CPU per-cgroup counters, but on the update
> side we update all the parents' counters, instead of just updating the
> current cgroup's counters. Flushing the stats then only needs to
> iterate the per-CPU counters and doesn't need to walk the cgroup tree
> at all. There is added work on the update side, but hopefully it's
> cheap enough to update per-CPU counters, and cgroup hierarchies aren't
> usually too deep.

I'm not sure about the zswap case, but for the writeback case there
are significantly more updates than reads. There are four stat updates
for every folio that gets dirtied / written back, so four per page
with 4k folios, while the read happens around every few hundred pages
dirtied. In the scenario I was testing with memory.max set to 8 GB and
using the default 20% dirty ratio, during freerun the stat gets read
once every 256 to 512 pages (with flushes happening less frequently
than reads).

I'm not too familiar with memcg or vmstat so apologies if this is a
naive question, but does it make sense for memcg to just do a similar
approach to what vmstat does? As I understand it, mod_lruvec_state()
updates the node counter and the memcg counter (if memcg applies),
where for updating the node counter, mod_node_state() does some
batching where it updates the global counter for that stat only once
it crosses some threshold (pcp->stat_threshold). Could memcg keep its
per-CPU counter as the accumulator but once it reaches some threshold,
it then propagates it up the parent/ancestors into a per-memcg atomic
global counter? The atomic would then only have to be touched once per
batch than on every update, which might eliminate the overhead of the
per-memcg atomic you saw previously? It seems like this would get rid
of needing to do any flushing altogether, as we could just read that
global counter directly.

>
> A more radical approach is to completely drop the per-CPU rstat trees.
> So instead of walking the parents to add them to the per-CPU tree, we
> just walk the parents and directly update the stat. The flush side
> will then always iterate the per-CPU counters to accumulate them. With
> this approach, the update cost roughly remains the same, but the flush
> cost changes. Instead of walking the trees and flushing all cgroups,
> but only doing it on CPUs/cgroups that actually have updates, we
> always iterate and sum the per-CPU counters. The average flush cost
> may become more expensive, but it should be more consistent and won't
> scale with the cgroup tree size. I am not sure if this would be a net
> win or not.

I wonder if this scales for machines that have a significant number of
cores? Onsomething like the 96-core / 192-thread machine Yunzhao
mentioned in the zswap case, it seems like the flush now would have to
touch multiple additional cachelines per CPU when it seems like for
the majority of the stats, only a small fraction of those hardware
threads would have any updates on them?

>
> We can also choose to do the above for all the stats or only the ones
> used by in-kernel flushers, the latter would probably perform better
> but at the cost of more complexity.

I agree. For the writeback case, afaict I think we really only need to
do this for NR_FILE_DIRTY and NR_WRITEBACK, neither of which are
updated from the page allocation path.

>
> I honestly don't have any time to pursue any of these approaches, so
> this is mostly just a brain dump. But I do think we should try to more
> systematically solve the stats flushing problem as it has given us a
> lot of pain over the years.
>

Thanks for writing up your thoughts on this - I found it helpful!

Thanks,
Joanne

[1] https://lore.kernel.org/linux-mm/CAO9r8zOiwgG80EjqdZFgWQvODUe1RzecLemxcB7xXNm_f3XfbA@mail.gmail.com/

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Thu, Aug 6, 2026 at 2:04 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
>

Sorry, I think I messed up how things got linked when trying to update
the thread title in gmail. This is a continuation of the previous
discussion thread ("PATCH] fuse: disable default bdi strictlimiting")
which is at https://lore.kernel.org/linux-fsdevel/s4kv7qep3jlncs67q72xfhserpd66hpbwzkoppiddc5raufyjd@ksuvlfqcrbk2/T/#t.

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

On Thu, Aug 6, 2026 at 2:04 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> Hi Yosry,
>
> On Wed, Jul 22, 2026 at 1:38 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > > > > First let's see if the general solution is doable and simple otherwise we can
> > > > > explore use-case specific solutions. In extreme case revert is an options as
> > > > > well but I would prefer a more sophisticated ratelimiting (that Jan suggested)
> > > > > than a revert.
> > > > >
> > > >
> > > > Sounds good. I'll keep an eye out for the work Yosry's doing on this
> > > > and look into hooking it up to writeback if/when his general solution
> > > > becomes available.
> > >
> > > I am not sure if I had in mind would generalize well. For zswap, I was
> > > just going to replace rstat with per-memcg atomic counters, and simply
> > > walk the cgroup parents in the update path, as the zswap load/store path
> > > shouldn't be too hot.
> > >
> > > However, looking at other in-kernel flushers (including
> > > mem_cgroup_wb_stats()), I see they consume other stats like
> > > NR_INACTIVE_FILE/NR_ACTIVE_FILE, which are updated in the page
> > > allocation path. I am not sure if a cgroup parent walk with atomic
> > > updates would fly there.
> > >
> > > A more problematic one is count_shadow_nodes(), which consumes slab
> > > object stats. I think we definitely cannot do atomic updates in the slab
> > > allocation path. Although it seems like count_shadow_nodes() is a rough
> > > estimate and perhaps we can forgoe using the stats there.
> > >
> > > If we want a generic solution for in-kernel flushers to improve stats
> > > accuracy without killing performance, I think we need a heavier lift to
> > > rework rstat or move away from it completely.
> >
> > (Adding a few more people and mailing lists)
> >
> > So I experimented with a per-memcg atomics in zswap (see [1] for
> > context), and it appears to be too expensive even for updating the
> > zswap stats during reclaim / swap faults. So I think zswap will need a
> > similar approach to writeback and other in-kernel users.
>
> Thanks for running the experiment.
>
> In a previous thread [1], you mentioned your team was investigating
> some premature OOM kills that might be attributed to this same stale
> stats issue in the vmscan path. Did that turn out to be the case?
> Asking because if so, that might be something we'd hit at Meta too.

I don't think we had time to look into this closely. We have a test
that runs VMs with some memory pressure / reclaim, and we observed OOM
kills in that test after the ratelimited flush in the vmscan path.
Whether or not this translates to additional OOM kills in our prod is
currently unknown, but it's a signal that reclaim effectiveness may be
negatively affected in some cases.

> >
> > One idea is to keep per-CPU per-cgroup counters, but on the update
> > side we update all the parents' counters, instead of just updating the
> > current cgroup's counters. Flushing the stats then only needs to
> > iterate the per-CPU counters and doesn't need to walk the cgroup tree
> > at all. There is added work on the update side, but hopefully it's
> > cheap enough to update per-CPU counters, and cgroup hierarchies aren't
> > usually too deep.
>
> I'm not sure about the zswap case, but for the writeback case there
> are significantly more updates than reads. There are four stat updates
> for every folio that gets dirtied / written back, so four per page
> with 4k folios, while the read happens around every few hundred pages
> dirtied. In the scenario I was testing with memory.max set to 8 GB and
> using the default 20% dirty ratio, during freerun the stat gets read
> once every 256 to 512 pages (with flushes happening less frequently
> than reads).

Yeah I think we generally want to keep updates faster than reads,
because some of them are also in latency-sensitive paths like
networking. However, since reads are becoming too slow in some cases,
the question here is whether we can afford some extra cost on the
update side (to iterate parent per-CPU counters) to make the read path
more manageable.

> I'm not too familiar with memcg or vmstat so apologies if this is a
> naive question, but does it make sense for memcg to just do a similar
> approach to what vmstat does? As I understand it, mod_lruvec_state()
> updates the node counter and the memcg counter (if memcg applies),
> where for updating the node counter, mod_node_state() does some
> batching where it updates the global counter for that stat only once
> it crosses some threshold (pcp->stat_threshold). Could memcg keep its
> per-CPU counter as the accumulator but once it reaches some threshold,
> it then propagates it up the parent/ancestors into a per-memcg atomic
> global counter? The atomic would then only have to be touched once per
> batch than on every update, which might eliminate the overhead of the
> per-memcg atomic you saw previously? It seems like this would get rid
> of needing to do any flushing altogether, as we could just read that
> global counter directly.

The memcg stats perform similar per-CPU counting and thresholding. The
main difference from vmstat AFAICT is the cgroup hierarchy. In vmstat
we have per-CPU counters and a global atomic counter for each stat.
For memcg, we have the same for each cgroup, then we also have a
cgroup tree. When we read the stats of a cgroup, we usually want the
hierarchical stats including all of its children, so we can't just
read the global counter.

We use rstat, a cgroup framework that keeps track of which cgroups
have updates on which CPUs, and then memcg has thresholding logic on
top to only flush if the number of pending updates exceeds a
threshold. Essentially, if the magnitude updates on a CPU exceeds
MEMCG_CHARGE_BATCH, we add it to a global per-memcg counter. We only
flush if that global per-memcg counter has > NR_CPUS *
MEMCG_CHARGE_BATCH updates. So in theory I think we tolerate up to 2 *
NR_CPUS * MEMCG_CHARGE_BATCH of stale stats (combined for all the
stats).

See memcg_rstat_updated() and memcg_vmstats_needs_flush() for this logic.

Looking at this code again, it made me realize that we already iterate
the per-CPU stats_updates on every update to increment the number of
pending stats on this CPU. I wonder if we can replace this with
actually updating the stat on each parent, then the flush path only
needs to accumulate CPU counters. Or maybe as you mention, we update
the global per-memcg counter when updates on a CPU exceed a threshold,
and forgo the rstat flushing logic completely.

> > A more radical approach is to completely drop the per-CPU rstat trees.
> > So instead of walking the parents to add them to the per-CPU tree, we
> > just walk the parents and directly update the stat. The flush side
> > will then always iterate the per-CPU counters to accumulate them. With
> > this approach, the update cost roughly remains the same, but the flush
> > cost changes. Instead of walking the trees and flushing all cgroups,
> > but only doing it on CPUs/cgroups that actually have updates, we
> > always iterate and sum the per-CPU counters. The average flush cost
> > may become more expensive, but it should be more consistent and won't
> > scale with the cgroup tree size. I am not sure if this would be a net
> > win or not.
>
> I wonder if this scales for machines that have a significant number of
> cores? Onsomething like the 96-core / 192-thread machine Yunzhao
> mentioned in the zswap case, it seems like the flush now would have to
> touch multiple additional cachelines per CPU when it seems like for
> the majority of the stats, only a small fraction of those hardware
> threads would have any updates on them?

Probably not (hence I called it radical), but I think this can be
side-stepped by either keeping the tracking of CPUs that have updates
via rstat (or similar), or just propagating the per-CPU stat updates
when they reach a threshold like vmstat.

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Thu, Aug 6, 2026 at 2:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Thu, Aug 6, 2026 at 2:04 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > I'm not too familiar with memcg or vmstat so apologies if this is a
> > naive question, but does it make sense for memcg to just do a similar
> > approach to what vmstat does? As I understand it, mod_lruvec_state()
> > updates the node counter and the memcg counter (if memcg applies),
> > where for updating the node counter, mod_node_state() does some
> > batching where it updates the global counter for that stat only once
> > it crosses some threshold (pcp->stat_threshold). Could memcg keep its
> > per-CPU counter as the accumulator but once it reaches some threshold,
> > it then propagates it up the parent/ancestors into a per-memcg atomic
> > global counter? The atomic would then only have to be touched once per
> > batch than on every update, which might eliminate the overhead of the
> > per-memcg atomic you saw previously? It seems like this would get rid
> > of needing to do any flushing altogether, as we could just read that
> > global counter directly.
>
> The memcg stats perform similar per-CPU counting and thresholding. The
> main difference from vmstat AFAICT is the cgroup hierarchy. In vmstat
> we have per-CPU counters and a global atomic counter for each stat.
> For memcg, we have the same for each cgroup, then we also have a
> cgroup tree. When we read the stats of a cgroup, we usually want the
> hierarchical stats including all of its children, so we can't just
> read the global counter.
>
> We use rstat, a cgroup framework that keeps track of which cgroups
> have updates on which CPUs, and then memcg has thresholding logic on
> top to only flush if the number of pending updates exceeds a
> threshold. Essentially, if the magnitude updates on a CPU exceeds
> MEMCG_CHARGE_BATCH, we add it to a global per-memcg counter. We only
> flush if that global per-memcg counter has > NR_CPUS *
> MEMCG_CHARGE_BATCH updates. So in theory I think we tolerate up to 2 *
> NR_CPUS * MEMCG_CHARGE_BATCH of stale stats (combined for all the
> stats).
>
> See memcg_rstat_updated() and memcg_vmstats_needs_flush() for this logic.
>
> Looking at this code again, it made me realize that we already iterate
> the per-CPU stats_updates on every update to increment the number of
> pending stats on this CPU. I wonder if we can replace this with
> actually updating the stat on each parent, then the flush path only
> needs to accumulate CPU counters. Or maybe as you mention, we update
> the global per-memcg counter when updates on a CPU exceed a threshold,
> and forgo the rstat flushing logic completely.
>

I can run some experiments and see if updating the global per-memcg
counter per batch and forgoing flushing could be a viable solution. I
can use my setup to test it for writeback, but for the zswap use case,
what's the best way to test if the changes are too expensive? Is there
a benchmark program you're running?

For updating the stat on each parent and having the flush path only
accumulate CPU counters, I think this means we would have to
unconditionally walk the ancestors + update its counters on every
update, as we wouldn't be able to early break if the flushable
threshold was already exceeded. I don't think we could replace the
stats_update update, as it seems like readers still need some way of
knowing when to flush. It seems like this could be a nontrivial
performance hit, but maybe it's a non-factor in reality. If you think
this would be useful to get benchmark numbers on, I can run some
experiments on this too.

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

On Mon, Aug 10, 2026 at 09:49:31AM -0700, Joanne Koong wrote:
> On Thu, Aug 6, 2026 at 2:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Thu, Aug 6, 2026 at 2:04 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >
> > > I'm not too familiar with memcg or vmstat so apologies if this is a
> > > naive question, but does it make sense for memcg to just do a similar
> > > approach to what vmstat does? As I understand it, mod_lruvec_state()
> > > updates the node counter and the memcg counter (if memcg applies),
> > > where for updating the node counter, mod_node_state() does some
> > > batching where it updates the global counter for that stat only once
> > > it crosses some threshold (pcp->stat_threshold). Could memcg keep its
> > > per-CPU counter as the accumulator but once it reaches some threshold,
> > > it then propagates it up the parent/ancestors into a per-memcg atomic
> > > global counter? The atomic would then only have to be touched once per
> > > batch than on every update, which might eliminate the overhead of the
> > > per-memcg atomic you saw previously? It seems like this would get rid
> > > of needing to do any flushing altogether, as we could just read that
> > > global counter directly.
> >
> > The memcg stats perform similar per-CPU counting and thresholding. The
> > main difference from vmstat AFAICT is the cgroup hierarchy. In vmstat
> > we have per-CPU counters and a global atomic counter for each stat.
> > For memcg, we have the same for each cgroup, then we also have a
> > cgroup tree. When we read the stats of a cgroup, we usually want the
> > hierarchical stats including all of its children, so we can't just
> > read the global counter.
> >
> > We use rstat, a cgroup framework that keeps track of which cgroups
> > have updates on which CPUs, and then memcg has thresholding logic on
> > top to only flush if the number of pending updates exceeds a
> > threshold. Essentially, if the magnitude updates on a CPU exceeds
> > MEMCG_CHARGE_BATCH, we add it to a global per-memcg counter. We only
> > flush if that global per-memcg counter has > NR_CPUS *
> > MEMCG_CHARGE_BATCH updates. So in theory I think we tolerate up to 2 *
> > NR_CPUS * MEMCG_CHARGE_BATCH of stale stats (combined for all the
> > stats).
> >
> > See memcg_rstat_updated() and memcg_vmstats_needs_flush() for this logic.
> >
> > Looking at this code again, it made me realize that we already iterate
> > the per-CPU stats_updates on every update to increment the number of
> > pending stats on this CPU. I wonder if we can replace this with
> > actually updating the stat on each parent, then the flush path only
> > needs to accumulate CPU counters. Or maybe as you mention, we update
> > the global per-memcg counter when updates on a CPU exceed a threshold,
> > and forgo the rstat flushing logic completely.
> >
> 
> I can run some experiments and see if updating the global per-memcg
> counter per batch and forgoing flushing could be a viable solution.
> I can use my setup to test it for writeback, but for the zswap use
> case, what's the best way to test if the changes are too expensive? Is
> there a benchmark program you're running?

I was running stress-ng and measuring the latency of zswap_load() and
zswap_store() with bpftrace. But I suspect whatever works for writeback
should work for zswap :)

> 
> For updating the stat on each parent and having the flush path only
> accumulate CPU counters, I think this means we would have to
> unconditionally walk the ancestors + update its counters on every
> update, as we wouldn't be able to early break if the flushable
> threshold was already exceeded. I don't think we could replace the
> stats_update update, as it seems like readers still need some way of
> knowing when to flush. It seems like this could be a nontrivial
> performance hit, but maybe it's a non-factor in reality. If you think
> this would be useful to get benchmark numbers on, I can run some
> experiments on this too.

I think we need to experiment with how much work we can shift around
between the update and reader sides.

Right now, on the update side, we call __css_rstat_updated(), which
iterates the parents and puts them on the per-CPU rstat tree, then we
iterate the parents again in memcg_rstat_updated() and:
- Check if flushing threshold is already met.
- Update per-CPU counter for stats_updates.
- If per-CPU update exceeds (another) threshold, we update an atomic for
  stats_updates.

The flush side checks the flushing threshold and does an rstat flush,
which will walk the update trees on all CPUs and iterates all child
cgroups on it. Pretty expensive when the threshold is met.

I think we discussed two different options:

(a) On stats updates, iterate the parents and update the per-CPU
counters of the stat directly.

In this case, we can't keep using rstat, as we would still need to add
the cgroup and all its ancestors to the update tree anyway and the flush
cost will remain similar. So on the flush path we'll need to iterate all
CPUs and add the counters. We can explore if use a cpumask can help
here, but perhaps not since we'll need to update it atomically on all
stat updates.

I imagine that if we do this we'll drop the stats_updates optimization
to avoid adding more cost to the update side. We'll be replacing parent
stats_updates modification with parent stat counter modification. But
maybe it's feasible to keep both to limit the cost of flusing.

This only works if the cost of iterate the CPUs on the flush side ends
up being cheaper than the current flush. I imagine the average cost will
be higher but the cost should be much more consistent vs the current
flush, so the tail should be better.

(b) On stats updates, iterate the parents and update the per-CPU
counters of the stat directly AND update a global atomic if the per-CPU
counter exceeds a threshold. I think updates may get too expensive as we
may need to update multiple atomics, especially when updating the root
counter as it will be contended by all cgroups.

The flush side becomes an atomic read, so should be very cheap, and we
can drop other heuristics and optimizations.

---

I think ultimately we need to experiment with these approachs (and
perhaps others) to check what actually works in practice. You'll
probably want some benchmarks or synthetic tests with many cgroups to
exercise the worst case scenarios.

For the zswap stats, I used to run stress-ng with a bunch of workers
(20, 50, 100, ..) in cgroups of different levels. Since all workers are
in the same cgroups, all the parents are common and all threads end up
competing on the same atomics, so worst case scenario. I was also
testing in a VM that has more CPUs than stress-ng workers, to exercise
the full extent of the concurrency and contention.

This could be a lot of work, so I am not really asking you to do it, but
just brainstorming and sharing ideas :)

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Mon, Aug 10, 2026 at 5:48 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Mon, Aug 10, 2026 at 09:49:31AM -0700, Joanne Koong wrote:
> > On Thu, Aug 6, 2026 at 2:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
> > > On Thu, Aug 6, 2026 at 2:04 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > > >
> > > > I'm not too familiar with memcg or vmstat so apologies if this is a
> > > > naive question, but does it make sense for memcg to just do a similar
> > > > approach to what vmstat does? As I understand it, mod_lruvec_state()
> > > > updates the node counter and the memcg counter (if memcg applies),
> > > > where for updating the node counter, mod_node_state() does some
> > > > batching where it updates the global counter for that stat only once
> > > > it crosses some threshold (pcp->stat_threshold). Could memcg keep its
> > > > per-CPU counter as the accumulator but once it reaches some threshold,
> > > > it then propagates it up the parent/ancestors into a per-memcg atomic
> > > > global counter? The atomic would then only have to be touched once per
> > > > batch than on every update, which might eliminate the overhead of the
> > > > per-memcg atomic you saw previously? It seems like this would get rid
> > > > of needing to do any flushing altogether, as we could just read that
> > > > global counter directly.
> > >
> > > The memcg stats perform similar per-CPU counting and thresholding. The
> > > main difference from vmstat AFAICT is the cgroup hierarchy. In vmstat
> > > we have per-CPU counters and a global atomic counter for each stat.
> > > For memcg, we have the same for each cgroup, then we also have a
> > > cgroup tree. When we read the stats of a cgroup, we usually want the
> > > hierarchical stats including all of its children, so we can't just
> > > read the global counter.
> > >
> > > We use rstat, a cgroup framework that keeps track of which cgroups
> > > have updates on which CPUs, and then memcg has thresholding logic on
> > > top to only flush if the number of pending updates exceeds a
> > > threshold. Essentially, if the magnitude updates on a CPU exceeds
> > > MEMCG_CHARGE_BATCH, we add it to a global per-memcg counter. We only
> > > flush if that global per-memcg counter has > NR_CPUS *
> > > MEMCG_CHARGE_BATCH updates. So in theory I think we tolerate up to 2 *
> > > NR_CPUS * MEMCG_CHARGE_BATCH of stale stats (combined for all the
> > > stats).
> > >
> > > See memcg_rstat_updated() and memcg_vmstats_needs_flush() for this logic.
> > >
> > > Looking at this code again, it made me realize that we already iterate
> > > the per-CPU stats_updates on every update to increment the number of
> > > pending stats on this CPU. I wonder if we can replace this with
> > > actually updating the stat on each parent, then the flush path only
> > > needs to accumulate CPU counters. Or maybe as you mention, we update
> > > the global per-memcg counter when updates on a CPU exceed a threshold,
> > > and forgo the rstat flushing logic completely.
> > >
> >
> > I can run some experiments and see if updating the global per-memcg
> > counter per batch and forgoing flushing could be a viable solution.
> > I can use my setup to test it for writeback, but for the zswap use
> > case, what's the best way to test if the changes are too expensive? Is
> > there a benchmark program you're running?
>
> I was running stress-ng and measuring the latency of zswap_load() and
> zswap_store() with bpftrace. But I suspect whatever works for writeback
> should work for zswap :)
>
> >
> > For updating the stat on each parent and having the flush path only
> > accumulate CPU counters, I think this means we would have to
> > unconditionally walk the ancestors + update its counters on every
> > update, as we wouldn't be able to early break if the flushable
> > threshold was already exceeded. I don't think we could replace the
> > stats_update update, as it seems like readers still need some way of
> > knowing when to flush. It seems like this could be a nontrivial
> > performance hit, but maybe it's a non-factor in reality. If you think
> > this would be useful to get benchmark numbers on, I can run some
> > experiments on this too.
>
> I think we need to experiment with how much work we can shift around
> between the update and reader sides.
>
> Right now, on the update side, we call __css_rstat_updated(), which
> iterates the parents and puts them on the per-CPU rstat tree, then we
> iterate the parents again in memcg_rstat_updated() and:
> - Check if flushing threshold is already met.
> - Update per-CPU counter for stats_updates.
> - If per-CPU update exceeds (another) threshold, we update an atomic for
>   stats_updates.
>
> The flush side checks the flushing threshold and does an rstat flush,
> which will walk the update trees on all CPUs and iterates all child
> cgroups on it. Pretty expensive when the threshold is met.
>
> I think we discussed two different options:
>
> (a) On stats updates, iterate the parents and update the per-CPU
> counters of the stat directly.
>
> In this case, we can't keep using rstat, as we would still need to add
> the cgroup and all its ancestors to the update tree anyway and the flush
> cost will remain similar. So on the flush path we'll need to iterate all
> CPUs and add the counters. We can explore if use a cpumask can help
> here, but perhaps not since we'll need to update it atomically on all
> stat updates.
>
> I imagine that if we do this we'll drop the stats_updates optimization
> to avoid adding more cost to the update side. We'll be replacing parent
> stats_updates modification with parent stat counter modification. But
> maybe it's feasible to keep both to limit the cost of flusing.
>
> This only works if the cost of iterate the CPUs on the flush side ends
> up being cheaper than the current flush. I imagine the average cost will
> be higher but the cost should be much more consistent vs the current
> flush, so the tail should be better.
>
> (b) On stats updates, iterate the parents and update the per-CPU
> counters of the stat directly AND update a global atomic if the per-CPU
> counter exceeds a threshold. I think updates may get too expensive as we
> may need to update multiple atomics, especially when updating the root
> counter as it will be contended by all cgroups.
>
> The flush side becomes an atomic read, so should be very cheap, and we
> can drop other heuristics and optimizations.
>
> ---
>
> I think ultimately we need to experiment with these approachs (and
> perhaps others) to check what actually works in practice. You'll
> probably want some benchmarks or synthetic tests with many cgroups to
> exercise the worst case scenarios.
>
> For the zswap stats, I used to run stress-ng with a bunch of workers
> (20, 50, 100, ..) in cgroups of different levels. Since all workers are
> in the same cgroups, all the parents are common and all threads end up
> competing on the same atomics, so worst case scenario. I was also
> testing in a VM that has more CPUs than stress-ng workers, to exercise
> the full extent of the concurrency and contention.
>
> This could be a lot of work, so I am not really asking you to do it, but
> just brainstorming and sharing ideas :)

Thanks for your guidance and insights on this, Yosry! Your responses
have been very helpful.

I'll do some experimentation with this and try to get a sense of what
the update costs are.

One thing I was unclear about and have been looking at is how
pervasive/important this issue is outside of the writeback case. It
looks like there are roughly 4 other categories of users that rely on
stats that can be 2s stale: zswap, cachestat(), working set, and
vmscan.

Of these, vmscan is the only one that looks troublesome. Some armchair
analysis below:

a) zswap (zswap_shrinker_count()): 2s of staleness looks fine. the
stats are only used to calculate the compression ratio and the ratio
scales a count that comes from a list_lru and *isn't* stale. The ratio
is an average over the whole pool, so it seems like that should be
fairly steady across 2s. A stale result doesn't seem too bad - afaict,
it is a heuristic of how zswap should get weighed against the other
shrinkers.

b) cachestat() syscall (filemap_cachestat()): seems preferable to just
rely on the 2s flusher worker. The results are returned back to
userspace and not used for any internal kernel decision-making, seems
ok to have one field in it (nr_recently_evicted) potentially be stale.
Seems better to err on the side of less frequent flushing anyways, as
the call path is invoked from unprivileged userspace and flushes the
root memcg.

c) workingset: looking at the original commits, the calculations are
estimates that don't require exact stats -

refaulting (workingset_test_recent()): commit 9b3016154c91 ("memcg:
sync flush only if periodic flush is delayed")
    Now the question: what are the side-effects of this change? The worst
    that can happen is the refault codepath will see 4sec old lruvec stats
    and may cause false (or missed) activations of the refaulted page which
    may under-or-overestimate the workingset size.  Though that is not very
    concerning as the kernel can already miss or do false activations.

shadow-node shrinker (count_shadow_nodes()): commit d4a5b369ad6d ("mm:
ratelimit stat flush from workingset shrinker")
    On further inspection it seems like we don't really need
    accurate stats in this function as it was already approximating the amount
    of appropriate shadow entries to keep for maintaining the refault
    information.

d) vmscan (prepare_scan_control()): for determining whether reclaim
can deactivate, there's this logic

                /*
                 * When refaults are being observed, it means a new
                 * workingset is being established. Deactivate to get
                 * rid of any stale active pages quickly.
                 */
                refaults = lruvec_page_state(target_lruvec,
                                WORKINGSET_ACTIVATE_ANON);
                if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
                        inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
                        sc->may_deactivate |= DEACTIVATE_ANON;
                else
                        sc->may_deactivate &= ~DEACTIVATE_ANON;

(and same for DEACTIVATE_FILE for file-backed folios)

target_lruvec->refaults[] gets set at the end of the reclaim pass. It
stores the same lruvec_page_state() value that prepare_scan_control()
will read at the start of the next reclaim pass, which means if
reclaim passes happen more frequently than the 2s flushes (eg under
sustained/heavy memory pressure), this logic will incorrectly assume
no refaults occurred and deactivation is disabled, which afaict means
folios won't be moved from the active -> inactive list, and reclaim
will start evicting pages from the inactive list that are hot (eg more
refaults).

This is from reading the code rather than a reproducer though, so I
don't know how much this matters in practice. Unlike for the writeback
case, I don't think flushign more frequently solves the problem. It
would shrink the staleness window but not eliminate it. Maybe one
solution is to keep the deactivation state for a period of time if
refaults were observed.

If it turns out iterating the parents and updating the per-CPU
counters of the stat on updates is too expensive, perhaps it makes
sense to keep the existing rstat logic as is and have caller-side
workarounds, as only writeback and vmscan would require them.

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

On Wed, Aug 12, 2026 at 1:59 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Mon, Aug 10, 2026 at 5:48 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Mon, Aug 10, 2026 at 09:49:31AM -0700, Joanne Koong wrote:
> > > On Thu, Aug 6, 2026 at 2:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > > >
> > > > On Thu, Aug 6, 2026 at 2:04 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > > > >
> > > > > I'm not too familiar with memcg or vmstat so apologies if this is a
> > > > > naive question, but does it make sense for memcg to just do a similar
> > > > > approach to what vmstat does? As I understand it, mod_lruvec_state()
> > > > > updates the node counter and the memcg counter (if memcg applies),
> > > > > where for updating the node counter, mod_node_state() does some
> > > > > batching where it updates the global counter for that stat only once
> > > > > it crosses some threshold (pcp->stat_threshold). Could memcg keep its
> > > > > per-CPU counter as the accumulator but once it reaches some threshold,
> > > > > it then propagates it up the parent/ancestors into a per-memcg atomic
> > > > > global counter? The atomic would then only have to be touched once per
> > > > > batch than on every update, which might eliminate the overhead of the
> > > > > per-memcg atomic you saw previously? It seems like this would get rid
> > > > > of needing to do any flushing altogether, as we could just read that
> > > > > global counter directly.
> > > >
> > > > The memcg stats perform similar per-CPU counting and thresholding. The
> > > > main difference from vmstat AFAICT is the cgroup hierarchy. In vmstat
> > > > we have per-CPU counters and a global atomic counter for each stat.
> > > > For memcg, we have the same for each cgroup, then we also have a
> > > > cgroup tree. When we read the stats of a cgroup, we usually want the
> > > > hierarchical stats including all of its children, so we can't just
> > > > read the global counter.
> > > >
> > > > We use rstat, a cgroup framework that keeps track of which cgroups
> > > > have updates on which CPUs, and then memcg has thresholding logic on
> > > > top to only flush if the number of pending updates exceeds a
> > > > threshold. Essentially, if the magnitude updates on a CPU exceeds
> > > > MEMCG_CHARGE_BATCH, we add it to a global per-memcg counter. We only
> > > > flush if that global per-memcg counter has > NR_CPUS *
> > > > MEMCG_CHARGE_BATCH updates. So in theory I think we tolerate up to 2 *
> > > > NR_CPUS * MEMCG_CHARGE_BATCH of stale stats (combined for all the
> > > > stats).
> > > >
> > > > See memcg_rstat_updated() and memcg_vmstats_needs_flush() for this logic.
> > > >
> > > > Looking at this code again, it made me realize that we already iterate
> > > > the per-CPU stats_updates on every update to increment the number of
> > > > pending stats on this CPU. I wonder if we can replace this with
> > > > actually updating the stat on each parent, then the flush path only
> > > > needs to accumulate CPU counters. Or maybe as you mention, we update
> > > > the global per-memcg counter when updates on a CPU exceed a threshold,
> > > > and forgo the rstat flushing logic completely.
> > > >
> > >
> > > I can run some experiments and see if updating the global per-memcg
> > > counter per batch and forgoing flushing could be a viable solution.
> > > I can use my setup to test it for writeback, but for the zswap use
> > > case, what's the best way to test if the changes are too expensive? Is
> > > there a benchmark program you're running?
> >
> > I was running stress-ng and measuring the latency of zswap_load() and
> > zswap_store() with bpftrace. But I suspect whatever works for writeback
> > should work for zswap :)
> >
> > >
> > > For updating the stat on each parent and having the flush path only
> > > accumulate CPU counters, I think this means we would have to
> > > unconditionally walk the ancestors + update its counters on every
> > > update, as we wouldn't be able to early break if the flushable
> > > threshold was already exceeded. I don't think we could replace the
> > > stats_update update, as it seems like readers still need some way of
> > > knowing when to flush. It seems like this could be a nontrivial
> > > performance hit, but maybe it's a non-factor in reality. If you think
> > > this would be useful to get benchmark numbers on, I can run some
> > > experiments on this too.
> >
> > I think we need to experiment with how much work we can shift around
> > between the update and reader sides.
> >
> > Right now, on the update side, we call __css_rstat_updated(), which
> > iterates the parents and puts them on the per-CPU rstat tree, then we
> > iterate the parents again in memcg_rstat_updated() and:
> > - Check if flushing threshold is already met.
> > - Update per-CPU counter for stats_updates.
> > - If per-CPU update exceeds (another) threshold, we update an atomic for
> >   stats_updates.
> >
> > The flush side checks the flushing threshold and does an rstat flush,
> > which will walk the update trees on all CPUs and iterates all child
> > cgroups on it. Pretty expensive when the threshold is met.
> >
> > I think we discussed two different options:
> >
> > (a) On stats updates, iterate the parents and update the per-CPU
> > counters of the stat directly.
> >
> > In this case, we can't keep using rstat, as we would still need to add
> > the cgroup and all its ancestors to the update tree anyway and the flush
> > cost will remain similar. So on the flush path we'll need to iterate all
> > CPUs and add the counters. We can explore if use a cpumask can help
> > here, but perhaps not since we'll need to update it atomically on all
> > stat updates.
> >
> > I imagine that if we do this we'll drop the stats_updates optimization
> > to avoid adding more cost to the update side. We'll be replacing parent
> > stats_updates modification with parent stat counter modification. But
> > maybe it's feasible to keep both to limit the cost of flusing.
> >
> > This only works if the cost of iterate the CPUs on the flush side ends
> > up being cheaper than the current flush. I imagine the average cost will
> > be higher but the cost should be much more consistent vs the current
> > flush, so the tail should be better.
> >
> > (b) On stats updates, iterate the parents and update the per-CPU
> > counters of the stat directly AND update a global atomic if the per-CPU
> > counter exceeds a threshold. I think updates may get too expensive as we
> > may need to update multiple atomics, especially when updating the root
> > counter as it will be contended by all cgroups.
> >
> > The flush side becomes an atomic read, so should be very cheap, and we
> > can drop other heuristics and optimizations.
> >
> > ---
> >
> > I think ultimately we need to experiment with these approachs (and
> > perhaps others) to check what actually works in practice. You'll
> > probably want some benchmarks or synthetic tests with many cgroups to
> > exercise the worst case scenarios.
> >
> > For the zswap stats, I used to run stress-ng with a bunch of workers
> > (20, 50, 100, ..) in cgroups of different levels. Since all workers are
> > in the same cgroups, all the parents are common and all threads end up
> > competing on the same atomics, so worst case scenario. I was also
> > testing in a VM that has more CPUs than stress-ng workers, to exercise
> > the full extent of the concurrency and contention.
> >
> > This could be a lot of work, so I am not really asking you to do it, but
> > just brainstorming and sharing ideas :)
>
> Thanks for your guidance and insights on this, Yosry! Your responses
> have been very helpful.
>
> I'll do some experimentation with this and try to get a sense of what
> the update costs are.

Looking forward to that!

>
> One thing I was unclear about and have been looking at is how
> pervasive/important this issue is outside of the writeback case. It
> looks like there are roughly 4 other categories of users that rely on
> stats that can be 2s stale: zswap, cachestat(), working set, and
> vmscan.
>
> Of these, vmscan is the only one that looks troublesome. Some armchair
> analysis below:
>
> a) zswap (zswap_shrinker_count()): 2s of staleness looks fine. the
> stats are only used to calculate the compression ratio and the ratio
> scales a count that comes from a list_lru and *isn't* stale. The ratio
> is an average over the whole pool, so it seems like that should be
> fairly steady across 2s. A stale result doesn't seem too bad - afaict,
> it is a heuristic of how zswap should get weighed against the other
> shrinkers.

There is also the zswap limits calculation, which Song sent a patch to
change to using ratelimited flushing. I think that case is more
problematic as it can cause OOM kills if we use stale stats (e.g.
after a burst of writeback frees up space in zswap). Adding Song here.

The shrinker case is more benign.

>
> b) cachestat() syscall (filemap_cachestat()): seems preferable to just
> rely on the 2s flusher worker. The results are returned back to
> userspace and not used for any internal kernel decision-making, seems
> ok to have one field in it (nr_recently_evicted) potentially be stale.
> Seems better to err on the side of less frequent flushing anyways, as
> the call path is invoked from unprivileged userspace and flushes the
> root memcg.

I am not sure how the cachestat() syscall is used, but my preference
would be to treat it like memory.stat and do a full flush on userspace
read, unless it's a syscall that is invoked frequently for some
reason.

>
> c) workingset: looking at the original commits, the calculations are
> estimates that don't require exact stats -
>
> refaulting (workingset_test_recent()): commit 9b3016154c91 ("memcg:
> sync flush only if periodic flush is delayed")
>     Now the question: what are the side-effects of this change? The worst
>     that can happen is the refault codepath will see 4sec old lruvec stats
>     and may cause false (or missed) activations of the refaulted page which
>     may under-or-overestimate the workingset size.  Though that is not very
>     concerning as the kernel can already miss or do false activations.

Incorrect activations can lead to thrashing in some cases, but I am
not sure how practical the concern is.

>
> shadow-node shrinker (count_shadow_nodes()): commit d4a5b369ad6d ("mm:
> ratelimit stat flush from workingset shrinker")
>     On further inspection it seems like we don't really need
>     accurate stats in this function as it was already approximating the amount
>     of appropriate shadow entries to keep for maintaining the refault
>     information.
>
> d) vmscan (prepare_scan_control()): for determining whether reclaim
> can deactivate, there's this logic
>
>                 /*
>                  * When refaults are being observed, it means a new
>                  * workingset is being established. Deactivate to get
>                  * rid of any stale active pages quickly.
>                  */
>                 refaults = lruvec_page_state(target_lruvec,
>                                 WORKINGSET_ACTIVATE_ANON);
>                 if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
>                         inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
>                         sc->may_deactivate |= DEACTIVATE_ANON;
>                 else
>                         sc->may_deactivate &= ~DEACTIVATE_ANON;
>
> (and same for DEACTIVATE_FILE for file-backed folios)
>
> target_lruvec->refaults[] gets set at the end of the reclaim pass. It
> stores the same lruvec_page_state() value that prepare_scan_control()
> will read at the start of the next reclaim pass, which means if
> reclaim passes happen more frequently than the 2s flushes (eg under
> sustained/heavy memory pressure), this logic will incorrectly assume
> no refaults occurred and deactivation is disabled, which afaict means
> folios won't be moved from the active -> inactive list, and reclaim
> will start evicting pages from the inactive list that are hot (eg more
> refaults).
>
> This is from reading the code rather than a reproducer though, so I
> don't know how much this matters in practice. Unlike for the writeback
> case, I don't think flushign more frequently solves the problem. It
> would shrink the staleness window but not eliminate it. Maybe one
> solution is to keep the deactivation state for a period of time if
> refaults were observed.

We do have an internal test that runs a VM under memory pressure and
we started observing OOM kills after the ratelimited flush was added
in vmscan, so I think it's more problematic than just reclaiming hot
memory, but I didn't look too closely.

>
> If it turns out iterating the parents and updating the per-CPU
> counters of the stat on updates is too expensive, perhaps it makes
> sense to keep the existing rstat logic as is and have caller-side
> workarounds, as only writeback and vmscan would require them.

Also zswap limits calculation. I am not sure how benign the workingset
case is, but it has been ratelimited for a while now AFAICT, so maybe
it's fine.

>
> Thanks,
> Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Tue, Aug 18, 2026 at 11:40 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Wed, Aug 12, 2026 at 1:59 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > d) vmscan (prepare_scan_control()): for determining whether reclaim
> > can deactivate, there's this logic
> >
> >                 /*
> >                  * When refaults are being observed, it means a new
> >                  * workingset is being established. Deactivate to get
> >                  * rid of any stale active pages quickly.
> >                  */
> >                 refaults = lruvec_page_state(target_lruvec,
> >                                 WORKINGSET_ACTIVATE_ANON);
> >                 if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
> >                         inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
> >                         sc->may_deactivate |= DEACTIVATE_ANON;
> >                 else
> >                         sc->may_deactivate &= ~DEACTIVATE_ANON;
> >
> > (and same for DEACTIVATE_FILE for file-backed folios)
> >
> > target_lruvec->refaults[] gets set at the end of the reclaim pass. It
> > stores the same lruvec_page_state() value that prepare_scan_control()
> > will read at the start of the next reclaim pass, which means if
> > reclaim passes happen more frequently than the 2s flushes (eg under
> > sustained/heavy memory pressure), this logic will incorrectly assume
> > no refaults occurred and deactivation is disabled, which afaict means
> > folios won't be moved from the active -> inactive list, and reclaim
> > will start evicting pages from the inactive list that are hot (eg more
> > refaults).
> >
> > This is from reading the code rather than a reproducer though, so I
> > don't know how much this matters in practice. Unlike for the writeback
> > case, I don't think flushign more frequently solves the problem. It
> > would shrink the staleness window but not eliminate it. Maybe one
> > solution is to keep the deactivation state for a period of time if
> > refaults were observed.
>
> We do have an internal test that runs a VM under memory pressure and
> we started observing OOM kills after the ratelimited flush was added
> in vmscan, so I think it's more problematic than just reclaiming hot
> memory, but I didn't look too closely.
>

Would you be able to share the test you're using? I've been trying to
repro it locally without much luck.

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

On Thu, Aug 20, 2026 at 4:39 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Tue, Aug 18, 2026 at 11:40 AM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Wed, Aug 12, 2026 at 1:59 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >
> > > d) vmscan (prepare_scan_control()): for determining whether reclaim
> > > can deactivate, there's this logic
> > >
> > >                 /*
> > >                  * When refaults are being observed, it means a new
> > >                  * workingset is being established. Deactivate to get
> > >                  * rid of any stale active pages quickly.
> > >                  */
> > >                 refaults = lruvec_page_state(target_lruvec,
> > >                                 WORKINGSET_ACTIVATE_ANON);
> > >                 if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
> > >                         inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
> > >                         sc->may_deactivate |= DEACTIVATE_ANON;
> > >                 else
> > >                         sc->may_deactivate &= ~DEACTIVATE_ANON;
> > >
> > > (and same for DEACTIVATE_FILE for file-backed folios)
> > >
> > > target_lruvec->refaults[] gets set at the end of the reclaim pass. It
> > > stores the same lruvec_page_state() value that prepare_scan_control()
> > > will read at the start of the next reclaim pass, which means if
> > > reclaim passes happen more frequently than the 2s flushes (eg under
> > > sustained/heavy memory pressure), this logic will incorrectly assume
> > > no refaults occurred and deactivation is disabled, which afaict means
> > > folios won't be moved from the active -> inactive list, and reclaim
> > > will start evicting pages from the inactive list that are hot (eg more
> > > refaults).
> > >
> > > This is from reading the code rather than a reproducer though, so I
> > > don't know how much this matters in practice. Unlike for the writeback
> > > case, I don't think flushign more frequently solves the problem. It
> > > would shrink the staleness window but not eliminate it. Maybe one
> > > solution is to keep the deactivation state for a period of time if
> > > refaults were observed.
> >
> > We do have an internal test that runs a VM under memory pressure and
> > we started observing OOM kills after the ratelimited flush was added
> > in vmscan, so I think it's more problematic than just reclaiming hot
> > memory, but I didn't look too closely.
> >
>
> Would you be able to share the test you're using? I've been trying to
> repro it locally without much luck.

The exact test uses some internal testing libraries, we're looking
into whether we can translate it to a standalone C repro.

From a high-level, the test:
- Creates a VM with 260 MB (256 MB for testing + 4 MB margin) in a
cgroup, backed by shmem THPs. The VM is not a fully fledged VM, just a
KVM VM running bare metal code (so 4 MB is enough margin).
- Enables zswap in the cgroup.
- Tells the VM to write to 256 MB of its memory.
- Checks the cgroup shmem usage and that no swap occurred (no limit).
- Sets the cgroup limit to half the VM memory.
- Checks the cgroup shmem usage and swap again, usage should be ~half
and so is swap.
- Tells the VM to read the 256 MB of memory again. This should cause a
lot of thrashing and reclaim, but no OOM kills as all guest memory is
reclaimable.

With the ratelimited flushing, the test started flaking 5-10% of the
time due to OOM kills.

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[James Houghton]

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

On Fri, Aug 21, 2026 at 12:41 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Thu, Aug 20, 2026 at 4:39 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > Would you be able to share the test you're using? I've been trying to
> > repro it locally without much luck.
>
> The exact test uses some internal testing libraries, we're looking
> into whether we can translate it to a standalone C repro.
>
> From a high-level, the test:
> - Creates a VM with 260 MB (256 MB for testing + 4 MB margin) in a
> cgroup, backed by shmem THPs. The VM is not a fully fledged VM, just a
> KVM VM running bare metal code (so 4 MB is enough margin).
> - Enables zswap in the cgroup.
> - Tells the VM to write to 256 MB of its memory.
> - Checks the cgroup shmem usage and that no swap occurred (no limit).
> - Sets the cgroup limit to half the VM memory.
> - Checks the cgroup shmem usage and swap again, usage should be ~half
> and so is swap.
> - Tells the VM to read the 256 MB of memory again. This should cause a
> lot of thrashing and reclaim, but no OOM kills as all guest memory is
> reclaimable.
>
> With the ratelimited flushing, the test started flaking 5-10% of the
> time due to OOM kills.

Hi Joanne, Yosry,

I've attached a reproducer for you (as a patch that should apply to
Linus's tree).

In tools/testing/selftests/kvm, `make` should build it. Instructions
for running it (like setting up swap/etc.) are in the comment at the
top of the file.

As part of the patch, I've also included a sysctl to dynamically tune
whether or not the memcg stats flush is rate-limited. You should find
that when it is rate-limited, OOMs occur, and when it is not, OOMs do
not occur.

This reproducer behaves slightly differently than our internal
version. This version, when run with `-b 512M` anyway, seems to OOM
simply by setting memory.max, whereas our internal version would OOM
upon re-accessing the memory. Not really a meaningful difference, but
just wanted to point it out.

Please let me know if you have any trouble running this test or if the
test is wrong in some way.

Thanks!

[-- Attachment #2: 0001-KVM-selftests-reclaim_test.patch --]
[-- Type: application/x-patch, Size: 11818 bytes --]

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Fri, Aug 21, 2026 at 6:39 PM James Houghton <jthoughton@google.com> wrote:
>
> On Fri, Aug 21, 2026 at 12:41 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Thu, Aug 20, 2026 at 4:39 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > > Would you be able to share the test you're using? I've been trying to
> > > repro it locally without much luck.
> >
> > The exact test uses some internal testing libraries, we're looking
> > into whether we can translate it to a standalone C repro.
> >
> > From a high-level, the test:
> > - Creates a VM with 260 MB (256 MB for testing + 4 MB margin) in a
> > cgroup, backed by shmem THPs. The VM is not a fully fledged VM, just a
> > KVM VM running bare metal code (so 4 MB is enough margin).
> > - Enables zswap in the cgroup.
> > - Tells the VM to write to 256 MB of its memory.
> > - Checks the cgroup shmem usage and that no swap occurred (no limit).
> > - Sets the cgroup limit to half the VM memory.
> > - Checks the cgroup shmem usage and swap again, usage should be ~half
> > and so is swap.
> > - Tells the VM to read the 256 MB of memory again. This should cause a
> > lot of thrashing and reclaim, but no OOM kills as all guest memory is
> > reclaimable.
> >
> > With the ratelimited flushing, the test started flaking 5-10% of the
> > time due to OOM kills.
>
> Hi Joanne, Yosry,
>
> I've attached a reproducer for you (as a patch that should apply to
> Linus's tree).
>
> In tools/testing/selftests/kvm, `make` should build it. Instructions
> for running it (like setting up swap/etc.) are in the comment at the
> top of the file.
>
> As part of the patch, I've also included a sysctl to dynamically tune
> whether or not the memcg stats flush is rate-limited. You should find
> that when it is rate-limited, OOMs occur, and when it is not, OOMs do
> not occur.
>
> This reproducer behaves slightly differently than our internal
> version. This version, when run with `-b 512M` anyway, seems to OOM
> simply by setting memory.max, whereas our internal version would OOM
> upon re-accessing the memory. Not really a meaningful difference, but
> just wanted to point it out.
>
> Please let me know if you have any trouble running this test or if the
> test is wrong in some way.
>

Thank you so much, James! This is very helpful - I will run some
experiments with your repro this week.

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Mon, Aug 10, 2026 at 5:48 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Mon, Aug 10, 2026 at 09:49:31AM -0700, Joanne Koong wrote:
> > On Thu, Aug 6, 2026 at 2:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > For updating the stat on each parent and having the flush path only
> > accumulate CPU counters, I think this means we would have to
> > unconditionally walk the ancestors + update its counters on every
> > update, as we wouldn't be able to early break if the flushable
> > threshold was already exceeded. I don't think we could replace the
> > stats_update update, as it seems like readers still need some way of
> > knowing when to flush. It seems like this could be a nontrivial
> > performance hit, but maybe it's a non-factor in reality. If you think
> > this would be useful to get benchmark numbers on, I can run some
> > experiments on this too.
>
> I think we need to experiment with how much work we can shift around
> between the update and reader sides.
>
> Right now, on the update side, we call __css_rstat_updated(), which
> iterates the parents and puts them on the per-CPU rstat tree, then we
> iterate the parents again in memcg_rstat_updated() and:
> - Check if flushing threshold is already met.
> - Update per-CPU counter for stats_updates.
> - If per-CPU update exceeds (another) threshold, we update an atomic for
>   stats_updates.
>
> The flush side checks the flushing threshold and does an rstat flush,
> which will walk the update trees on all CPUs and iterates all child
> cgroups on it. Pretty expensive when the threshold is met.
>
> I think we discussed two different options:
>
> (a) On stats updates, iterate the parents and update the per-CPU
> counters of the stat directly.
>
> In this case, we can't keep using rstat, as we would still need to add
> the cgroup and all its ancestors to the update tree anyway and the flush
> cost will remain similar. So on the flush path we'll need to iterate all
> CPUs and add the counters. We can explore if use a cpumask can help
> here, but perhaps not since we'll need to update it atomically on all
> stat updates.
>
> I imagine that if we do this we'll drop the stats_updates optimization
> to avoid adding more cost to the update side. We'll be replacing parent
> stats_updates modification with parent stat counter modification. But
> maybe it's feasible to keep both to limit the cost of flusing.
>
> This only works if the cost of iterate the CPUs on the flush side ends
> up being cheaper than the current flush. I imagine the average cost will
> be higher but the cost should be much more consistent vs the current
> flush, so the tail should be better.
>
> (b) On stats updates, iterate the parents and update the per-CPU
> counters of the stat directly AND update a global atomic if the per-CPU
> counter exceeds a threshold. I think updates may get too expensive as we
> may need to update multiple atomics, especially when updating the root
> counter as it will be contended by all cgroups.
>
> The flush side becomes an atomic read, so should be very cheap, and we
> can drop other heuristics and optimizations.
>
> ---
>
> I think ultimately we need to experiment with these approachs (and
> perhaps others) to check what actually works in practice. You'll
> probably want some benchmarks or synthetic tests with many cgroups to
> exercise the worst case scenarios.
>

These are the results I'm seeing from the benchmarks. I'm comparing
non-ratelimited rstat vs (a) vs (b). I'm comparing non-ratelimited
rstat instead of ratelimited rstat, because ratelimited rstat relies
on the 2s flusher and doesn't incur any additional flushing when
called.

~On the update side~
ns per update (measured by having a kthread pinned to each CPU and all
threads updating the same cgroup for a 200ms window, run on 80 CPUs,
computed as total CPU time / # updates)
depth      rstat      (a)         (b)
    1        22.34       32.49      60.10
    8        22.46       47.12      140.72
    16      22.49       70.96      206.39
    32      22.51       139.16    305.40

(a) and (b) perform worse on higher memcg depths, whereas rstat is
unaffected by depth level, which makes sense given that (a) and (b)
can't break early if the flushable threshold has already been
exceeded.

~On the flush side~
ns per flush call (measured at leaf of memcg chain, ie best scenario
for rstat, # CPUs on the system is 80)
  rstat                      10165.60 ns   (min 9924)
    (a)                       734.10 ns
    (b)                        4.79 ns

(a) is around 14x cheaper than an rstat flush, which I think is
because the rstat flush goes through every stat, even ones that are
not directly relevant.


Using some napkin math, I think the breakeven point for (a) is around:
                      updates per read
depth 1             ~900
depth 8             ~380
depth 32           ~80

eg for depth 1:  (10165.60 - 734.10) / (32.49 -  22.34)

This is assuming one read per flush, which is what (a) and (b) would
necessarily have to pay, whereas rstat only needs to flush when the #
of stats updates exceeds the MEMCG_CHARGE_BATCH * nr_cpus threshold,
so the updates per read numbers above are in reality lower.

Running a fio write job and using tracepoints to count # of stat
updates, # of reads of those stats, and how many of those reads flush,
I'm seeing:
                     updates/s              reads/s
flushes/s         updates per read
bs=1M            643                           119                 45
                       5
bs=4k            156,415                     123                 58
                     1267

For larger folio sizes, (a) is cheaper for writeback (since there are
4 update events *per folio* in the dirtying/writeback lifecycle) but
for smaller folio sizes (eg order 0 folios), (a) is worse, as a lot
more updates are incurred, and the performance is worse the deeper the
memcg is nested.


I also ran some experiments with Jan's suggestion of scaling the flush
threshold to the memcg's limit rather than to the number of cpus. It
helps the overshoot we saw and slightly outperforms doing a plain
non-ratelimited flush (100% of dirty limit vs 101% on a 1G memcg, 103%
vs 107% on a 256M memcg) but led to ~2.4x more flushes.

The initial reason we switched to the ratelimited version of the flush
was because flushing was expensive, but I'm not sure if flushing is as
expensive as it used to be. Testing the non-ratelimited version, the
flushes account for about 0.9% of the system CPU the write path
already spends, and I didn't see any throughput difference between the
two (249-252 MB/s either way), on a 32G memcg. With the
non-ratelimited version, it's also not a flush per read - it's still
"ratelimited" by number of updates that have elapsed since the last
flush (only flushing if stats_updates >= MEMCG_CHARGE_BATCH * nr_cpus)
which means concurrent writers writing back at the same time will
"share" flushes rather than triggering their own.

In light of the above, imo it seems to make the most sense to just do
the plain non-ratelimited flush for mem_cgroup_wb_stats() and make the
flushing magnitude bound again, wh ich also aligns with what the
global non-memcg domain does. The question is whether the extra flush
cost is acceptable, but the numbers above make me think it is.

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 5:29 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Mon, Aug 10, 2026 at 5:48 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Mon, Aug 10, 2026 at 09:49:31AM -0700, Joanne Koong wrote:
> > > On Thu, Aug 6, 2026 at 2:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
> > > For updating the stat on each parent and having the flush path only
> > > accumulate CPU counters, I think this means we would have to
> > > unconditionally walk the ancestors + update its counters on every
> > > update, as we wouldn't be able to early break if the flushable
> > > threshold was already exceeded. I don't think we could replace the
> > > stats_update update, as it seems like readers still need some way of
> > > knowing when to flush. It seems like this could be a nontrivial
> > > performance hit, but maybe it's a non-factor in reality. If you think
> > > this would be useful to get benchmark numbers on, I can run some
> > > experiments on this too.
> >
> > I think we need to experiment with how much work we can shift around
> > between the update and reader sides.
> >
> > Right now, on the update side, we call __css_rstat_updated(), which
> > iterates the parents and puts them on the per-CPU rstat tree, then we
> > iterate the parents again in memcg_rstat_updated() and:
> > - Check if flushing threshold is already met.
> > - Update per-CPU counter for stats_updates.
> > - If per-CPU update exceeds (another) threshold, we update an atomic for
> >   stats_updates.
> >
> > The flush side checks the flushing threshold and does an rstat flush,
> > which will walk the update trees on all CPUs and iterates all child
> > cgroups on it. Pretty expensive when the threshold is met.
> >
> > I think we discussed two different options:
> >
> > (a) On stats updates, iterate the parents and update the per-CPU
> > counters of the stat directly.
> >
> > In this case, we can't keep using rstat, as we would still need to add
> > the cgroup and all its ancestors to the update tree anyway and the flush
> > cost will remain similar. So on the flush path we'll need to iterate all
> > CPUs and add the counters. We can explore if use a cpumask can help
> > here, but perhaps not since we'll need to update it atomically on all
> > stat updates.
> >
> > I imagine that if we do this we'll drop the stats_updates optimization
> > to avoid adding more cost to the update side. We'll be replacing parent
> > stats_updates modification with parent stat counter modification. But
> > maybe it's feasible to keep both to limit the cost of flusing.
> >
> > This only works if the cost of iterate the CPUs on the flush side ends
> > up being cheaper than the current flush. I imagine the average cost will
> > be higher but the cost should be much more consistent vs the current
> > flush, so the tail should be better.
> >
> > (b) On stats updates, iterate the parents and update the per-CPU
> > counters of the stat directly AND update a global atomic if the per-CPU
> > counter exceeds a threshold. I think updates may get too expensive as we
> > may need to update multiple atomics, especially when updating the root
> > counter as it will be contended by all cgroups.
> >
> > The flush side becomes an atomic read, so should be very cheap, and we
> > can drop other heuristics and optimizations.
> >
> > ---
> >
> > I think ultimately we need to experiment with these approachs (and
> > perhaps others) to check what actually works in practice. You'll
> > probably want some benchmarks or synthetic tests with many cgroups to
> > exercise the worst case scenarios.
> >
>
> These are the results I'm seeing from the benchmarks. I'm comparing
> non-ratelimited rstat vs (a) vs (b). I'm comparing non-ratelimited
> rstat instead of ratelimited rstat, because ratelimited rstat relies
> on the 2s flusher and doesn't incur any additional flushing when
> called.
>
> ~On the update side~
> ns per update (measured by having a kthread pinned to each CPU and all
> threads updating the same cgroup for a 200ms window, run on 80 CPUs,
> computed as total CPU time / # updates)
> depth      rstat      (a)         (b)
>     1        22.34       32.49      60.10
>     8        22.46       47.12      140.72
>     16      22.49       70.96      206.39
>     32      22.51       139.16    305.40

There goes my dream if an rstat-free world :')

I am very surprised though that you care about hierarchies this deep.
I don't think we have anything close to even 8 at Google, and in all
my previous testing I usually test up to 4 or 5 levels. I suspect at
that level the difference might be acceptable. But aren't other
operations also really expensive at that depth? I imagine the charging
path is really slow if it has to do atomic updates for 32 parents. Is
the update path really a problem given everything else?

Given the constant cost of rstat updates, it also seems like
memcg_vmstats_needs_flush() is mostly evaluating to true and skipping
the parent walk. I wonder how much this remains true in practice, I
think Shakeel mentioned that it does help in production.

>
> (a) and (b) perform worse on higher memcg depths, whereas rstat is
> unaffected by depth level, which makes sense given that (a) and (b)
> can't break early if the flushable threshold has already been
> exceeded.
>
> ~On the flush side~
> ns per flush call (measured at leaf of memcg chain, ie best scenario
> for rstat, # CPUs on the system is 80)
>   rstat                      10165.60 ns   (min 9924)
>     (a)                       734.10 ns
>     (b)                        4.79 ns
>
> (a) is around 14x cheaper than an rstat flush, which I think is
> because the rstat flush goes through every stat, even ones that are
> not directly relevant.

I think that's only part of the story. rstat iterates all children
cgroups as well. I think reading multiple contiguous stats may
actually be really cheap since they are likely in the same cacheline.

>
> Using some napkin math, I think the breakeven point for (a) is around:
>                       updates per read
> depth 1             ~900
> depth 8             ~380
> depth 32           ~80
>
> eg for depth 1:  (10165.60 - 734.10) / (32.49 -  22.34)
>
> This is assuming one read per flush, which is what (a) and (b) would
> necessarily have to pay, whereas rstat only needs to flush when the #
> of stats updates exceeds the MEMCG_CHARGE_BATCH * nr_cpus threshold,
> so the updates per read numbers above are in reality lower.
>
> Running a fio write job and using tracepoints to count # of stat
> updates, # of reads of those stats, and how many of those reads flush,
> I'm seeing:
>                      updates/s              reads/s
> flushes/s         updates per read
> bs=1M            643                           119                 45
>                        5
> bs=4k            156,415                     123                 58
>                      1267
>
> For larger folio sizes, (a) is cheaper for writeback (since there are
> 4 update events *per folio* in the dirtying/writeback lifecycle) but
> for smaller folio sizes (eg order 0 folios), (a) is worse, as a lot
> more updates are incurred, and the performance is worse the deeper the
> memcg is nested.

Hmm interesting analysis, but I actually think a more interesting one
is the effect of the update latency in respective update paths. For
example, in the zswap_store() case you analyzed earlier, it ends up
being in the noise. I would check what the hottest update paths are
(probably networking?) and check what the update paths are, to check
if they are really sensitive to the increased update cost. It could be
the case that at least stats that are used by in-kernel flushers do
not really care about the extra update cost.

>
>
> I also ran some experiments with Jan's suggestion of scaling the flush
> threshold to the memcg's limit rather than to the number of cpus. It
> helps the overshoot we saw and slightly outperforms doing a plain
> non-ratelimited flush (100% of dirty limit vs 101% on a 1G memcg, 103%
> vs 107% on a 256M memcg) but led to ~2.4x more flushes.
>
> The initial reason we switched to the ratelimited version of the flush
> was because flushing was expensive, but I'm not sure if flushing is as
> expensive as it used to be. Testing the non-ratelimited version, the
> flushes account for about 0.9% of the system CPU the write path
> already spends, and I didn't see any throughput difference between the
> two (249-252 MB/s either way), on a 32G memcg. With the
> non-ratelimited version, it's also not a flush per read - it's still
> "ratelimited" by number of updates that have elapsed since the last
> flush (only flushing if stats_updates >= MEMCG_CHARGE_BATCH * nr_cpus)
> which means concurrent writers writing back at the same time will
> "share" flushes rather than triggering their own.

Yup, this is exactly what I mean. This is a good analysis for the cost
of the flush: how much it matters end-to-end. Even though the full
flush is 14x more expensive according to your numbers, it seems like
it ultimately doesn't affect performance much. I would do the same for
the update paths, see if the extra cost really matters. WDYT?

>
> In light of the above, imo it seems to make the most sense to just do
> the plain non-ratelimited flush for mem_cgroup_wb_stats() and make the
> flushing magnitude bound again, wh ich also aligns with what the
> global non-memcg domain does. The question is whether the extra flush
> cost is acceptable, but the numbers above make me think it is.

If we can eat the cost of the flush here that's great, but I think we
can't say the same for the vmscan and zswap paths (at least)? :/

>
> Thanks,
> Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Wed, Aug 26, 2026 at 11:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Wed, Aug 26, 2026 at 5:29 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > On Mon, Aug 10, 2026 at 5:48 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
> > > On Mon, Aug 10, 2026 at 09:49:31AM -0700, Joanne Koong wrote:
> > > > On Thu, Aug 6, 2026 at 2:31 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > > >
> > > > For updating the stat on each parent and having the flush path only
> > > > accumulate CPU counters, I think this means we would have to
> > > > unconditionally walk the ancestors + update its counters on every
> > > > update, as we wouldn't be able to early break if the flushable
> > > > threshold was already exceeded. I don't think we could replace the
> > > > stats_update update, as it seems like readers still need some way of
> > > > knowing when to flush. It seems like this could be a nontrivial
> > > > performance hit, but maybe it's a non-factor in reality. If you think
> > > > this would be useful to get benchmark numbers on, I can run some
> > > > experiments on this too.
> > >
> > > I think we need to experiment with how much work we can shift around
> > > between the update and reader sides.
> > >
> > > Right now, on the update side, we call __css_rstat_updated(), which
> > > iterates the parents and puts them on the per-CPU rstat tree, then we
> > > iterate the parents again in memcg_rstat_updated() and:
> > > - Check if flushing threshold is already met.
> > > - Update per-CPU counter for stats_updates.
> > > - If per-CPU update exceeds (another) threshold, we update an atomic for
> > >   stats_updates.
> > >
> > > The flush side checks the flushing threshold and does an rstat flush,
> > > which will walk the update trees on all CPUs and iterates all child
> > > cgroups on it. Pretty expensive when the threshold is met.
> > >
> > > I think we discussed two different options:
> > >
> > > (a) On stats updates, iterate the parents and update the per-CPU
> > > counters of the stat directly.
> > >
> > > In this case, we can't keep using rstat, as we would still need to add
> > > the cgroup and all its ancestors to the update tree anyway and the flush
> > > cost will remain similar. So on the flush path we'll need to iterate all
> > > CPUs and add the counters. We can explore if use a cpumask can help
> > > here, but perhaps not since we'll need to update it atomically on all
> > > stat updates.
> > >
> > > I imagine that if we do this we'll drop the stats_updates optimization
> > > to avoid adding more cost to the update side. We'll be replacing parent
> > > stats_updates modification with parent stat counter modification. But
> > > maybe it's feasible to keep both to limit the cost of flusing.
> > >
> > > This only works if the cost of iterate the CPUs on the flush side ends
> > > up being cheaper than the current flush. I imagine the average cost will
> > > be higher but the cost should be much more consistent vs the current
> > > flush, so the tail should be better.
> > >
> > > (b) On stats updates, iterate the parents and update the per-CPU
> > > counters of the stat directly AND update a global atomic if the per-CPU
> > > counter exceeds a threshold. I think updates may get too expensive as we
> > > may need to update multiple atomics, especially when updating the root
> > > counter as it will be contended by all cgroups.
> > >
> > > The flush side becomes an atomic read, so should be very cheap, and we
> > > can drop other heuristics and optimizations.
> > >
> > > ---
> > >
> > > I think ultimately we need to experiment with these approachs (and
> > > perhaps others) to check what actually works in practice. You'll
> > > probably want some benchmarks or synthetic tests with many cgroups to
> > > exercise the worst case scenarios.
> > >
> >
> > These are the results I'm seeing from the benchmarks. I'm comparing
> > non-ratelimited rstat vs (a) vs (b). I'm comparing non-ratelimited
> > rstat instead of ratelimited rstat, because ratelimited rstat relies
> > on the 2s flusher and doesn't incur any additional flushing when
> > called.
> >
> > ~On the update side~
> > ns per update (measured by having a kthread pinned to each CPU and all
> > threads updating the same cgroup for a 200ms window, run on 80 CPUs,
> > computed as total CPU time / # updates)
> > depth      rstat      (a)         (b)
> >     1        22.34       32.49      60.10
> >     8        22.46       47.12      140.72
> >     16      22.49       70.96      206.39
> >     32      22.51       139.16    305.40
>
> There goes my dream if an rstat-free world :')
>
> I am very surprised though that you care about hierarchies this deep.
> I don't think we have anything close to even 8 at Google, and in all
> my previous testing I usually test up to 4 or 5 levels. I suspect at

At Meta, we often see depths 8 to 16.

> that level the difference might be acceptable. But aren't other
> operations also really expensive at that depth? I imagine the charging
> path is really slow if it has to do atomic updates for 32 parents. Is

I agree. This aligns with what I saw when running the page counter
approach for zwap on depth 32. For the writeback path though, the
dirtying / writeback folio lifecycle is just stat updates. There's a
charge, but that happens only once, when the folio enters the page
cache.

> the update path really a problem given everything else?
>
> Given the constant cost of rstat updates, it also seems like
> memcg_vmstats_needs_flush() is mostly evaluating to true and skipping
> the parent walk. I wonder how much this remains true in practice, I
> think Shakeel mentioned that it does help in production.

I think for the writeback path, this is probably often encountered in
practice, since the threshold is 64 * nr_cpus and any sustained write
would cross that quickly. It would matter most with order-0 folios
since there's 4 updates incurred per folio independent of folio size.

>
> >
> > (a) and (b) perform worse on higher memcg depths, whereas rstat is
> > unaffected by depth level, which makes sense given that (a) and (b)
> > can't break early if the flushable threshold has already been
> > exceeded.
> >
> > ~On the flush side~
> > ns per flush call (measured at leaf of memcg chain, ie best scenario
> > for rstat, # CPUs on the system is 80)
> >   rstat                      10165.60 ns   (min 9924)
> >     (a)                       734.10 ns
> >     (b)                        4.79 ns
> >
> > (a) is around 14x cheaper than an rstat flush, which I think is
> > because the rstat flush goes through every stat, even ones that are
> > not directly relevant.
>
> I think that's only part of the story. rstat iterates all children
> cgroups as well. I think reading multiple contiguous stats may
> actually be really cheap since they are likely in the same cacheline.

In that benchmark, the flush cost was measured at the leaf of the
memcg chain, so I don't think there were any children to iterate here.
Maybe some of it is attributed to the rstat locking? I'm not too sure.

>
> >
> > Using some napkin math, I think the breakeven point for (a) is around:
> >                       updates per read
> > depth 1             ~900
> > depth 8             ~380
> > depth 32           ~80
> >
> > eg for depth 1:  (10165.60 - 734.10) / (32.49 -  22.34)
> >
> > This is assuming one read per flush, which is what (a) and (b) would
> > necessarily have to pay, whereas rstat only needs to flush when the #
> > of stats updates exceeds the MEMCG_CHARGE_BATCH * nr_cpus threshold,
> > so the updates per read numbers above are in reality lower.
> >
> > Running a fio write job and using tracepoints to count # of stat
> > updates, # of reads of those stats, and how many of those reads flush,
> > I'm seeing:
> >                      updates/s              reads/s
> > flushes/s         updates per read
> > bs=1M            643                           119                 45
> >                        5
> > bs=4k            156,415                     123                 58
> >                      1267
> >
> > For larger folio sizes, (a) is cheaper for writeback (since there are
> > 4 update events *per folio* in the dirtying/writeback lifecycle) but
> > for smaller folio sizes (eg order 0 folios), (a) is worse, as a lot
> > more updates are incurred, and the performance is worse the deeper the
> > memcg is nested.
>
> Hmm interesting analysis, but I actually think a more interesting one
> is the effect of the update latency in respective update paths. For
> example, in the zswap_store() case you analyzed earlier, it ends up
> being in the noise. I would check what the hottest update paths are
> (probably networking?) and check what the update paths are, to check
> if they are really sensitive to the increased update cost. It could be
> the case that at least stats that are used by in-kernel flushers do
> not really care about the extra update cost.

Extrapolating a bit from the synthetic benchmarks, at depth 8, I think
the extra overhead incurred on an update comes out to roughly around
25ns, and at depth 16, around 48ns. I haven't looked at the networking
path, but the lru path (__update_lru_size()) looks concerning, as it
would have that extra overhead while holding the lru lock, which "is
already a heavy contention point on memory-heavy workloads" [1]. As I
understand it, reclaim, compaction, and the page cache path all
serialize on that lock.

If we were to move all the callers off rstat, I think this also would
unfortunately make the walk more expensive. As I understand it, each
stat would be a per-cpu counter which would be added to the
memcg_vmstats_percpu struct, and that would have to spill onto
multiple cachelines. Or maybe I'm missing something here and there
would be a better way to do it.

>
> >
> >
> > I also ran some experiments with Jan's suggestion of scaling the flush
> > threshold to the memcg's limit rather than to the number of cpus. It
> > helps the overshoot we saw and slightly outperforms doing a plain
> > non-ratelimited flush (100% of dirty limit vs 101% on a 1G memcg, 103%
> > vs 107% on a 256M memcg) but led to ~2.4x more flushes.
> >
> > The initial reason we switched to the ratelimited version of the flush
> > was because flushing was expensive, but I'm not sure if flushing is as
> > expensive as it used to be. Testing the non-ratelimited version, the
> > flushes account for about 0.9% of the system CPU the write path
> > already spends, and I didn't see any throughput difference between the
> > two (249-252 MB/s either way), on a 32G memcg. With the
> > non-ratelimited version, it's also not a flush per read - it's still
> > "ratelimited" by number of updates that have elapsed since the last
> > flush (only flushing if stats_updates >= MEMCG_CHARGE_BATCH * nr_cpus)
> > which means concurrent writers writing back at the same time will
> > "share" flushes rather than triggering their own.
>
> Yup, this is exactly what I mean. This is a good analysis for the cost
> of the flush: how much it matters end-to-end. Even though the full
> flush is 14x more expensive according to your numbers, it seems like
> it ultimately doesn't affect performance much. I would do the same for
> the update paths, see if the extra cost really matters. WDYT?

imo ideally it would be great to minimize any overhead we can where if
we don't need the precision for a certain stat, then it would be
preferable to not incur any extra overhead for it on updates if the
overhead saved on reads doesn't justify it. In the grand scheme of
things though, I think in most cases, the extra overhead/latency isn't
really discernible except at high depths and except for the lock
contention issue mentioned above.

I don't really have an issue with rstat for cases like writeback where
we're not doing a lot of reads compared to updates, but I think we do
need to get rid of the rstat rate-limiting, as it seems so arbitrary
to rely on two seconds for stats being updated. Bounding it by time
and not stats_updates feels wrong. Even in the cases where precision
doesn't matter, it seems better to give those callers a higher stats
updates threshold depending on how much error they can tolerate, than
to rely on a blanket 2 seconds.

>
> >
> > In light of the above, imo it seems to make the most sense to just do
> > the plain non-ratelimited flush for mem_cgroup_wb_stats() and make the
> > flushing magnitude bound again, wh ich also aligns with what the
> > global non-memcg domain does. The question is whether the extra flush
> > cost is acceptable, but the numbers above make me think it is.
>
> If we can eat the cost of the flush here that's great, but I think we
> can't say the same for the vmscan and zswap paths (at least)? :/

I haven't tried out the vmscan repro or run any benchmarks on that
path, but the logic in vmscan seems weird to me, as it will always
conclude that there were no refaults whenever reclaim scans faster
than the refaults counter advances (which I think would be most of the
time under memory pressure). I'm thinking maybe we could do something
similar to what Usama did in [1] where we fold the refault delta into
a per-lruvec accumulator and let it decay. I'm planning to look more
into this and will follow up then.

For the zswap path, it's still a bit unclear to me whether exactness
is necessary or whether the 64 * nr_cpus error tolerance is
acceptable. If that error tolerance is acceptable, I think it makes
sense to use the non-ratelimited version of rstat with no forced
flushing, given that reads are supposed to be rare since rarely
anybody (from what I understand) in practice sets zswap.max. Quick
benchmarks at depth=4 with zswap.max set at every level, 20 workers on
80 cpus:  rstat forced = ~7,600 stores/s, rstat unforced = ~300k
stores/s, and hierarchical per-cpu counters = ~440k stores/s. The
hierarchical per-cpu counters outperforms rstat unforced on the stores
given that it removes the flush, but we already see massive
improvements from using rstat unforced over rstat forced.

Thanks,
Joanne

[1] https://lore.kernel.org/linux-mm/20260727162550.2032-4-usama.arif@linux.dev/

>
> >
> > Thanks,
> > Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

> > > These are the results I'm seeing from the benchmarks. I'm comparing
> > > non-ratelimited rstat vs (a) vs (b). I'm comparing non-ratelimited
> > > rstat instead of ratelimited rstat, because ratelimited rstat relies
> > > on the 2s flusher and doesn't incur any additional flushing when
> > > called.
> > >
> > > ~On the update side~
> > > ns per update (measured by having a kthread pinned to each CPU and all
> > > threads updating the same cgroup for a 200ms window, run on 80 CPUs,
> > > computed as total CPU time / # updates)
> > > depth      rstat      (a)         (b)
> > >     1        22.34       32.49      60.10
> > >     8        22.46       47.12      140.72
> > >     16      22.49       70.96      206.39
> > >     32      22.51       139.16    305.40
> >
> > There goes my dream if an rstat-free world :')
> >
> > I am very surprised though that you care about hierarchies this deep.
> > I don't think we have anything close to even 8 at Google, and in all
> > my previous testing I usually test up to 4 or 5 levels. I suspect at
>
> At Meta, we often see depths 8 to 16.

Interesting :)

> > that level the difference might be acceptable. But aren't other
> > operations also really expensive at that depth? I imagine the charging
> > path is really slow if it has to do atomic updates for 32 parents. Is
>
> I agree. This aligns with what I saw when running the page counter
> approach for zwap on depth 32. For the writeback path though, the
> dirtying / writeback folio lifecycle is just stat updates. There's a
> charge, but that happens only once, when the folio enters the page
> cache.

FWIW, when I added the per-memcg thresholding for flush (i.e.
stats_updates), there were some regressions in some microbenchmarks on
the update side due to the parent iteration in deep hierarchies.
However, we decided the regressions were acceptable because they were
of low significance compared to the extra of charging/uncharging in
those deep hierarchies. As you said, charges are not 1:1 to stat
updates, but it was considered a general indicator of acceptable
performance for the workload at that depth. That is all obviously very
hand wavy :)

> > the update path really a problem given everything else?
> >
> > Given the constant cost of rstat updates, it also seems like
> > memcg_vmstats_needs_flush() is mostly evaluating to true and skipping
> > the parent walk. I wonder how much this remains true in practice, I
> > think Shakeel mentioned that it does help in production.
>
> I think for the writeback path, this is probably often encountered in
> practice, since the threshold is 64 * nr_cpus and any sustained write
> would cross that quickly. It would matter most with order-0 folios
> since there's 4 updates incurred per folio independent of folio size.

Makes sense.

> > > (a) and (b) perform worse on higher memcg depths, whereas rstat is
> > > unaffected by depth level, which makes sense given that (a) and (b)
> > > can't break early if the flushable threshold has already been
> > > exceeded.
> > >
> > > ~On the flush side~
> > > ns per flush call (measured at leaf of memcg chain, ie best scenario
> > > for rstat, # CPUs on the system is 80)
> > >   rstat                      10165.60 ns   (min 9924)
> > >     (a)                       734.10 ns
> > >     (b)                        4.79 ns
> > >
> > > (a) is around 14x cheaper than an rstat flush, which I think is
> > > because the rstat flush goes through every stat, even ones that are
> > > not directly relevant.
> >
> > I think that's only part of the story. rstat iterates all children
> > cgroups as well. I think reading multiple contiguous stats may
> > actually be really cheap since they are likely in the same cacheline.
>
> In that benchmark, the flush cost was measured at the leaf of the
> memcg chain, so I don't think there were any children to iterate here.
> Maybe some of it is attributed to the rstat locking? I'm not too sure.
>
> >
> > >
> > > Using some napkin math, I think the breakeven point for (a) is around:
> > >                       updates per read
> > > depth 1             ~900
> > > depth 8             ~380
> > > depth 32           ~80
> > >
> > > eg for depth 1:  (10165.60 - 734.10) / (32.49 -  22.34)
> > >
> > > This is assuming one read per flush, which is what (a) and (b) would
> > > necessarily have to pay, whereas rstat only needs to flush when the #
> > > of stats updates exceeds the MEMCG_CHARGE_BATCH * nr_cpus threshold,
> > > so the updates per read numbers above are in reality lower.
> > >
> > > Running a fio write job and using tracepoints to count # of stat
> > > updates, # of reads of those stats, and how many of those reads flush,
> > > I'm seeing:
> > >                      updates/s              reads/s
> > > flushes/s         updates per read
> > > bs=1M            643                           119                 45
> > >                        5
> > > bs=4k            156,415                     123                 58
> > >                      1267
> > >
> > > For larger folio sizes, (a) is cheaper for writeback (since there are
> > > 4 update events *per folio* in the dirtying/writeback lifecycle) but
> > > for smaller folio sizes (eg order 0 folios), (a) is worse, as a lot
> > > more updates are incurred, and the performance is worse the deeper the
> > > memcg is nested.
> >
> > Hmm interesting analysis, but I actually think a more interesting one
> > is the effect of the update latency in respective update paths. For
> > example, in the zswap_store() case you analyzed earlier, it ends up
> > being in the noise. I would check what the hottest update paths are
> > (probably networking?) and check what the update paths are, to check
> > if they are really sensitive to the increased update cost. It could be
> > the case that at least stats that are used by in-kernel flushers do
> > not really care about the extra update cost.
>
> Extrapolating a bit from the synthetic benchmarks, at depth 8, I think
> the extra overhead incurred on an update comes out to roughly around
> 25ns, and at depth 16, around 48ns. I haven't looked at the networking
> path, but the lru path (__update_lru_size()) looks concerning, as it
> would have that extra overhead while holding the lru lock, which "is
> already a heavy contention point on memory-heavy workloads" [1]. As I
> understand it, reclaim, compaction, and the page cache path all
> serialize on that lock.

Yeah it could be a problem, but I think we won't know for sure until
we run a reclaim-heavy workload (e.g. build the kernel in a constraint
cgroup) and check for actual regressions. I am often surprised by what
actually shows up (or doesn't) in higher-level workloads.

>
> If we were to move all the callers off rstat, I think this also would
> unfortunately make the walk more expensive. As I understand it, each
> stat would be a per-cpu counter which would be added to the
> memcg_vmstats_percpu struct, and that would have to spill onto
> multiple cachelines. Or maybe I'm missing something here and there
> would be a better way to do it.

Yeah. I count 54 stats, with 8 bytes per counter that's ~7 cachelines
(on x86_64). I can see that being really expensive, but I can also see
it disappear into the noise if the stats updated together live in
close cachelines and the CPU prefetches surrounding cachelines.

> > > I also ran some experiments with Jan's suggestion of scaling the flush
> > > threshold to the memcg's limit rather than to the number of cpus. It
> > > helps the overshoot we saw and slightly outperforms doing a plain
> > > non-ratelimited flush (100% of dirty limit vs 101% on a 1G memcg, 103%
> > > vs 107% on a 256M memcg) but led to ~2.4x more flushes.
> > >
> > > The initial reason we switched to the ratelimited version of the flush
> > > was because flushing was expensive, but I'm not sure if flushing is as
> > > expensive as it used to be. Testing the non-ratelimited version, the
> > > flushes account for about 0.9% of the system CPU the write path
> > > already spends, and I didn't see any throughput difference between the
> > > two (249-252 MB/s either way), on a 32G memcg. With the
> > > non-ratelimited version, it's also not a flush per read - it's still
> > > "ratelimited" by number of updates that have elapsed since the last
> > > flush (only flushing if stats_updates >= MEMCG_CHARGE_BATCH * nr_cpus)
> > > which means concurrent writers writing back at the same time will
> > > "share" flushes rather than triggering their own.
> >
> > Yup, this is exactly what I mean. This is a good analysis for the cost
> > of the flush: how much it matters end-to-end. Even though the full
> > flush is 14x more expensive according to your numbers, it seems like
> > it ultimately doesn't affect performance much. I would do the same for
> > the update paths, see if the extra cost really matters. WDYT?
>
> imo ideally it would be great to minimize any overhead we can where if
> we don't need the precision for a certain stat, then it would be
> preferable to not incur any extra overhead for it on updates if the
> overhead saved on reads doesn't justify it. In the grand scheme of
> things though, I think in most cases, the extra overhead/latency isn't
> really discernible except at high depths and except for the lock
> contention issue mentioned above.

Right. I am just trying to avoid the complexity of having two update
schemes if we can help it. If we can move everything to a new "better"
scheme, that would be great. But it might be the case that we have to
have a different scheme just for stats where the current update/read
tradeoff doesn't make sense.

> I don't really have an issue with rstat for cases like writeback where
> we're not doing a lot of reads compared to updates,

I don't remember why ratelimiting was added, but I suspect for cases
with large trees, high number of CPUs, and/or high cgroup lock
contention -- the cost of a normal flush could get really high. Back
in the day when we used to always flush the entire tree (before
per-memcg thresholding), we ran into hard lockups for flushes taking
>10s with irqs off (we had atomic flushing back then).

>  but I think we do
> need to get rid of the rstat rate-limiting, as it seems so arbitrary
> to rely on two seconds for stats being updated. Bounding it by time
> and not stats_updates feels wrong. Even in the cases where precision
> doesn't matter, it seems better to give those callers a higher stats
> updates threshold depending on how much error they can tolerate, than
> to rely on a blanket 2 seconds.

100% agreed. I never liked the ratelimited flush tbh. I think we
practically end up just not flushing in most cases (but I never really
checked), so it's just a way to feel good about skipping a flush :P

> > > In light of the above, imo it seems to make the most sense to just do
> > > the plain non-ratelimited flush for mem_cgroup_wb_stats() and make the
> > > flushing magnitude bound again, wh ich also aligns with what the
> > > global non-memcg domain does. The question is whether the extra flush
> > > cost is acceptable, but the numbers above make me think it is.
> >
> > If we can eat the cost of the flush here that's great, but I think we
> > can't say the same for the vmscan and zswap paths (at least)? :/
>
> I haven't tried out the vmscan repro or run any benchmarks on that
> path, but the logic in vmscan seems weird to me, as it will always
> conclude that there were no refaults whenever reclaim scans faster
> than the refaults counter advances (which I think would be most of the
> time under memory pressure). I'm thinking maybe we could do something
> similar to what Usama did in [1] where we fold the refault delta into
> a per-lruvec accumulator and let it decay. I'm planning to look more
> into this and will follow up then.

Looking forward to it.

> For the zswap path, it's still a bit unclear to me whether exactness
> is necessary or whether the 64 * nr_cpus error tolerance is
> acceptable. If that error tolerance is acceptable, I think it makes
> sense to use the non-ratelimited version of rstat with no forced
> flushing, given that reads are supposed to be rare since rarely
> anybody (from what I understand) in practice sets zswap.max. Quick
> benchmarks at depth=4 with zswap.max set at every level, 20 workers on
> 80 cpus:  rstat forced = ~7,600 stores/s, rstat unforced = ~300k
> stores/s, and hierarchical per-cpu counters = ~440k stores/s. The
> hierarchical per-cpu counters outperforms rstat unforced on the stores
> given that it removes the flush, but we already see massive
> improvements from using rstat unforced over rstat forced.

I would like to avoid the ratelimited flush for the limit checking for
the same reasons discussed above. I think we were all under the
assumption that the limit is rarely set in practice (other than 0 or
max), until we saw [*].

[*]https://lore.kernel.org/linux-mm/20260817131843.45121-1-husong@kylinos.cn/.

>
> Thanks,
> Joanne
>
> [1] https://lore.kernel.org/linux-mm/20260727162550.2032-4-usama.arif@linux.dev/
>
> >
> > >
> > > Thanks,
> > > Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Fri, Aug 21, 2026 at 6:39 PM James Houghton <jthoughton@google.com> wrote:
>
> On Fri, Aug 21, 2026 at 12:41 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Thu, Aug 20, 2026 at 4:39 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > > Would you be able to share the test you're using? I've been trying to
> > > repro it locally without much luck.
> >
> > The exact test uses some internal testing libraries, we're looking
> > into whether we can translate it to a standalone C repro.
> >
> > From a high-level, the test:
> > - Creates a VM with 260 MB (256 MB for testing + 4 MB margin) in a
> > cgroup, backed by shmem THPs. The VM is not a fully fledged VM, just a
> > KVM VM running bare metal code (so 4 MB is enough margin).
> > - Enables zswap in the cgroup.
> > - Tells the VM to write to 256 MB of its memory.
> > - Checks the cgroup shmem usage and that no swap occurred (no limit).
> > - Sets the cgroup limit to half the VM memory.
> > - Checks the cgroup shmem usage and swap again, usage should be ~half
> > and so is swap.
> > - Tells the VM to read the 256 MB of memory again. This should cause a
> > lot of thrashing and reclaim, but no OOM kills as all guest memory is
> > reclaimable.
> >
> > With the ratelimited flushing, the test started flaking 5-10% of the
> > time due to OOM kills.
>
> Hi Joanne, Yosry,
>
> I've attached a reproducer for you (as a patch that should apply to
> Linus's tree).
>
> In tools/testing/selftests/kvm, `make` should build it. Instructions
> for running it (like setting up swap/etc.) are in the comment at the
> top of the file.
>
> As part of the patch, I've also included a sysctl to dynamically tune
> whether or not the memcg stats flush is rate-limited. You should find
> that when it is rate-limited, OOMs occur, and when it is not, OOMs do
> not occur.
>
> This reproducer behaves slightly differently than our internal
> version. This version, when run with `-b 512M` anyway, seems to OOM
> simply by setting memory.max, whereas our internal version would OOM
> upon re-accessing the memory. Not really a meaningful difference, but
> just wanted to point it out.

Thanks again for the repro, James.

I added some instrumentation to prepare_scan_control() and have been
seeing that the OOM gets triggered in this pathway:
a) the memcg is created with memory.max set to unlimited. 512 M gets
faulted in. The new anon folios get put on the inactive list. Nothing
reclaims them yet
b) userspace then sets memory.max to half of memory.current (~271M
from ~542 M currently used). This causes memory_max_write() to trigger
reclaim logic
c) the folios have already moved to the active list (my
instrumentation shows 122684 active vs. 179 inactive) when reclaim
does the scanning, which means the only way for pages to be reclaimed
is for pages to get deactivated from the active list back to the
inactive list.
d) prepare_scan_control() (which decides if the pages get deactivated)
reads the LRU sizes from rstat. This memcg hasn't been flushed yet
(the 2s flusher hasn't yet triggered since when the memcg was
created), so the NR_INACTIVE_ANON and NR_ACTIVE_ANON stats are all
still zero. The real values are all still sitting in per-cpu buffers
that haven't yet been flushed. My instrumentation shows:
        inactive_anon       rstat: 0    actual: 179
        active_anon          rstat: 0    actual: 122684  (~479 MB)
e) the deactivation logic in prepare_scan_control() is:

                refaults = lruvec_page_state(target_lruvec,
                                WORKINGSET_ACTIVATE_ANON);
                if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
                        inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
                        sc->may_deactivate |= DEACTIVATE_ANON;
                else
                        sc->may_deactivate &= ~DEACTIVATE_ANON;

The if check fails:
     refaults != prev refaults: 0 != 0      (false)
     inactive_is_low(): both LRU inactive and active values are 0: 0 *
inactive_ratio < 0     ( false)

DEACTIVATE_ANON gets cleared

f) pages don't get moved from the active list to the inactive list, so
reclaim ends up freeing nothing. memory_max_write() goes through all
MAX_RECLAIM_RETRIES and OOMs, even though there's 479 MB reclaimable
(but it's all on the active list).

I saw that the skipped_deactivate retry did fire (on 858 of 4264
scans), but not enough to get the memory.current back under the halved
memory.max, so memory_max_write() still exhausted MAX_RECLAIM_RETRIES
and OOMed.


imo I think the best fix would be
a) for non-root memcgs, revert back to the non-ratelimited version
(mem_cgroup_flush_stats())
b) for root-memcgs (which if flushed will flush every cgroup on the
machine), don't use rstat at all and use the existing node counters
instead (which is what the non-memcg case already uses).
mod_lruvec_state() always updates the node vmstat and the memcg lruvec
stat in the same call. For the root lruvec, the node counter reflects
the same value and doesn't need any flushing. It has an error bound of
125 pages/cpu at most, but this is tighter than what we currently have
with the ratelimited version.

commit 90a6f2a8f442 ("memcg: use ratelimited stats flush in the
reclaim") was what switched vmscan to using ratelimited flushing. The
reason for this was to fix the stalls from concurrent reclaimers
contending on the rstat lock. For the Cloudflare case, kswapd threads
were contending on the rstat lock on 8 numa-node machines. That's the
global path (eg sc->target_mem_cgroup is NULL,
mem_cgroup_flush_stats() turns it into root, so this flush essentially
ends up flushing every cgroup on the machine), which gets addressed by
adding the fix in b), which takes kswapd and global direct reclaim off
rstat entirely. For the non-root memcg case, the flushing is bounded
by the MEMCG_CHARGE_BATCH * num_online_cpus() threshold (a non-root
memcg's stats_updates only accumulates its own subtree's updates, so
it takes a lot longer for it to be exceeded than the root's does).

I ran some benchmarks to get a rough idea of what a) costs. I set up a
workload that allocates 512M of anon memory in a memcg with memory.max
set to half of that to trigger reclaim continuously. I varied the
number of threads touching the memory, so that N of them hit
memory.max and enter direct reclaim on the same memcg at once. I used
bpftrace to measure the time spent inside css_rstat_flush() and
shrink_node() and used those two numbers to get a percentage for how
much of reclaim is spent in flushing. These are the numbers I saw:

# threads      flush as % of shrink_node()
1                ratelimited: 1.10%       non-ratelimited: 2.22%
16              ratelimited: 0.36%       non-ratelimited: 2.87%
64              ratelimited: 0.16%       non-ratelimited: 2.52%

Between the ratelimited and the non-ratelimited runs, walltime and
pgsteal were pretty much indistinguishable (42.7s vs 43.6s at 16
threads and 41.7 s vs 42.4s at 64, with the ranges overlapping, and
pgsteal within 0.2%).

I also measured the lock contention. At 16 and 64 threads, 15-20% of
__css_rstat_lock() acquisitions hit a contended trylock with the
non-ratelimited version. At 16 threads, the mean flush time rises
about 8% and at 64 it roughly triples from the single-thread case
(26us at 1 thread, 42 us at 16, 87us at 64)  but I didn't see any
effect of this on wall time or pgsteal between the non-ratelimited and
the ratelimited version, and the percentage of reclaim time spent
flushing stayed at 2-3%. I think that's because css_rstat_flush()
takes and drops the lock once per cpu rather than holding it across
the walk.

I also ran it with 32 actively churning sub-cgroups under the reclaim
target, since css_rstat_flush() walks the descendants that have
pending updates. I saw that per-flush cost rises slightly (~24 us to
27 us) but the share of reclaim time spent flushing went down rather
than up (4.1% to 3.2%), because reclaim gets more expensive faster
than flushing does. I don't have a ratelimited baseline at 32 children
though, because with the ratelimited version, all the runs I tried
OOMed. I think this is an alternative repro to the OOM issue but
doesn't need/have KVM dependencies, so if this would be useful to
clean up and send as a standalone C repro, please let me know.

(Also btw (this is a bit tangential to this OOM issue, but somewhat
semi-related), I don't think the WORKINGSET_ACTIVATE_{ANON,FILE} stats
checking really does anything in practice. On my benchmark runs, the
"refaults != prev_refaults" check evaluates to false in pretty much
every scan. Even with flushing working properly, it didn't trigger on
1673718 of 1674912 samples, even though the cgroup was thrashing the
whole time (a separate run of the same workload counted 6371860 anon
refaults vs 6523 activations). I think it's because reclaim is
scanning faster than the refaults counter advances, so it'll almost
always see no change. I'm not sure how much this matters in practice,
but I think effectively in that check above in prepare_scan_control(),
deactivation relies entirely on the inactive_is_low() check.)

What do you all think about adding a) and b) as the fix for this issue?

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[James Houghton]

On Mon, Sep 14, 2026 at 12:01 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Fri, Aug 21, 2026 at 6:39 PM James Houghton <jthoughton@google.com> wrote:
> >
> > On Fri, Aug 21, 2026 at 12:41 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
> > > On Thu, Aug 20, 2026 at 4:39 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > > > Would you be able to share the test you're using? I've been trying to
> > > > repro it locally without much luck.
> > >
> > > The exact test uses some internal testing libraries, we're looking
> > > into whether we can translate it to a standalone C repro.
> > >
> > > From a high-level, the test:
> > > - Creates a VM with 260 MB (256 MB for testing + 4 MB margin) in a
> > > cgroup, backed by shmem THPs. The VM is not a fully fledged VM, just a
> > > KVM VM running bare metal code (so 4 MB is enough margin).
> > > - Enables zswap in the cgroup.
> > > - Tells the VM to write to 256 MB of its memory.
> > > - Checks the cgroup shmem usage and that no swap occurred (no limit).
> > > - Sets the cgroup limit to half the VM memory.
> > > - Checks the cgroup shmem usage and swap again, usage should be ~half
> > > and so is swap.
> > > - Tells the VM to read the 256 MB of memory again. This should cause a
> > > lot of thrashing and reclaim, but no OOM kills as all guest memory is
> > > reclaimable.
> > >
> > > With the ratelimited flushing, the test started flaking 5-10% of the
> > > time due to OOM kills.
> >
> > Hi Joanne, Yosry,
> >
> > I've attached a reproducer for you (as a patch that should apply to
> > Linus's tree).
> >
> > In tools/testing/selftests/kvm, `make` should build it. Instructions
> > for running it (like setting up swap/etc.) are in the comment at the
> > top of the file.
> >
> > As part of the patch, I've also included a sysctl to dynamically tune
> > whether or not the memcg stats flush is rate-limited. You should find
> > that when it is rate-limited, OOMs occur, and when it is not, OOMs do
> > not occur.
> >
> > This reproducer behaves slightly differently than our internal
> > version. This version, when run with `-b 512M` anyway, seems to OOM
> > simply by setting memory.max, whereas our internal version would OOM
> > upon re-accessing the memory. Not really a meaningful difference, but
> > just wanted to point it out.
>
> Thanks again for the repro, James.
>
> I added some instrumentation to prepare_scan_control() and have been
> seeing that the OOM gets triggered in this pathway:
> a) the memcg is created with memory.max set to unlimited. 512 M gets
> faulted in. The new anon folios get put on the inactive list. Nothing
> reclaims them yet
> b) userspace then sets memory.max to half of memory.current (~271M
> from ~542 M currently used). This causes memory_max_write() to trigger
> reclaim logic
> c) the folios have already moved to the active list (my
> instrumentation shows 122684 active vs. 179 inactive) when reclaim
> does the scanning, which means the only way for pages to be reclaimed
> is for pages to get deactivated from the active list back to the
> inactive list.
> d) prepare_scan_control() (which decides if the pages get deactivated)
> reads the LRU sizes from rstat. This memcg hasn't been flushed yet
> (the 2s flusher hasn't yet triggered since when the memcg was
> created), so the NR_INACTIVE_ANON and NR_ACTIVE_ANON stats are all
> still zero. The real values are all still sitting in per-cpu buffers
> that haven't yet been flushed. My instrumentation shows:
>         inactive_anon       rstat: 0    actual: 179
>         active_anon          rstat: 0    actual: 122684  (~479 MB)
> e) the deactivation logic in prepare_scan_control() is:
>
>                 refaults = lruvec_page_state(target_lruvec,
>                                 WORKINGSET_ACTIVATE_ANON);
>                 if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
>                         inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
>                         sc->may_deactivate |= DEACTIVATE_ANON;
>                 else
>                         sc->may_deactivate &= ~DEACTIVATE_ANON;
>
> The if check fails:
>      refaults != prev refaults: 0 != 0      (false)
>      inactive_is_low(): both LRU inactive and active values are 0: 0 *
> inactive_ratio < 0     ( false)
>
> DEACTIVATE_ANON gets cleared
>
> f) pages don't get moved from the active list to the inactive list, so
> reclaim ends up freeing nothing. memory_max_write() goes through all
> MAX_RECLAIM_RETRIES and OOMs, even though there's 479 MB reclaimable
> (but it's all on the active list).
>
> I saw that the skipped_deactivate retry did fire (on 858 of 4264
> scans), but not enough to get the memory.current back under the halved
> memory.max, so memory_max_write() still exhausted MAX_RECLAIM_RETRIES
> and OOMed.

Great! Thanks for figuring that out.

>
> imo I think the best fix would be
> a) for non-root memcgs, revert back to the non-ratelimited version
> (mem_cgroup_flush_stats())
> b) for root-memcgs (which if flushed will flush every cgroup on the
> machine), don't use rstat at all and use the existing node counters
> instead (which is what the non-memcg case already uses).
> mod_lruvec_state() always updates the node vmstat and the memcg lruvec
> stat in the same call. For the root lruvec, the node counter reflects
> the same value and doesn't need any flushing. It has an error bound of
> 125 pages/cpu at most, but this is tighter than what we currently have
> with the ratelimited version.
>
> commit 90a6f2a8f442 ("memcg: use ratelimited stats flush in the
> reclaim") was what switched vmscan to using ratelimited flushing. The
> reason for this was to fix the stalls from concurrent reclaimers
> contending on the rstat lock. For the Cloudflare case, kswapd threads
> were contending on the rstat lock on 8 numa-node machines. That's the
> global path (eg sc->target_mem_cgroup is NULL,
> mem_cgroup_flush_stats() turns it into root, so this flush essentially
> ends up flushing every cgroup on the machine), which gets addressed by
> adding the fix in b), which takes kswapd and global direct reclaim off
> rstat entirely. For the non-root memcg case, the flushing is bounded
> by the MEMCG_CHARGE_BATCH * num_online_cpus() threshold (a non-root
> memcg's stats_updates only accumulates its own subtree's updates, so
> it takes a lot longer for it to be exceeded than the root's does).
>
> I ran some benchmarks to get a rough idea of what a) costs. I set up a
> workload that allocates 512M of anon memory in a memcg with memory.max
> set to half of that to trigger reclaim continuously. I varied the
> number of threads touching the memory, so that N of them hit
> memory.max and enter direct reclaim on the same memcg at once. I used
> bpftrace to measure the time spent inside css_rstat_flush() and
> shrink_node() and used those two numbers to get a percentage for how
> much of reclaim is spent in flushing. These are the numbers I saw:
>
> # threads      flush as % of shrink_node()
> 1                ratelimited: 1.10%       non-ratelimited: 2.22%
> 16              ratelimited: 0.36%       non-ratelimited: 2.87%
> 64              ratelimited: 0.16%       non-ratelimited: 2.52%
>
> Between the ratelimited and the non-ratelimited runs, walltime and
> pgsteal were pretty much indistinguishable (42.7s vs 43.6s at 16
> threads and 41.7 s vs 42.4s at 64, with the ranges overlapping, and
> pgsteal within 0.2%).
>
> I also measured the lock contention. At 16 and 64 threads, 15-20% of
> __css_rstat_lock() acquisitions hit a contended trylock with the
> non-ratelimited version. At 16 threads, the mean flush time rises
> about 8% and at 64 it roughly triples from the single-thread case
> (26us at 1 thread, 42 us at 16, 87us at 64)  but I didn't see any
> effect of this on wall time or pgsteal between the non-ratelimited and
> the ratelimited version, and the percentage of reclaim time spent
> flushing stayed at 2-3%. I think that's because css_rstat_flush()
> takes and drops the lock once per cpu rather than holding it across
> the walk.
>
> I also ran it with 32 actively churning sub-cgroups under the reclaim
> target, since css_rstat_flush() walks the descendants that have
> pending updates. I saw that per-flush cost rises slightly (~24 us to
> 27 us) but the share of reclaim time spent flushing went down rather
> than up (4.1% to 3.2%), because reclaim gets more expensive faster
> than flushing does. I don't have a ratelimited baseline at 32 children
> though, because with the ratelimited version, all the runs I tried
> OOMed. I think this is an alternative repro to the OOM issue but
> doesn't need/have KVM dependencies, so if this would be useful to
> clean up and send as a standalone C repro, please let me know.

It will probably be useful to send with the (a) and (b) patches. If
it's not very clean, just including it for demonstration is probably
fine (doesn't have to be merged).

> (Also btw (this is a bit tangential to this OOM issue, but somewhat
> semi-related), I don't think the WORKINGSET_ACTIVATE_{ANON,FILE} stats
> checking really does anything in practice. On my benchmark runs, the
> "refaults != prev_refaults" check evaluates to false in pretty much
> every scan. Even with flushing working properly, it didn't trigger on
> 1673718 of 1674912 samples, even though the cgroup was thrashing the
> whole time (a separate run of the same workload counted 6371860 anon
> refaults vs 6523 activations). I think it's because reclaim is
> scanning faster than the refaults counter advances, so it'll almost
> always see no change. I'm not sure how much this matters in practice,
> but I think effectively in that check above in prepare_scan_control(),
> deactivation relies entirely on the inactive_is_low() check.)

Interesting finding. Maybe Yosry will have something intelligent to
say about it; I don't. Yosry will be back on Oct 6.

>
> What do you all think about adding a) and b) as the fix for this issue?

It seems like the right thing to do to me, but I'm not really an
expert on reclaim. I'm guessing Shakeel is happy with this too? I'll
remind Yosry about this thread when he gets back on the 6th so he can
give his opinion.

>
> Thanks,
> Joanne

Thank you very much Joanne, sorry for the delay getting back to you.

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

On Tue, Sep 22, 2026 at 11:28 AM James Houghton <jthoughton@google.com> wrote:
>
> On Mon, Sep 14, 2026 at 12:01 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> >
> > On Fri, Aug 21, 2026 at 6:39 PM James Houghton <jthoughton@google.com> wrote:
> > >
> > > On Fri, Aug 21, 2026 at 12:41 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > > >
> > > > On Thu, Aug 20, 2026 at 4:39 PM Joanne Koong <joannelkoong@gmail.com> wrote:
> > > > > Would you be able to share the test you're using? I've been trying to
> > > > > repro it locally without much luck.
> > > >
> > > > The exact test uses some internal testing libraries, we're looking
> > > > into whether we can translate it to a standalone C repro.
> > > >
> > > > From a high-level, the test:
> > > > - Creates a VM with 260 MB (256 MB for testing + 4 MB margin) in a
> > > > cgroup, backed by shmem THPs. The VM is not a fully fledged VM, just a
> > > > KVM VM running bare metal code (so 4 MB is enough margin).
> > > > - Enables zswap in the cgroup.
> > > > - Tells the VM to write to 256 MB of its memory.
> > > > - Checks the cgroup shmem usage and that no swap occurred (no limit).
> > > > - Sets the cgroup limit to half the VM memory.
> > > > - Checks the cgroup shmem usage and swap again, usage should be ~half
> > > > and so is swap.
> > > > - Tells the VM to read the 256 MB of memory again. This should cause a
> > > > lot of thrashing and reclaim, but no OOM kills as all guest memory is
> > > > reclaimable.
> > > >
> > > > With the ratelimited flushing, the test started flaking 5-10% of the
> > > > time due to OOM kills.
> > >
> > > Hi Joanne, Yosry,
> > >
> > > I've attached a reproducer for you (as a patch that should apply to
> > > Linus's tree).
> > >
> > > In tools/testing/selftests/kvm, `make` should build it. Instructions
> > > for running it (like setting up swap/etc.) are in the comment at the
> > > top of the file.
> > >
> > > As part of the patch, I've also included a sysctl to dynamically tune
> > > whether or not the memcg stats flush is rate-limited. You should find
> > > that when it is rate-limited, OOMs occur, and when it is not, OOMs do
> > > not occur.
> > >
> > > This reproducer behaves slightly differently than our internal
> > > version. This version, when run with `-b 512M` anyway, seems to OOM
> > > simply by setting memory.max, whereas our internal version would OOM
> > > upon re-accessing the memory. Not really a meaningful difference, but
> > > just wanted to point it out.
> >
> > Thanks again for the repro, James.
> >
> > I added some instrumentation to prepare_scan_control() and have been
> > seeing that the OOM gets triggered in this pathway:
> > a) the memcg is created with memory.max set to unlimited. 512 M gets
> > faulted in. The new anon folios get put on the inactive list. Nothing
> > reclaims them yet
> > b) userspace then sets memory.max to half of memory.current (~271M
> > from ~542 M currently used). This causes memory_max_write() to trigger
> > reclaim logic
> > c) the folios have already moved to the active list (my
> > instrumentation shows 122684 active vs. 179 inactive) when reclaim
> > does the scanning, which means the only way for pages to be reclaimed
> > is for pages to get deactivated from the active list back to the
> > inactive list.
> > d) prepare_scan_control() (which decides if the pages get deactivated)
> > reads the LRU sizes from rstat. This memcg hasn't been flushed yet
> > (the 2s flusher hasn't yet triggered since when the memcg was
> > created), so the NR_INACTIVE_ANON and NR_ACTIVE_ANON stats are all
> > still zero. The real values are all still sitting in per-cpu buffers
> > that haven't yet been flushed. My instrumentation shows:
> >         inactive_anon       rstat: 0    actual: 179
> >         active_anon          rstat: 0    actual: 122684  (~479 MB)
> > e) the deactivation logic in prepare_scan_control() is:
> >
> >                 refaults = lruvec_page_state(target_lruvec,
> >                                 WORKINGSET_ACTIVATE_ANON);
> >                 if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
> >                         inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
> >                         sc->may_deactivate |= DEACTIVATE_ANON;
> >                 else
> >                         sc->may_deactivate &= ~DEACTIVATE_ANON;
> >
> > The if check fails:
> >      refaults != prev refaults: 0 != 0      (false)
> >      inactive_is_low(): both LRU inactive and active values are 0: 0 *
> > inactive_ratio < 0     ( false)
> >
> > DEACTIVATE_ANON gets cleared
> >
> > f) pages don't get moved from the active list to the inactive list, so
> > reclaim ends up freeing nothing. memory_max_write() goes through all
> > MAX_RECLAIM_RETRIES and OOMs, even though there's 479 MB reclaimable
> > (but it's all on the active list).
> >
> > I saw that the skipped_deactivate retry did fire (on 858 of 4264
> > scans), but not enough to get the memory.current back under the halved
> > memory.max, so memory_max_write() still exhausted MAX_RECLAIM_RETRIES
> > and OOMed.
>
> Great! Thanks for figuring that out.
>
> >
> > imo I think the best fix would be
> > a) for non-root memcgs, revert back to the non-ratelimited version
> > (mem_cgroup_flush_stats())
> > b) for root-memcgs (which if flushed will flush every cgroup on the
> > machine), don't use rstat at all and use the existing node counters
> > instead (which is what the non-memcg case already uses).
> > mod_lruvec_state() always updates the node vmstat and the memcg lruvec
> > stat in the same call. For the root lruvec, the node counter reflects
> > the same value and doesn't need any flushing. It has an error bound of
> > 125 pages/cpu at most, but this is tighter than what we currently have
> > with the ratelimited version.
> >
> > commit 90a6f2a8f442 ("memcg: use ratelimited stats flush in the
> > reclaim") was what switched vmscan to using ratelimited flushing. The
> > reason for this was to fix the stalls from concurrent reclaimers
> > contending on the rstat lock. For the Cloudflare case, kswapd threads
> > were contending on the rstat lock on 8 numa-node machines. That's the
> > global path (eg sc->target_mem_cgroup is NULL,
> > mem_cgroup_flush_stats() turns it into root, so this flush essentially
> > ends up flushing every cgroup on the machine), which gets addressed by
> > adding the fix in b), which takes kswapd and global direct reclaim off
> > rstat entirely. For the non-root memcg case, the flushing is bounded
> > by the MEMCG_CHARGE_BATCH * num_online_cpus() threshold (a non-root
> > memcg's stats_updates only accumulates its own subtree's updates, so
> > it takes a lot longer for it to be exceeded than the root's does).
> >
> > I ran some benchmarks to get a rough idea of what a) costs. I set up a
> > workload that allocates 512M of anon memory in a memcg with memory.max
> > set to half of that to trigger reclaim continuously. I varied the
> > number of threads touching the memory, so that N of them hit
> > memory.max and enter direct reclaim on the same memcg at once. I used
> > bpftrace to measure the time spent inside css_rstat_flush() and
> > shrink_node() and used those two numbers to get a percentage for how
> > much of reclaim is spent in flushing. These are the numbers I saw:
> >
> > # threads      flush as % of shrink_node()
> > 1                ratelimited: 1.10%       non-ratelimited: 2.22%
> > 16              ratelimited: 0.36%       non-ratelimited: 2.87%
> > 64              ratelimited: 0.16%       non-ratelimited: 2.52%
> >
> > Between the ratelimited and the non-ratelimited runs, walltime and
> > pgsteal were pretty much indistinguishable (42.7s vs 43.6s at 16
> > threads and 41.7 s vs 42.4s at 64, with the ranges overlapping, and
> > pgsteal within 0.2%).
> >
> > I also measured the lock contention. At 16 and 64 threads, 15-20% of
> > __css_rstat_lock() acquisitions hit a contended trylock with the
> > non-ratelimited version. At 16 threads, the mean flush time rises
> > about 8% and at 64 it roughly triples from the single-thread case
> > (26us at 1 thread, 42 us at 16, 87us at 64)  but I didn't see any
> > effect of this on wall time or pgsteal between the non-ratelimited and
> > the ratelimited version, and the percentage of reclaim time spent
> > flushing stayed at 2-3%. I think that's because css_rstat_flush()
> > takes and drops the lock once per cpu rather than holding it across
> > the walk.
> >
> > I also ran it with 32 actively churning sub-cgroups under the reclaim
> > target, since css_rstat_flush() walks the descendants that have
> > pending updates. I saw that per-flush cost rises slightly (~24 us to
> > 27 us) but the share of reclaim time spent flushing went down rather
> > than up (4.1% to 3.2%), because reclaim gets more expensive faster
> > than flushing does. I don't have a ratelimited baseline at 32 children
> > though, because with the ratelimited version, all the runs I tried
> > OOMed. I think this is an alternative repro to the OOM issue but
> > doesn't need/have KVM dependencies, so if this would be useful to
> > clean up and send as a standalone C repro, please let me know.
>
> It will probably be useful to send with the (a) and (b) patches. If
> it's not very clean, just including it for demonstration is probably
> fine (doesn't have to be merged).

Great! I will clean it up and attach it to the patches.

>
> > (Also btw (this is a bit tangential to this OOM issue, but somewhat
> > semi-related), I don't think the WORKINGSET_ACTIVATE_{ANON,FILE} stats
> > checking really does anything in practice. On my benchmark runs, the
> > "refaults != prev_refaults" check evaluates to false in pretty much
> > every scan. Even with flushing working properly, it didn't trigger on
> > 1673718 of 1674912 samples, even though the cgroup was thrashing the
> > whole time (a separate run of the same workload counted 6371860 anon
> > refaults vs 6523 activations). I think it's because reclaim is
> > scanning faster than the refaults counter advances, so it'll almost
> > always see no change. I'm not sure how much this matters in practice,
> > but I think effectively in that check above in prepare_scan_control(),
> > deactivation relies entirely on the inactive_is_low() check.)
>
> Interesting finding. Maybe Yosry will have something intelligent to
> say about it; I don't. Yosry will be back on Oct 6.
>
> >
> > What do you all think about adding a) and b) as the fix for this issue?
>
> It seems like the right thing to do to me, but I'm not really an
> expert on reclaim. I'm guessing Shakeel is happy with this too? I'll
> remind Yosry about this thread when he gets back on the 6th so he can
> give his opinion.
>

Thanks, James! I will hold tight until the second week of October then
:) Thanks for the update on this - I appreciate it!

> >
> > Thanks,
> > Joanne
>
> Thank you very much Joanne, sorry for the delay getting back to you.

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

> I added some instrumentation to prepare_scan_control() and have been
> seeing that the OOM gets triggered in this pathway:
> a) the memcg is created with memory.max set to unlimited. 512 M gets
> faulted in. The new anon folios get put on the inactive list. Nothing
> reclaims them yet
> b) userspace then sets memory.max to half of memory.current (~271M
> from ~542 M currently used). This causes memory_max_write() to trigger
> reclaim logic
> c) the folios have already moved to the active list (my
> instrumentation shows 122684 active vs. 179 inactive) when reclaim
> does the scanning, which means the only way for pages to be reclaimed
> is for pages to get deactivated from the active list back to the
> inactive list.
> d) prepare_scan_control() (which decides if the pages get deactivated)
> reads the LRU sizes from rstat. This memcg hasn't been flushed yet
> (the 2s flusher hasn't yet triggered since when the memcg was
> created), so the NR_INACTIVE_ANON and NR_ACTIVE_ANON stats are all
> still zero. The real values are all still sitting in per-cpu buffers
> that haven't yet been flushed. My instrumentation shows:
>         inactive_anon       rstat: 0    actual: 179
>         active_anon          rstat: 0    actual: 122684  (~479 MB)
> e) the deactivation logic in prepare_scan_control() is:
>
>                 refaults = lruvec_page_state(target_lruvec,
>                                 WORKINGSET_ACTIVATE_ANON);
>                 if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
>                         inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
>                         sc->may_deactivate |= DEACTIVATE_ANON;
>                 else
>                         sc->may_deactivate &= ~DEACTIVATE_ANON;
>
> The if check fails:
>      refaults != prev refaults: 0 != 0      (false)
>      inactive_is_low(): both LRU inactive and active values are 0: 0 *
> inactive_ratio < 0     ( false)
>
> DEACTIVATE_ANON gets cleared
>
> f) pages don't get moved from the active list to the inactive list, so
> reclaim ends up freeing nothing. memory_max_write() goes through all
> MAX_RECLAIM_RETRIES and OOMs, even though there's 479 MB reclaimable
> (but it's all on the active list).
>
> I saw that the skipped_deactivate retry did fire (on 858 of 4264
> scans), but not enough to get the memory.current back under the halved
> memory.max, so memory_max_write() still exhausted MAX_RECLAIM_RETRIES
> and OOMed.
>
>
> imo I think the best fix would be
> a) for non-root memcgs, revert back to the non-ratelimited version
> (mem_cgroup_flush_stats())
> b) for root-memcgs (which if flushed will flush every cgroup on the
> machine), don't use rstat at all and use the existing node counters
> instead (which is what the non-memcg case already uses).
> mod_lruvec_state() always updates the node vmstat and the memcg lruvec
> stat in the same call. For the root lruvec, the node counter reflects
> the same value and doesn't need any flushing. It has an error bound of
> 125 pages/cpu at most, but this is tighter than what we currently have
> with the ratelimited version.

This seems like the right thing to do. If we have global stats, we
should avoid a rstat flush. I actually wonder if we should start
systematically eliminating root flushes, and if that ends up being
good enough to keep everything else as-is for in-kernel flushers. The
flush in zswap_shrinker_count() is also a good candidate, we already
use global stats when memcg is disabled there as well.

>
> commit 90a6f2a8f442 ("memcg: use ratelimited stats flush in the
> reclaim") was what switched vmscan to using ratelimited flushing. The
> reason for this was to fix the stalls from concurrent reclaimers
> contending on the rstat lock. For the Cloudflare case, kswapd threads
> were contending on the rstat lock on 8 numa-node machines. That's the
> global path (eg sc->target_mem_cgroup is NULL,
> mem_cgroup_flush_stats() turns it into root, so this flush essentially
> ends up flushing every cgroup on the machine), which gets addressed by
> adding the fix in b), which takes kswapd and global direct reclaim off
> rstat entirely. For the non-root memcg case, the flushing is bounded
> by the MEMCG_CHARGE_BATCH * num_online_cpus() threshold (a non-root
> memcg's stats_updates only accumulates its own subtree's updates, so
> it takes a lot longer for it to be exceeded than the root's does).
>
> I ran some benchmarks to get a rough idea of what a) costs. I set up a
> workload that allocates 512M of anon memory in a memcg with memory.max
> set to half of that to trigger reclaim continuously. I varied the
> number of threads touching the memory, so that N of them hit
> memory.max and enter direct reclaim on the same memcg at once. I used
> bpftrace to measure the time spent inside css_rstat_flush() and
> shrink_node() and used those two numbers to get a percentage for how
> much of reclaim is spent in flushing. These are the numbers I saw:
>
> # threads      flush as % of shrink_node()
> 1                ratelimited: 1.10%       non-ratelimited: 2.22%
> 16              ratelimited: 0.36%       non-ratelimited: 2.87%
> 64              ratelimited: 0.16%       non-ratelimited: 2.52%
>
> Between the ratelimited and the non-ratelimited runs, walltime and
> pgsteal were pretty much indistinguishable (42.7s vs 43.6s at 16
> threads and 41.7 s vs 42.4s at 64, with the ranges overlapping, and
> pgsteal within 0.2%).
>
> I also measured the lock contention. At 16 and 64 threads, 15-20% of
> __css_rstat_lock() acquisitions hit a contended trylock with the
> non-ratelimited version. At 16 threads, the mean flush time rises
> about 8% and at 64 it roughly triples from the single-thread case
> (26us at 1 thread, 42 us at 16, 87us at 64)  but I didn't see any
> effect of this on wall time or pgsteal between the non-ratelimited and
> the ratelimited version, and the percentage of reclaim time spent
> flushing stayed at 2-3%. I think that's because css_rstat_flush()
> takes and drops the lock once per cpu rather than holding it across
> the walk.
>
> I also ran it with 32 actively churning sub-cgroups under the reclaim
> target, since css_rstat_flush() walks the descendants that have
> pending updates. I saw that per-flush cost rises slightly (~24 us to
> 27 us) but the share of reclaim time spent flushing went down rather
> than up (4.1% to 3.2%), because reclaim gets more expensive faster
> than flushing does. I don't have a ratelimited baseline at 32 children
> though, because with the ratelimited version, all the runs I tried
> OOMed. I think this is an alternative repro to the OOM issue but
> doesn't need/have KVM dependencies, so if this would be useful to
> clean up and send as a standalone C repro, please let me know.

It might be useful if it can be added as a selftest or similar, so
that such regressions can be caught more easily in the future?

>
> (Also btw (this is a bit tangential to this OOM issue, but somewhat
> semi-related), I don't think the WORKINGSET_ACTIVATE_{ANON,FILE} stats
> checking really does anything in practice. On my benchmark runs, the
> "refaults != prev_refaults" check evaluates to false in pretty much
> every scan. Even with flushing working properly, it didn't trigger on
> 1673718 of 1674912 samples, even though the cgroup was thrashing the
> whole time (a separate run of the same workload counted 6371860 anon
> refaults vs 6523 activations). I think it's because reclaim is
> scanning faster than the refaults counter advances, so it'll almost
> always see no change. I'm not sure how much this matters in practice,
> but I think effectively in that check above in prepare_scan_control(),
> deactivation relies entirely on the inactive_is_low() check.)

I am also not sure about this one. I am not sure if reclaim moving
faster than refaults is something universal, it seems like it would be
workload-specific. Johannes is the person that comes to mind when I
think about these heuristics, so maybe he has an opinion about this?
:)

>
> What do you all think about adding a) and b) as the fix for this issue?

I generally agree. Doesn't really matter much, but I think we should
do (b) then (a) to avoid reintroducing the regression temporarily?

>
> Thanks,
> Joanne
>

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Joanne Koong]

Hi Yosry,

On Mon, Oct 5, 2026 at 1:45 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> > I added some instrumentation to prepare_scan_control() and have been
> > seeing that the OOM gets triggered in this pathway:
> > a) the memcg is created with memory.max set to unlimited. 512 M gets
> > faulted in. The new anon folios get put on the inactive list. Nothing
> > reclaims them yet
> > b) userspace then sets memory.max to half of memory.current (~271M
> > from ~542 M currently used). This causes memory_max_write() to trigger
> > reclaim logic
> > c) the folios have already moved to the active list (my
> > instrumentation shows 122684 active vs. 179 inactive) when reclaim
> > does the scanning, which means the only way for pages to be reclaimed
> > is for pages to get deactivated from the active list back to the
> > inactive list.
> > d) prepare_scan_control() (which decides if the pages get deactivated)
> > reads the LRU sizes from rstat. This memcg hasn't been flushed yet
> > (the 2s flusher hasn't yet triggered since when the memcg was
> > created), so the NR_INACTIVE_ANON and NR_ACTIVE_ANON stats are all
> > still zero. The real values are all still sitting in per-cpu buffers
> > that haven't yet been flushed. My instrumentation shows:
> >         inactive_anon       rstat: 0    actual: 179
> >         active_anon          rstat: 0    actual: 122684  (~479 MB)
> > e) the deactivation logic in prepare_scan_control() is:
> >
> >                 refaults = lruvec_page_state(target_lruvec,
> >                                 WORKINGSET_ACTIVATE_ANON);
> >                 if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
> >                         inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
> >                         sc->may_deactivate |= DEACTIVATE_ANON;
> >                 else
> >                         sc->may_deactivate &= ~DEACTIVATE_ANON;
> >
> > The if check fails:
> >      refaults != prev refaults: 0 != 0      (false)
> >      inactive_is_low(): both LRU inactive and active values are 0: 0 *
> > inactive_ratio < 0     ( false)
> >
> > DEACTIVATE_ANON gets cleared
> >
> > f) pages don't get moved from the active list to the inactive list, so
> > reclaim ends up freeing nothing. memory_max_write() goes through all
> > MAX_RECLAIM_RETRIES and OOMs, even though there's 479 MB reclaimable
> > (but it's all on the active list).
> >
> > I saw that the skipped_deactivate retry did fire (on 858 of 4264
> > scans), but not enough to get the memory.current back under the halved
> > memory.max, so memory_max_write() still exhausted MAX_RECLAIM_RETRIES
> > and OOMed.
> >
> >
> > imo I think the best fix would be
> > a) for non-root memcgs, revert back to the non-ratelimited version
> > (mem_cgroup_flush_stats())
> > b) for root-memcgs (which if flushed will flush every cgroup on the
> > machine), don't use rstat at all and use the existing node counters
> > instead (which is what the non-memcg case already uses).
> > mod_lruvec_state() always updates the node vmstat and the memcg lruvec
> > stat in the same call. For the root lruvec, the node counter reflects
> > the same value and doesn't need any flushing. It has an error bound of
> > 125 pages/cpu at most, but this is tighter than what we currently have
> > with the ratelimited version.
>
> This seems like the right thing to do. If we have global stats, we
> should avoid a rstat flush. I actually wonder if we should start
> systematically eliminating root flushes, and if that ends up being
> good enough to keep everything else as-is for in-kernel flushers. The
> flush in zswap_shrinker_count() is also a good candidate, we already
> use global stats when memcg is disabled there as well.

I agree, I think we should replace all root flushes with reading the
node counter values. I'll submit some patches for that.

>
> >
> > commit 90a6f2a8f442 ("memcg: use ratelimited stats flush in the
> > reclaim") was what switched vmscan to using ratelimited flushing. The
> > reason for this was to fix the stalls from concurrent reclaimers
> > contending on the rstat lock. For the Cloudflare case, kswapd threads
> > were contending on the rstat lock on 8 numa-node machines. That's the
> > global path (eg sc->target_mem_cgroup is NULL,
> > mem_cgroup_flush_stats() turns it into root, so this flush essentially
> > ends up flushing every cgroup on the machine), which gets addressed by
> > adding the fix in b), which takes kswapd and global direct reclaim off
> > rstat entirely. For the non-root memcg case, the flushing is bounded
> > by the MEMCG_CHARGE_BATCH * num_online_cpus() threshold (a non-root
> > memcg's stats_updates only accumulates its own subtree's updates, so
> > it takes a lot longer for it to be exceeded than the root's does).
> >
> > I ran some benchmarks to get a rough idea of what a) costs. I set up a
> > workload that allocates 512M of anon memory in a memcg with memory.max
> > set to half of that to trigger reclaim continuously. I varied the
> > number of threads touching the memory, so that N of them hit
> > memory.max and enter direct reclaim on the same memcg at once. I used
> > bpftrace to measure the time spent inside css_rstat_flush() and
> > shrink_node() and used those two numbers to get a percentage for how
> > much of reclaim is spent in flushing. These are the numbers I saw:
> >
> > # threads      flush as % of shrink_node()
> > 1                ratelimited: 1.10%       non-ratelimited: 2.22%
> > 16              ratelimited: 0.36%       non-ratelimited: 2.87%
> > 64              ratelimited: 0.16%       non-ratelimited: 2.52%
> >
> > Between the ratelimited and the non-ratelimited runs, walltime and
> > pgsteal were pretty much indistinguishable (42.7s vs 43.6s at 16
> > threads and 41.7 s vs 42.4s at 64, with the ranges overlapping, and
> > pgsteal within 0.2%).
> >
> > I also measured the lock contention. At 16 and 64 threads, 15-20% of
> > __css_rstat_lock() acquisitions hit a contended trylock with the
> > non-ratelimited version. At 16 threads, the mean flush time rises
> > about 8% and at 64 it roughly triples from the single-thread case
> > (26us at 1 thread, 42 us at 16, 87us at 64)  but I didn't see any
> > effect of this on wall time or pgsteal between the non-ratelimited and
> > the ratelimited version, and the percentage of reclaim time spent
> > flushing stayed at 2-3%. I think that's because css_rstat_flush()
> > takes and drops the lock once per cpu rather than holding it across
> > the walk.
> >
> > I also ran it with 32 actively churning sub-cgroups under the reclaim
> > target, since css_rstat_flush() walks the descendants that have
> > pending updates. I saw that per-flush cost rises slightly (~24 us to
> > 27 us) but the share of reclaim time spent flushing went down rather
> > than up (4.1% to 3.2%), because reclaim gets more expensive faster
> > than flushing does. I don't have a ratelimited baseline at 32 children
> > though, because with the ratelimited version, all the runs I tried
> > OOMed. I think this is an alternative repro to the OOM issue but
> > doesn't need/have KVM dependencies, so if this would be useful to
> > clean up and send as a standalone C repro, please let me know.
>
> It might be useful if it can be added as a selftest or similar, so
> that such regressions can be caught more easily in the future?
>
> >
> > (Also btw (this is a bit tangential to this OOM issue, but somewhat
> > semi-related), I don't think the WORKINGSET_ACTIVATE_{ANON,FILE} stats
> > checking really does anything in practice. On my benchmark runs, the
> > "refaults != prev_refaults" check evaluates to false in pretty much
> > every scan. Even with flushing working properly, it didn't trigger on
> > 1673718 of 1674912 samples, even though the cgroup was thrashing the
> > whole time (a separate run of the same workload counted 6371860 anon
> > refaults vs 6523 activations). I think it's because reclaim is
> > scanning faster than the refaults counter advances, so it'll almost
> > always see no change. I'm not sure how much this matters in practice,
> > but I think effectively in that check above in prepare_scan_control(),
> > deactivation relies entirely on the inactive_is_low() check.)
>
> I am also not sure about this one. I am not sure if reclaim moving
> faster than refaults is something universal, it seems like it would be
> workload-specific. Johannes is the person that comes to mind when I
> think about these heuristics, so maybe he has an opinion about this?
> :)
>
> >
> > What do you all think about adding a) and b) as the fix for this issue?
>
> I generally agree. Doesn't really matter much, but I think we should
> do (b) then (a) to avoid reintroducing the regression temporarily?

I'll send out the patch for (b) and then (a).

Thinking some more about how stats are used... I think there's two
categories the stats fall into: a) hot in-kernel ones (eg LRU sizes,
workingset counters, dirty/writeback, zswap) which are read very
frequently from the reclaim, refault, writeback and zswap paths and b)
userspace-only ones (everything else) that are read through
memory.stat / bpf / etc and much less often. The hot in-kernel ones
are a small subset (~20 of ~80). Most of the stats fall into category
b). However right now, a flush always aggregates *all* of these stats,
even when the caller only needs / cares about the hot in-kernel ones.
Flushing does all the work for the stats in category b), which the
in-kernel flushers don't need. With the per-node lruvec copies, a
flush goes through 100+ counters per cgroup per CPU vs ~30 for just
the hot in-kernel ones (on a single-node machine).

What do you think about splitting the stats into two rstat trees, one
for each category? In-kernel flushers would then only flush the hot
tree (~30 counters). memory.stat would still flush both (eg userspace
would see the same thing as it sees today). Each tree would have its
own lock and its own stats_updates threshold. I can run some
benchmarks but I think this would make flushes a lot cheaper, and
since flushes would hold the lock for less time, it should also reduce
the contention we see between concurrent flushers right now. Together
with using node counters for root readers, hopefully this will make
the non-ratelimited flush cheap enough for all non-root in-kernel
readers and ideally let us get rid of the ratelimiting variant
altogether.

Do you think something like this makes sense?

Separate from this but also kind of semi-related, I noticed
css_rstat_flush() takes the rstat lock for every possible cpu, even
cpus that have nothing to flush for the subtree. Maybe another
improvement is to just first check locklessly whether there's any
updates and then just skip the cpu if there aren't any. I can send a
patch out for this.

Thanks,
Joanne

Re: memcg stats flushing (WAS "Re: [PATCH] fuse: disable default bdi strictlimiting")

[Yosry Ahmed]

> Thinking some more about how stats are used... I think there's two
> categories the stats fall into: a) hot in-kernel ones (eg LRU sizes,
> workingset counters, dirty/writeback, zswap) which are read very
> frequently from the reclaim, refault, writeback and zswap paths and b)
> userspace-only ones (everything else) that are read through
> memory.stat / bpf / etc and much less often. The hot in-kernel ones
> are a small subset (~20 of ~80). Most of the stats fall into category
> b). However right now, a flush always aggregates *all* of these stats,
> even when the caller only needs / cares about the hot in-kernel ones.
> Flushing does all the work for the stats in category b), which the
> in-kernel flushers don't need. With the per-node lruvec copies, a
> flush goes through 100+ counters per cgroup per CPU vs ~30 for just
> the hot in-kernel ones (on a single-node machine).

I didn't measure this before, but I did tree adding a bitmask to check
which stats have updates and that actually made flushing more expensive.
It could have been the case that hot stats were distributed enough that
all the cachelines were being fetched anyway and the bitmask was just
added overhead.

I think you should do a simple experiment first about how much flushing
gets faster if you skip the stats not used by the kernel. It may or may
not matter, depends on prefetching (conversely, I wonder if explicitly
prefetching the entire stats array on each CPU would help).

I think it would matter less if we manage to completely get rid of root
flushes (except for userspace stat reads).

> 
> What do you think about splitting the stats into two rstat trees, one
> for each category? In-kernel flushers would then only flush the hot
> tree (~30 counters). memory.stat would still flush both (eg userspace
> would see the same thing as it sees today). Each tree would have its
> own lock and its own stats_updates threshold.  I can run some
> benchmarks but I think this would make flushes a lot cheaper, and
> since flushes would hold the lock for less time, it should also reduce
> the contention we see between concurrent flushers right now. Together
> with using node counters for root readers, hopefully this will make
> the non-ratelimited flush cheap enough for all non-root in-kernel
> readers and ideally let us get rid of the ratelimiting variant
> altogether.
> 
> Do you think something like this makes sense?

We have to consider the update side of things as well. How many code
paths will need to update both trees? That could be a problem.

From a code perspective, it would also be a bit annoying to maintain. I
would check if we can eliminate root flushes first tbh, then check any
other potential optimizations on top. Subtrees should me a lot smaller
than the entire tree.

> Separate from this but also kind of semi-related, I noticed
> css_rstat_flush() takes the rstat lock for every possible cpu, even
> cpus that have nothing to flush for the subtree. Maybe another
> improvement is to just first check locklessly whether there's any
> updates and then just skip the cpu if there aren't any. I can send a
> patch out for this.

Only the updated cgroups are on the update tree, not their ancestors,
not until the flush time. We used to put the ancestors on the tree on
the update side, but we don't anymore.

So we can only check the if the per-subsystem list is empty, not the
subtree specifically, right? Can we walk the per-subsystem list without
holding the lock or deleting first?

> 
> Thanks,
> Joanne