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

On Thu, Aug 6, 2026 at 2:04 PM Joanne Koong <[email protected]> wrote:
>
> Hi Yosry,
>
> On Wed, Jul 22, 2026 at 1:38 PM Yosry Ahmed <[email protected]> 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.