[PATCH v2 0/4] mm: read root memcg stats from global counters instead of flushing

[PATCH v2 0/4] mm: read root memcg stats from global counters instead of flushing

[Joanne Koong]

Reclaim, refault detection, the zswap shrinker, and DAMON quota goals all
flush the memcg rstat stats before reading them. When the target is the
root memcg (eg global reclaim), that flushes every cgroup in the system,
which is expensive. Most of these flushes are ratelimited today, but the
stats they read can be stale for seconds.

For the root memcg, the call sites can read global counters directly
instead of going through rstat. Global counters are always up to date
and don't need flushing. For stats that only track memory charged to a
memcg (eg LRU sizes, workingset, rotation, and reclaim writeback
counts), the root memcg's hierarchical value equals the node counter.

Patch 1 is a zswap fix for the shrinker's count for the root memcg and
can be applied independently of the rest. It doesn't have the stable
tag, since the zswap shrinker is opt-in and the fix can change writeback
behavior. With a workload running in the root cgroup, the shrinker wrote
back 0 pages before the fix and drained the whole pool after it.

Patch 2 makes lruvec_page_state() and lruvec_page_state_monotonic() read
the node counters for the root memcg by default, except for the stats
whose node counters also count memory that isn't charged to a memcg.

Patch 3 skips the flush in workingset_test_recent() when the eviction
memcg is the root memcg. Reclaim keeps its flush for now, since global
and root reclaim also read the stats of the child memcgs they scan.

Patch 4 skips the root flush for DAMON's node_memcg_used_bp and
node_memcg_free_bp quota goals, whose flush is not ratelimited.

Of the excluded stats, kernel stack, secondary page tables and
WORKINGSET_NODERECLAIM only differ from the node counters because
accounted kernel memory allocated by tasks in the root cgroup isn't
charged to the root memcg. If the root memcg's stats should include that
memory, then those could read the node counters as well. This is left
for a separate subsequent patchset though since that changes what the
root memcg reports and needs more upstream consensus.

Three root flushes remain. Reclaim also reads the stats of the child
memcgs it scans, and count_shadow_nodes() and filemap_cachestat() need
the root memcg's own (local) stats or the stats of arbitrary memcgs,
which means global counters can't be used as a substitute there. This
will be addressed in a subsequent patchset, as well as getting rid of
ratelimiting for most call sites.

On an 80-CPU machine running proactive reclaim on the root cgroup, a
root flush took 69us on average with little cgroup activity and ~0.9ms
with 128 memcgs repeatedly allocating and freeing memory (wall time,
including waiting for the rstat lock). Reading a node counter doesn't
add any cost: it took the same amount of time as reading the rstat
counter (~150ns, which includes bpftrace probe overhead).

For more context on the background of this series, please see the
discussion about memcg stats flushing in [1].

Changes since v1 [2]:
- Read the node counters for the root memcg by default in
  lruvec_page_state()/lruvec_page_state_monotonic(), with explicit
  exclusions, instead of adding lruvec_lru_page_state() helpers that
  callers opt in to (Yosry). As a result, the root memcg's memory.numa_stat
  now also reports the node counters for the non-excluded stats
- Split the workingset_test_recent() flush skip out into its own patch
  (patch 3)
- The DAMON patch (now patch 4) only skips the flush, as
  lruvec_page_state() reads the node counters for the root memcg
- Rebased onto current mm-new

[1] https://lore.kernel.org/linux-fsdevel/CAJnrk1aHdBm4CofvKfx7B3H-uF2xRXZUQs47J_4xeRmRZ4hcZg@mail.gmail.com/
[2] https://lore.kernel.org/linux-mm/20261102225030.2743129-1-joannelkoong@gmail.com/

Joanne Koong (4):
  mm: zswap: fix shrinker count for the root memcg
  mm: memcg: read root lruvec stats from the node counters
  mm: workingset: skip the root memcg flush in workingset_test_recent()
  mm/damon/core: use node counters instead of flushing the root memcg

 mm/damon/core.c |  7 ++++++-
 mm/memcontrol.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++---
 mm/workingset.c |  5 ++++-
 mm/zswap.c      |  7 +++++--
 4 files changed, 64 insertions(+), 7 deletions(-)


base-commit: b95f8bb86713c860ce8536d22a988b949b884d5f
-- 
2.52.0

[PATCH v2 1/4] mm: zswap: fix shrinker count for the root memcg

[Joanne Koong]

zswap_shrinker_count() scales the number of freeable entries on a
memcg's zswap LRU by the memcg's compression ratio. That compression
ratio is computed from the memcg's MEMCG_ZSWAP_B and MEMCG_ZSWAPPED
stats.

However, for the root memcg, the zswap entries owned by the root memcg
itself are never accounted for in the stats (obj_cgroup_charge_zswap()
skips the root objcg, and before the root memcg had an objcg, its
entries weren't charged at all). The stats only reflect the entries
owned by the root memcg's descendants. When the root memcg's descendants
have no zswap entries (eg when the whole workload runs in the root
cgroup), the shrinker reports nothing to reclaim for the root memcg, and
its zswap entries are never written back by the shrinker. When its
descendants do have entries, the root memcg's entries are scaled by the
descendants' compression ratio instead of a ratio that includes them.

Fix this by using the zswap pool-wide counters for the root memcg
instead. The zswap pool-wide counters cover every zswap entry, including
those owned by the root memcg.

This also gets rid of having to flush the root memcg's stats, which
flushes every cgroup in the system.

In a VM running a reclaim-heavy workload in the root cgroup with the
zswap shrinker enabled, the shrinker wrote back no pages before this
change (zswpwb stayed at 0) and 1016 pages with it.

Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")
Acked-by: Yosry Ahmed <yosry@kernel.org>
Reviewed-by: SJ Park <sj@kernel.org>
Acked-by: Nhat Pham <nphamcs@gmail.com>
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 mm/zswap.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mm/zswap.c b/mm/zswap.c
index 37d217860a6b..c11cd2a6e24e 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1332,9 +1332,12 @@ static unsigned long zswap_shrinker_count(struct shrinker *shrinker,
 	 * runtime-disabled: we can get sc->memcg == NULL, which is ok
 	 * for the lruvec, but not for memcg_page_state().
 	 *
-	 * Without memcg, use the zswap pool-wide metrics.
+	 * Without memcg, or for the root memcg, use the zswap pool-wide
+	 * metrics. The root memcg's stats don't count the entries it owns
+	 * itself (obj_cgroup_charge_zswap() skips the root objcg), whereas the
+	 * pool-wide metrics cover every entry and don't need a flush.
 	 */
-	if (!mem_cgroup_disabled()) {
+	if (!mem_cgroup_disabled() && !mem_cgroup_is_root(memcg)) {
 		mem_cgroup_flush_stats_ratelimited(memcg);
 		nr_backing = memcg_page_state(memcg, MEMCG_ZSWAP_B) >> PAGE_SHIFT;
 		nr_stored = memcg_page_state(memcg, MEMCG_ZSWAPPED);
-- 
2.52.0

[PATCH v2 2/4] mm: memcg: read root lruvec stats from the node counters

[Joanne Koong]

For the root memcg, lruvec_page_state() returns the rstat value, which
needs a flush to be up to date. Flushing the root memcg is expensive, as
it flushes every memcg in the system.

Most memcg node stats only track memory charged to a memcg, so the root
memcg's hierarchical value equals the node counter, which is always up
to date and doesn't need a flush. For those stats, have
lruvec_page_state() and lruvec_page_state_monotonic() read the node
counter for the root memcg, as they already do when memcg is disabled.
Callers that only read the root memcg's own lruvec then no longer need
to flush it, which the following patches take advantage of. It also
makes the root memcg's stats more accurate, as callers that ratelimit
the flush can otherwise read values that are seconds old.

The exceptions are the kernel memory stats (slab, kernel stack, page
tables, secondary page tables and vmalloc), hugetlb, and
WORKINGSET_NODERECLAIM, which counts reclaimed xarray nodes (slab
objects). Their node counters also count memory that isn't charged to a
memcg, which falls into two categories:

a) the allocation isn't accounted to any memcg (eg kernel page tables,
   or hugetlb when hugetlb accounting is off)
b) the allocation is accounted, but it comes from a task running in the
   root cgroup, and the root memcg isn't charged for it

As such, these stats still need to read the rstat value, since the node
counter value can be higher than the rstat value.

One user-visible change is that the root memcg's memory.numa_stat (and
the hierarchical_* entries of the cgroup v1 memory.numa_stat) now
reports the node counters for the non-excluded stats, as it reads
lruvec_page_state(). This is more accurate, but as a result, it may not
exactly match the root memcg's memory.stat, which still reads the rstat
values.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 mm/memcontrol.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 1ebc9330aeae..2575bf2ba0ea 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -389,7 +389,14 @@ ino_t page_cgroup_ino(const struct page *page)
 }
 EXPORT_SYMBOL_GPL(page_cgroup_ino);
 
-/* Subset of node_stat_item for memcg stats */
+/*
+ * Subset of node_stat_item for memcg stats.
+ *
+ * Stats whose node counters can also count memory that isn't charged to a memcg
+ * must be listed in node_stat_includes_unaccounted(). Otherwise,
+ * lruvec_page_state() returns the node counter for them for the root memcg,
+ * which can be larger than the root memcg's actual value.
+ */
 static const unsigned int memcg_node_stat_items[] = {
 	NR_INACTIVE_ANON,
 	NR_ACTIVE_ANON,
@@ -504,13 +511,52 @@ struct lruvec_stats {
 	long state_pending[NR_MEMCG_NODE_STAT_ITEMS];
 };
 
+/*
+ * Whether the node counter for @idx can also count memory that isn't charged
+ * to a memcg (eg kernel memory that isn't accounted). For these stats, the
+ * node counter can be larger than the root memcg's value, so the root memcg
+ * has to read its rstat value instead.
+ */
+static bool node_stat_includes_unaccounted(enum node_stat_item idx)
+{
+	switch (idx) {
+	case NR_SLAB_RECLAIMABLE_B:
+	case NR_SLAB_UNRECLAIMABLE_B:
+	case WORKINGSET_NODERECLAIM:
+	case NR_VMALLOC:
+	case NR_KERNEL_STACK_KB:
+	case NR_PAGETABLE:
+	case NR_SECONDARY_PAGETABLE:
+#ifdef CONFIG_HUGETLB_PAGE
+	case NR_HUGETLB:
+#endif
+		return true;
+	default:
+		return false;
+	}
+}
+
+/*
+ * Whether @lruvec should read the node counter for @idx instead of its rstat
+ * value. For the root memcg, stats that only count memory charged to a memcg
+ * have the same hierarchical value as the node counter. The node counter is
+ * preferred since it is always up to date, whereas the rstat value needs a
+ * flush, which for the root memcg flushes every cgroup in the system.
+ */
+static bool lruvec_use_node_stat(const struct lruvec *lruvec,
+				 enum node_stat_item idx)
+{
+	return mem_cgroup_is_root(lruvec_memcg(lruvec)) &&
+	       !node_stat_includes_unaccounted(idx);
+}
+
 unsigned long lruvec_page_state(const struct lruvec *lruvec, enum node_stat_item idx)
 {
 	const struct mem_cgroup_per_node *pn;
 	long x;
 	int i;
 
-	if (mem_cgroup_disabled())
+	if (mem_cgroup_disabled() || lruvec_use_node_stat(lruvec, idx))
 		return node_page_state(lruvec_pgdat(lruvec), idx);
 
 	i = memcg_stats_index(idx);
@@ -551,7 +597,7 @@ unsigned long lruvec_page_state_monotonic(const struct lruvec *lruvec,
 	const struct mem_cgroup_per_node *pn;
 	int i;
 
-	if (mem_cgroup_disabled())
+	if (mem_cgroup_disabled() || lruvec_use_node_stat(lruvec, idx))
 		return node_page_state_monotonic(lruvec_pgdat(lruvec), idx);
 
 	i = memcg_stats_index(idx);
-- 
2.52.0

Re: [PATCH v2 2/4] mm: memcg: read root lruvec stats from the node counters

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm: memcg: node_stat_includes_unaccounted() omits file and
  writeback stats
--

commit d2ba314724a8ba015e3bf2a30f155f1beba25a8c
Author: Joanne Koong <joannelkoong@gmail.com>

mm: memcg: read root lruvec stats from the node counters

This commit changes lruvec_page_state() and lruvec_page_state_monotonic()
to read per-node vmstat counters instead of rstat values for the root
memcg when querying stats that only track memory charged to a memcg.

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1ebc9330aeae5..2575bf2ba0ea6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c

[ ... ]

> @@ -504,13 +511,52 @@ struct lruvec_stats {
>  	long state_pending[NR_MEMCG_NODE_STAT_ITEMS];
>  };
>  
> +/*
> + * Whether the node counter for @idx can also count memory that isn't charged
> + * to a memcg (eg kernel memory that isn't accounted). For these stats, the
> + * node counter can be larger than the root memcg's value, so the root memcg
> + * has to read its rstat value instead.
> + */
> +static bool node_stat_includes_unaccounted(enum node_stat_item idx)
> +{
> +	switch (idx) {
> +	case NR_SLAB_RECLAIMABLE_B:
> +	case NR_SLAB_UNRECLAIMABLE_B:
> +	case WORKINGSET_NODERECLAIM:
> +	case NR_VMALLOC:
> +	case NR_KERNEL_STACK_KB:
> +	case NR_PAGETABLE:
> +	case NR_SECONDARY_PAGETABLE:
> +#ifdef CONFIG_HUGETLB_PAGE
> +	case NR_HUGETLB:
> +#endif
> +		return true;
> +	default:
> +		return false;
> +	}
> +}

[Severity: Medium]
Should node_stat_includes_unaccounted() also include NR_FILE_PAGES,
NR_FILE_MAPPED, and NR_WRITEBACK?

For NR_FILE_PAGES, swapcache operations such as __swap_cache_add_folio(),
__swap_cache_del_folio(), and __swap_cache_alloc() update NR_FILE_PAGES
only in the node counter while accounting swapcache in the lruvec under
NR_SWAPCACHE:

mm/swap_state.c:__swap_cache_add_folio() {
	...
	node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
	lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
	...
}

Reading node_page_state(pgdat, NR_FILE_PAGES) for the root lruvec
therefore includes all swapcache pages, which inflates the "file" counter
in the root cgroup's memory.numa_stat (double-counting swapcache alongside
"swapcached") and causes it to diverge from memory.stat even after a
flush.

Similarly, __folio_mod_stat() -> lruvec_stat_mod_folio() updates the node
NR_FILE_MAPPED counter without updating memcg rstat whenever an uncharged
folio (folio_memcg(folio) == NULL, such as a driver mapping or a
root-cgroup __GFP_ACCOUNT kernel folio) is mapped into userspace, and
nfs_folio_mark_unstable() and nfs_folio_clear_commit() update
NR_WRITEBACK only in the node counter for unstable NFS pages awaiting
server COMMIT.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110015321.611093-1-joannelkoong@gmail.com?part=2

Re: [PATCH v2 2/4] mm: memcg: read root lruvec stats from the node counters

[Joanne Koong]

On Mon, Nov 9, 2026 at 6:23 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] mm: memcg: node_stat_includes_unaccounted() omits file and
>   writeback stats
> --
>
> commit d2ba314724a8ba015e3bf2a30f155f1beba25a8c
> Author: Joanne Koong <joannelkoong@gmail.com>
>
> mm: memcg: read root lruvec stats from the node counters
>
> This commit changes lruvec_page_state() and lruvec_page_state_monotonic()
> to read per-node vmstat counters instead of rstat values for the root
> memcg when querying stats that only track memory charged to a memcg.
>
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index 1ebc9330aeae5..2575bf2ba0ea6 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
>
> [ ... ]
>
> > @@ -504,13 +511,52 @@ struct lruvec_stats {
> >       long state_pending[NR_MEMCG_NODE_STAT_ITEMS];
> >  };
> >
> > +/*
> > + * Whether the node counter for @idx can also count memory that isn't charged
> > + * to a memcg (eg kernel memory that isn't accounted). For these stats, the
> > + * node counter can be larger than the root memcg's value, so the root memcg
> > + * has to read its rstat value instead.
> > + */
> > +static bool node_stat_includes_unaccounted(enum node_stat_item idx)
> > +{
> > +     switch (idx) {
> > +     case NR_SLAB_RECLAIMABLE_B:
> > +     case NR_SLAB_UNRECLAIMABLE_B:
> > +     case WORKINGSET_NODERECLAIM:
> > +     case NR_VMALLOC:
> > +     case NR_KERNEL_STACK_KB:
> > +     case NR_PAGETABLE:
> > +     case NR_SECONDARY_PAGETABLE:
> > +#ifdef CONFIG_HUGETLB_PAGE
> > +     case NR_HUGETLB:
> > +#endif
> > +             return true;
> > +     default:
> > +             return false;
> > +     }
> > +}
>
> [Severity: Medium]
> Should node_stat_includes_unaccounted() also include NR_FILE_PAGES,
> NR_FILE_MAPPED, and NR_WRITEBACK?
>
> For NR_FILE_PAGES, swapcache operations such as __swap_cache_add_folio(),
> __swap_cache_del_folio(), and __swap_cache_alloc() update NR_FILE_PAGES
> only in the node counter while accounting swapcache in the lruvec under
> NR_SWAPCACHE:
>
> mm/swap_state.c:__swap_cache_add_folio() {
>         ...
>         node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
>         lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
>         ...
> }
>
> Reading node_page_state(pgdat, NR_FILE_PAGES) for the root lruvec
> therefore includes all swapcache pages, which inflates the "file" counter
> in the root cgroup's memory.numa_stat (double-counting swapcache alongside
> "swapcached") and causes it to diverge from memory.stat even after a
> flush.
>
> Similarly, __folio_mod_stat() -> lruvec_stat_mod_folio() updates the node
> NR_FILE_MAPPED counter without updating memcg rstat whenever an uncharged
> folio (folio_memcg(folio) == NULL, such as a driver mapping or a
> root-cgroup __GFP_ACCOUNT kernel folio) is mapped into userspace, and
> nfs_folio_mark_unstable() and nfs_folio_clear_commit() update
> NR_WRITEBACK only in the node counter for unstable NFS pages awaiting
> server COMMIT.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261110015321.611093-1-joannelkoong@gmail.com?part=2

Thanks, this is a good point about the folio stats.

Hmm... I wonder if we should go with an allowlist instead of a
denylist. The allowlist is longer (31 stats vs 11) but would be more
resilient to failures (a stat missing from the allowlist just reads
the rstat value whereas a stat missing from the denylist will silently
report the wrong value) for any stats added in the future. Any
thoughts?

Thanks,
Joanne

Re: [PATCH v2 2/4] mm: memcg: read root lruvec stats from the node counters

[Michal Koutný]

On Tue, Nov 10, 2026 at 03:10:54PM -0800, Joanne Koong <joannelkoong@gmail.com> wrote:
> On Mon, Nov 9, 2026 at 6:23 PM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue to consider:
> > - [Medium] mm: memcg: node_stat_includes_unaccounted() omits file and
> >   writeback stats
> > --
> >
> > commit d2ba314724a8ba015e3bf2a30f155f1beba25a8c
> > Author: Joanne Koong <joannelkoong@gmail.com>
> >
> > mm: memcg: read root lruvec stats from the node counters
> >
> > This commit changes lruvec_page_state() and lruvec_page_state_monotonic()
> > to read per-node vmstat counters instead of rstat values for the root
> > memcg when querying stats that only track memory charged to a memcg.
> >
> > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > index 1ebc9330aeae5..2575bf2ba0ea6 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> >
> > [ ... ]
> >
> > > @@ -504,13 +511,52 @@ struct lruvec_stats {
> > >       long state_pending[NR_MEMCG_NODE_STAT_ITEMS];
> > >  };
> > >
> > > +/*
> > > + * Whether the node counter for @idx can also count memory that isn't charged
> > > + * to a memcg (eg kernel memory that isn't accounted). For these stats, the
> > > + * node counter can be larger than the root memcg's value, so the root memcg
> > > + * has to read its rstat value instead.
> > > + */
> > > +static bool node_stat_includes_unaccounted(enum node_stat_item idx)
> > > +{
> > > +     switch (idx) {
> > > +     case NR_SLAB_RECLAIMABLE_B:
> > > +     case NR_SLAB_UNRECLAIMABLE_B:
> > > +     case WORKINGSET_NODERECLAIM:
> > > +     case NR_VMALLOC:
> > > +     case NR_KERNEL_STACK_KB:
> > > +     case NR_PAGETABLE:
> > > +     case NR_SECONDARY_PAGETABLE:
> > > +#ifdef CONFIG_HUGETLB_PAGE
> > > +     case NR_HUGETLB:
> > > +#endif
> > > +             return true;
> > > +     default:
> > > +             return false;
> > > +     }
> > > +}
> >
> > [Severity: Medium]
> > Should node_stat_includes_unaccounted() also include NR_FILE_PAGES,
> > NR_FILE_MAPPED, and NR_WRITEBACK?
> >
> > For NR_FILE_PAGES, swapcache operations such as __swap_cache_add_folio(),
> > __swap_cache_del_folio(), and __swap_cache_alloc() update NR_FILE_PAGES
> > only in the node counter while accounting swapcache in the lruvec under
> > NR_SWAPCACHE:
> >
> > mm/swap_state.c:__swap_cache_add_folio() {
> >         ...
> >         node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
> >         lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
> >         ...
> > }
> >
> > Reading node_page_state(pgdat, NR_FILE_PAGES) for the root lruvec
> > therefore includes all swapcache pages, which inflates the "file" counter
> > in the root cgroup's memory.numa_stat (double-counting swapcache alongside
> > "swapcached") and causes it to diverge from memory.stat even after a
> > flush.
> >
> > Similarly, __folio_mod_stat() -> lruvec_stat_mod_folio() updates the node
> > NR_FILE_MAPPED counter without updating memcg rstat whenever an uncharged
> > folio (folio_memcg(folio) == NULL, such as a driver mapping or a
> > root-cgroup __GFP_ACCOUNT kernel folio) is mapped into userspace, and
> > nfs_folio_mark_unstable() and nfs_folio_clear_commit() update
> > NR_WRITEBACK only in the node counter for unstable NFS pages awaiting
> > server COMMIT.
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20261110015321.611093-1-joannelkoong@gmail.com?part=2
> 
> Thanks, this is a good point about the folio stats.
> 
> Hmm... I wonder if we should go with an allowlist instead of a
> denylist. The allowlist is longer (31 stats vs 11) but would be more
> resilient to failures (a stat missing from the allowlist just reads
> the rstat value whereas a stat missing from the denylist will silently
> report the wrong value) for any stats added in the future. Any
> thoughts?

I agree the opposite predicate would be more robust and potentially
easier to review (review only the _listed_ cases, not the "unknown"
complement).

Also, instead of

       if (something && !mem_cgroup_is_root(eviction_memcg))
                mem_cgroup_flush_stats_ratelimited(eviction_memcg);

I'd consider

       if (something)
                mem_cgroup_flush_stats_ratelimited_opt(eviction_memcg,
		1UL<<NR_ACTIVE_ANON|
		1UL<<NR_INACTIVE_ANON| ... );

and collect the exceptional handling in memcontrol.c:

        void mem_cgroup_flush_stats_ratelimited_opt(struct mem_cgroup *memcg, unsigned long needs)
        {
                if ((needs & ~node_stat_equivalent) == 0 &&
                        mem_cgroup_is_root(mem_cgroup))
                        return;
		...

This way in-kernel flushers will be expicitly annotated so that
potential mismatch is more obvious to spot (and it adds possibility of
future optimizations).

(This is all only a concept, it may turn out uglier when worked out to
details...)

HTH,
Michal

[PATCH v2 3/4] mm: workingset: skip the root memcg flush in workingset_test_recent()

[Joanne Koong]

We can skip the rstat flush in workingset_test_recent() when the
eviction memcg is the root memcg. lruvec_page_state() now reads the LRU
sizes of the root memcg from the node counters, which are always up to
date, and workingset_test_recent() only reads the eviction memcg's own
lruvec. This avoids an expensive flush on the refault path, as flushing
the root memcg flushes every memcg in the system.

For reclaim however, we still need to keep the flush (for now) since
global and root reclaim also read the rstat stats of the child memcgs
they scan, which the flush is needed for in order to keep up to date.

On an 80-CPU machine, a root flush took 69us on average with little
cgroup activity and ~0.9ms with 128 memcgs actively allocating (wall
time, including waiting for the rstat lock). Reading a node counter
doesn't add any cost. It took the same amount of time as reading the
rstat counter (~150ns, which includes bpftrace probe overhead).

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 mm/workingset.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/workingset.c b/mm/workingset.c
index 1504f91cdca5..383f951eaad4 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -483,10 +483,13 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset,
 	 * section (for e.g, in cachestat) - these callers need to skip flushing
 	 * stats (via the flush argument).
 	 *
+	 * For the root memcg, the LRU sizes are read directly from the node
+	 * counters (see lruvec_page_state()).
+	 *
 	 * XXX: With per-memcg flushing and thresholding, is ratelimiting
 	 * still needed here?
 	 */
-	if (flush)
+	if (flush && !mem_cgroup_is_root(eviction_memcg))
 		mem_cgroup_flush_stats_ratelimited(eviction_memcg);
 
 	eviction_lruvec = mem_cgroup_lruvec(eviction_memcg, pgdat);
-- 
2.52.0

[PATCH v2 4/4] mm/damon/core: use node counters instead of flushing the root memcg

[Joanne Koong]

damos_get_node_memcg_used_bp() flushes the memcg stats before reading
the LRU sizes of the goal's memcg on the goal's node. For the root
memcg, that flushes every cgroup in the system. The flush is not
ratelimited, which means it happens every time the goal is evaluated.

lruvec_page_state() now reads the LRU sizes of the root memcg from the
node counters, which are always up to date, so skip the flush for the
root memcg.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 mm/damon/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 0a056949604f..14f2a36c1025 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3057,7 +3057,12 @@ static unsigned long damos_get_node_memcg_used_bp(
 			return 10000;
 	}
 
-	mem_cgroup_flush_stats(memcg);
+	/*
+	 * The root memcg's LRU sizes are read directly from the node counters
+	 * (see lruvec_page_state()).
+	 */
+	if (!mem_cgroup_is_root(memcg))
+		mem_cgroup_flush_stats(memcg);
 	lruvec = mem_cgroup_lruvec(memcg, NODE_DATA(goal->nid));
 	used_pages = lruvec_page_state(lruvec, NR_ACTIVE_ANON);
 	used_pages += lruvec_page_state(lruvec, NR_INACTIVE_ANON);
-- 
2.52.0