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

[PATCH v1 0/3] 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.
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 just directly read global counters
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
(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.

Patch 2 adds lruvec_lru_page_state(), which reads the node counters for
the root memcg, and uses it in reclaim and workingset_test_recent().
workingset_test_recent() can then skip its flush 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 3 does the same for DAMON's node_memcg_used_bp and
node_memcg_free_bp quota goals, whose flush is not ratelimited.

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

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

Joanne Koong (3):
  mm: zswap: fix shrinker count for the root memcg
  mm: memcg: read root lruvec LRU stats from the node counters
  mm/damon/core: use node counters instead of flushing the root memcg

 include/linux/memcontrol.h | 16 ++++++++++++++
 mm/damon/core.c            | 15 ++++++++-----
 mm/memcontrol.c            | 44 ++++++++++++++++++++++++++++++++++++++
 mm/vmscan.c                | 36 +++++++++++++++----------------
 mm/workingset.c            | 19 +++++++++-------
 mm/zswap.c                 |  7 ++++--
 6 files changed, 104 insertions(+), 33 deletions(-)


base-commit: 0e02eb17f263e4a3ae6bf1fe45122c23361f0cf7
-- 
2.52.0

[PATCH v1 1/3] 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")
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 v1 2/3] mm: memcg: read root lruvec LRU stats from the node counters

[Joanne Koong]

Reclaim and refault detection read LRU sizes and workingset counters
from the target lruvec after a ratelimited rstat flush. For global
reclaim and the root memcg, that flush covers every cgroup in the
system, and the ratelimiting can leave the stats stale for seconds.

For stats that only track memory charged to a memcg (LRU sizes,
workingset, rotation and reclaim writeback counts), the root memcg's
hierarchical value equals the node counter, which is always up to date.
Add lruvec_lru_page_state() and a monotonic variant that read the node
counters for the root memcg, and use them in reclaim and
workingset_test_recent(). Root reclaim and refault detection then see
current values instead of stats that can lag by seconds.

workingset_test_recent() only reads the eviction memcg's own lruvec, so
skip its rstat flush when that is the root memcg. 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>
---
 include/linux/memcontrol.h | 16 ++++++++++++++
 mm/memcontrol.c            | 44 ++++++++++++++++++++++++++++++++++++++
 mm/vmscan.c                | 36 +++++++++++++++----------------
 mm/workingset.c            | 19 +++++++++-------
 4 files changed, 89 insertions(+), 26 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 980358bc6f49..656bd677dd3c 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -968,6 +968,10 @@ unsigned long lruvec_page_state_monotonic(const struct lruvec *lruvec,
 					  enum node_stat_item idx);
 unsigned long lruvec_page_state_local(const struct lruvec *lruvec,
 				      enum node_stat_item idx);
+unsigned long lruvec_lru_page_state(const struct lruvec *lruvec,
+				    enum node_stat_item idx);
+unsigned long lruvec_lru_page_state_monotonic(const struct lruvec *lruvec,
+					      enum node_stat_item idx);
 
 void mod_memcg_lruvec_state(struct lruvec *lruvec,
 			    enum node_stat_item idx, int val);
@@ -1430,6 +1434,18 @@ static inline unsigned long lruvec_page_state_local(const struct lruvec *lruvec,
 	return node_page_state(lruvec_pgdat(lruvec), idx);
 }
 
+static inline unsigned long lruvec_lru_page_state(const struct lruvec *lruvec,
+						  enum node_stat_item idx)
+{
+	return node_page_state(lruvec_pgdat(lruvec), idx);
+}
+
+static inline unsigned long lruvec_lru_page_state_monotonic(const struct lruvec *lruvec,
+							    enum node_stat_item idx)
+{
+	return node_page_state_monotonic(lruvec_pgdat(lruvec), idx);
+}
+
 static inline void mod_memcg_lruvec_state(struct lruvec *lruvec,
 					  enum node_stat_item idx, int val)
 {
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ac6c43ea8c5a..938843346571 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -587,6 +587,50 @@ unsigned long lruvec_page_state_local(const struct lruvec *lruvec,
 	return x;
 }
 
+static bool lruvec_is_root(const struct lruvec *lruvec)
+{
+	return !mem_cgroup_disabled() && mem_cgroup_is_root(lruvec_memcg(lruvec));
+}
+
+/**
+ * lruvec_lru_page_state - read an LRU stat of an lruvec
+ * @lruvec: the LRU vector to read from
+ * @idx: the node_stat_item to read
+ *
+ * Like lruvec_page_state(), but for the root memcg the node counter is read
+ * instead. For stats that only track memory charged to a memcg (LRU sizes,
+ * workingset, rotation and reclaim writeback counts), the root memcg's
+ * hierarchical value equals the node counter. The root memcg can just read the
+ * node counter directly instead of going through rstat, which requires
+ * flushing. Flushing the root memcg flushes every cgroup in the system.
+ *
+ * Use lruvec_page_state() for other stats (eg slab).
+ */
+unsigned long lruvec_lru_page_state(const struct lruvec *lruvec,
+				    enum node_stat_item idx)
+{
+	if (lruvec_is_root(lruvec))
+		return node_page_state(lruvec_pgdat(lruvec), idx);
+
+	return lruvec_page_state(lruvec, idx);
+}
+
+/**
+ * lruvec_lru_page_state_monotonic - lruvec_lru_page_state() for delta sampling
+ * @lruvec: the LRU vector to read from
+ * @idx: the node_stat_item to read
+ *
+ * See lruvec_page_state_monotonic() and lruvec_lru_page_state().
+ */
+unsigned long lruvec_lru_page_state_monotonic(const struct lruvec *lruvec,
+					      enum node_stat_item idx)
+{
+	if (lruvec_is_root(lruvec))
+		return node_page_state_monotonic(lruvec_pgdat(lruvec), idx);
+
+	return lruvec_page_state_monotonic(lruvec, idx);
+}
+
 static void __mod_memcg_lruvec_state(struct mem_cgroup *memcg, int nid,
 				     enum node_stat_item idx, long val);
 
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7ddef3b488b3..fc93a1d8afe0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2379,8 +2379,8 @@ static bool inactive_is_low(struct lruvec *lruvec, enum lru_list inactive_lru)
 	unsigned long inactive_ratio;
 	unsigned long gb;
 
-	inactive = lruvec_page_state(lruvec, NR_LRU_BASE + inactive_lru);
-	active = lruvec_page_state(lruvec, NR_LRU_BASE + active_lru);
+	inactive = lruvec_lru_page_state(lruvec, NR_LRU_BASE + inactive_lru);
+	active = lruvec_lru_page_state(lruvec, NR_LRU_BASE + active_lru);
 
 	gb = (inactive + active) >> (30 - PAGE_SHIFT);
 	if (gb)
@@ -2428,7 +2428,7 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
 	 * into a decaying average. This ensures currency, as workloads
 	 * change over time, and avoids overflow in the calculations.
 	 *
-	 * Use lruvec_page_state_monotonic() so unsigned subtraction
+	 * Use lruvec_lru_page_state_monotonic() so unsigned subtraction
 	 * yields the correct delta across a signed-long wraparound of
 	 * the underlying counter (a real hazard on 32-bit that the
 	 * clamp in lruvec_page_state() would otherwise turn into a huge
@@ -2440,13 +2440,13 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
 		struct lru_cost *cost = &target_lruvec->cost[f];
 		unsigned long rotated, io, nr_rotated, nr_io;
 
-		rotated = lruvec_page_state_monotonic(target_lruvec,
-						      PGROTATE_ANON + f);
-		io = lruvec_page_state_monotonic(target_lruvec,
-						 WORKINGSET_RESTORE_BASE + f);
+		rotated = lruvec_lru_page_state_monotonic(target_lruvec,
+							  PGROTATE_ANON + f);
+		io = lruvec_lru_page_state_monotonic(target_lruvec,
+						     WORKINGSET_RESTORE_BASE + f);
 		if (f == WORKINGSET_ANON)
-			io += lruvec_page_state_monotonic(target_lruvec,
-							  NR_VMSCAN_WRITE);
+			io += lruvec_lru_page_state_monotonic(target_lruvec,
+							      NR_VMSCAN_WRITE);
 
 		nr_rotated = rotated - cost->last_rotated;
 		nr_io = io - cost->last_io;
@@ -2468,10 +2468,10 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
 	anon_cost = &target_lruvec->cost[WORKINGSET_ANON];
 	file_cost = &target_lruvec->cost[WORKINGSET_FILE];
 
-	lrusize = lruvec_page_state(target_lruvec, NR_INACTIVE_ANON) +
-		  lruvec_page_state(target_lruvec, NR_ACTIVE_ANON) +
-		  lruvec_page_state(target_lruvec, NR_INACTIVE_FILE) +
-		  lruvec_page_state(target_lruvec, NR_ACTIVE_FILE);
+	lrusize = lruvec_lru_page_state(target_lruvec, NR_INACTIVE_ANON) +
+		  lruvec_lru_page_state(target_lruvec, NR_ACTIVE_ANON) +
+		  lruvec_lru_page_state(target_lruvec, NR_INACTIVE_FILE) +
+		  lruvec_lru_page_state(target_lruvec, NR_ACTIVE_FILE);
 
 	while (anon_cost->count + file_cost->count > lrusize / 4) {
 		anon_cost->count /= 2;
@@ -2495,7 +2495,7 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
 		 * workingset is being established. Deactivate to get
 		 * rid of any stale active pages quickly.
 		 */
-		refaults = lruvec_page_state(target_lruvec,
+		refaults = lruvec_lru_page_state(target_lruvec,
 				WORKINGSET_ACTIVATE_ANON);
 		if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
 			inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
@@ -2503,7 +2503,7 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
 		else
 			sc->may_deactivate &= ~DEACTIVATE_ANON;
 
-		refaults = lruvec_page_state(target_lruvec,
+		refaults = lruvec_lru_page_state(target_lruvec,
 				WORKINGSET_ACTIVATE_FILE);
 		if (refaults != target_lruvec->refaults[WORKINGSET_FILE] ||
 		    inactive_is_low(target_lruvec, LRU_INACTIVE_FILE))
@@ -2518,7 +2518,7 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
 	 * thrashing, try to reclaim those first before touching
 	 * anonymous pages.
 	 */
-	file = lruvec_page_state(target_lruvec, NR_INACTIVE_FILE);
+	file = lruvec_lru_page_state(target_lruvec, NR_INACTIVE_FILE);
 	if (file >> sc->priority && !(sc->may_deactivate & DEACTIVATE_FILE) &&
 	    !sc->no_cache_trim_mode)
 		sc->cache_trim_mode = 1;
@@ -6701,9 +6701,9 @@ static void snapshot_refaults(struct mem_cgroup *target_memcg, pg_data_t *pgdat)
 		return;
 
 	target_lruvec = mem_cgroup_lruvec(target_memcg, pgdat);
-	refaults = lruvec_page_state(target_lruvec, WORKINGSET_ACTIVATE_ANON);
+	refaults = lruvec_lru_page_state(target_lruvec, WORKINGSET_ACTIVATE_ANON);
 	target_lruvec->refaults[WORKINGSET_ANON] = refaults;
-	refaults = lruvec_page_state(target_lruvec, WORKINGSET_ACTIVATE_FILE);
+	refaults = lruvec_lru_page_state(target_lruvec, WORKINGSET_ACTIVATE_FILE);
 	target_lruvec->refaults[WORKINGSET_FILE] = refaults;
 }
 
diff --git a/mm/workingset.c b/mm/workingset.c
index 1504f91cdca5..e8620ae1c516 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_lru_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);
@@ -518,17 +521,17 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset,
 	 * workingset competition needs to consider anon or not depends
 	 * on having free swap space.
 	 */
-	workingset_size = lruvec_page_state(eviction_lruvec, NR_ACTIVE_FILE);
+	workingset_size = lruvec_lru_page_state(eviction_lruvec, NR_ACTIVE_FILE);
 	if (!file) {
-		workingset_size += lruvec_page_state(eviction_lruvec,
-						     NR_INACTIVE_FILE);
+		workingset_size += lruvec_lru_page_state(eviction_lruvec,
+							 NR_INACTIVE_FILE);
 	}
 	if (mem_cgroup_get_nr_swap_pages(eviction_memcg) > 0) {
-		workingset_size += lruvec_page_state(eviction_lruvec,
-						     NR_ACTIVE_ANON);
+		workingset_size += lruvec_lru_page_state(eviction_lruvec,
+							 NR_ACTIVE_ANON);
 		if (file) {
-			workingset_size += lruvec_page_state(eviction_lruvec,
-						     NR_INACTIVE_ANON);
+			workingset_size += lruvec_lru_page_state(eviction_lruvec,
+								 NR_INACTIVE_ANON);
 		}
 	}
 
-- 
2.52.0

[PATCH v1 3/3] 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.

For the root memcg, the node counters hold the same LRU sizes as the
memcg rstat stats, and they are always up to date. Use
lruvec_lru_page_state(), which reads the node counters for the root
memcg, and skip the flush for the root memcg.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index bcb6395d9f66..61d92e572c53 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3049,12 +3049,17 @@ 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_lru_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);
-	used_pages += lruvec_page_state(lruvec, NR_ACTIVE_FILE);
-	used_pages += lruvec_page_state(lruvec, NR_INACTIVE_FILE);
+	used_pages = lruvec_lru_page_state(lruvec, NR_ACTIVE_ANON);
+	used_pages += lruvec_lru_page_state(lruvec, NR_INACTIVE_ANON);
+	used_pages += lruvec_lru_page_state(lruvec, NR_ACTIVE_FILE);
+	used_pages += lruvec_lru_page_state(lruvec, NR_INACTIVE_FILE);
 
 	mem_cgroup_put(memcg);
 
-- 
2.52.0

Re: [PATCH v1 1/3] mm: zswap: fix shrinker count for the root memcg

[Yosry Ahmed]

On Mon, Nov 2, 2026 at 2:51 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> 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.

I would have taken the patch just for the dropped flush ;)

>
> 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")
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>

Acked-by: Yosry Ahmed <yosry@kernel.org>

Thanks!

Re: [PATCH v1 2/3] mm: memcg: read root lruvec LRU stats from the node counters

[Yosry Ahmed]

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index ac6c43ea8c5a..938843346571 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -587,6 +587,50 @@ unsigned long lruvec_page_state_local(const struct lruvec *lruvec,
>         return x;
>  }
>
> +static bool lruvec_is_root(const struct lruvec *lruvec)
> +{
> +       return !mem_cgroup_disabled() && mem_cgroup_is_root(lruvec_memcg(lruvec));
> +}
> +
> +/**
> + * lruvec_lru_page_state - read an LRU stat of an lruvec
> + * @lruvec: the LRU vector to read from
> + * @idx: the node_stat_item to read
> + *
> + * Like lruvec_page_state(), but for the root memcg the node counter is read
> + * instead. For stats that only track memory charged to a memcg (LRU sizes,
> + * workingset, rotation and reclaim writeback counts), the root memcg's
> + * hierarchical value equals the node counter. The root memcg can just read the
> + * node counter directly instead of going through rstat, which requires
> + * flushing. Flushing the root memcg flushes every cgroup in the system.
> + *
> + * Use lruvec_page_state() for other stats (eg slab).
> + */
> +unsigned long lruvec_lru_page_state(const struct lruvec *lruvec,
> +                                   enum node_stat_item idx)
> +{
> +       if (lruvec_is_root(lruvec))
> +               return node_page_state(lruvec_pgdat(lruvec), idx);
> +
> +       return lruvec_page_state(lruvec, idx);
> +}
> +
> +/**
> + * lruvec_lru_page_state_monotonic - lruvec_lru_page_state() for delta sampling
> + * @lruvec: the LRU vector to read from
> + * @idx: the node_stat_item to read
> + *
> + * See lruvec_page_state_monotonic() and lruvec_lru_page_state().
> + */
> +unsigned long lruvec_lru_page_state_monotonic(const struct lruvec *lruvec,
> +                                             enum node_stat_item idx)
> +{
> +       if (lruvec_is_root(lruvec))
> +               return node_page_state_monotonic(lruvec_pgdat(lruvec), idx);
> +
> +       return lruvec_page_state_monotonic(lruvec, idx);
> +}

Why not do this directly in lruvec_page_state() and
lruvec_page_state_monotonic()? They already fallback to node stats if
memcg is disabled, why not do the same for the root? Are there cases
where the node stats are not the right thing to do?

Re: [PATCH v1 2/3] mm: memcg: read root lruvec LRU stats from the node counters

[Joanne Koong]

On Mon, Nov 2, 2026 at 3:29 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index ac6c43ea8c5a..938843346571 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -587,6 +587,50 @@ unsigned long lruvec_page_state_local(const struct lruvec *lruvec,
> >         return x;
> >  }
> >
> > +static bool lruvec_is_root(const struct lruvec *lruvec)
> > +{
> > +       return !mem_cgroup_disabled() && mem_cgroup_is_root(lruvec_memcg(lruvec));
> > +}
> > +
> > +/**
> > + * lruvec_lru_page_state - read an LRU stat of an lruvec
> > + * @lruvec: the LRU vector to read from
> > + * @idx: the node_stat_item to read
> > + *
> > + * Like lruvec_page_state(), but for the root memcg the node counter is read
> > + * instead. For stats that only track memory charged to a memcg (LRU sizes,
> > + * workingset, rotation and reclaim writeback counts), the root memcg's
> > + * hierarchical value equals the node counter. The root memcg can just read the
> > + * node counter directly instead of going through rstat, which requires
> > + * flushing. Flushing the root memcg flushes every cgroup in the system.
> > + *
> > + * Use lruvec_page_state() for other stats (eg slab).
> > + */
> > +unsigned long lruvec_lru_page_state(const struct lruvec *lruvec,
> > +                                   enum node_stat_item idx)
> > +{
> > +       if (lruvec_is_root(lruvec))
> > +               return node_page_state(lruvec_pgdat(lruvec), idx);
> > +
> > +       return lruvec_page_state(lruvec, idx);
> > +}
> > +
> > +/**
> > + * lruvec_lru_page_state_monotonic - lruvec_lru_page_state() for delta sampling
> > + * @lruvec: the LRU vector to read from
> > + * @idx: the node_stat_item to read
> > + *
> > + * See lruvec_page_state_monotonic() and lruvec_lru_page_state().
> > + */
> > +unsigned long lruvec_lru_page_state_monotonic(const struct lruvec *lruvec,
> > +                                             enum node_stat_item idx)
> > +{
> > +       if (lruvec_is_root(lruvec))
> > +               return node_page_state_monotonic(lruvec_pgdat(lruvec), idx);
> > +
> > +       return lruvec_page_state_monotonic(lruvec, idx);
> > +}
>
> Why not do this directly in lruvec_page_state() and
> lruvec_page_state_monotonic()? They already fallback to node stats if
> memcg is disabled, why not do the same for the root? Are there cases
> where the node stats are not the right thing to do?

Hi Yosry,

Thanks for taking a look at this series! I don't think we're able to
do this directly in lruvec_page_state()/lruvec_page_state_monotonic()
because I think there's still some places where the node stat and the
root memcg stat are not guaranteed to be equal.

The stat I was looking at last week was slab's NR_SLAB_RECLAIMABLE_B
and NR_SLAB_UNRECLAIMABLE_B where for those, the node counter value
can contain memory/stats that wasn't actually charged to any memcg,
which I think means for these cases, the node counter values can be
larger than what reading the root memcg rstat value would give. The
functions I was looking at were account_slab() and
memcg_slab_post_alloc_hook() where the logic adds every new slab page
to the node counter but the memcg stats only get updated if specific
criteria are met (eg the cache had SLAB_ACCOUNT set or the allocation
had the __GFP_ACCOUNT gfp flag set and the allocation was not from a
root memcg).

Thanks,
Joanne

Re: [PATCH v1 2/3] mm: memcg: read root lruvec LRU stats from the node counters

[Yosry Ahmed]

On Mon, Nov 2, 2026 at 4:34 PM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Mon, Nov 2, 2026 at 3:29 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > index ac6c43ea8c5a..938843346571 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> > > @@ -587,6 +587,50 @@ unsigned long lruvec_page_state_local(const struct lruvec *lruvec,
> > >         return x;
> > >  }
> > >
> > > +static bool lruvec_is_root(const struct lruvec *lruvec)
> > > +{
> > > +       return !mem_cgroup_disabled() && mem_cgroup_is_root(lruvec_memcg(lruvec));
> > > +}
> > > +
> > > +/**
> > > + * lruvec_lru_page_state - read an LRU stat of an lruvec
> > > + * @lruvec: the LRU vector to read from
> > > + * @idx: the node_stat_item to read
> > > + *
> > > + * Like lruvec_page_state(), but for the root memcg the node counter is read
> > > + * instead. For stats that only track memory charged to a memcg (LRU sizes,
> > > + * workingset, rotation and reclaim writeback counts), the root memcg's
> > > + * hierarchical value equals the node counter. The root memcg can just read the
> > > + * node counter directly instead of going through rstat, which requires
> > > + * flushing. Flushing the root memcg flushes every cgroup in the system.
> > > + *
> > > + * Use lruvec_page_state() for other stats (eg slab).
> > > + */
> > > +unsigned long lruvec_lru_page_state(const struct lruvec *lruvec,
> > > +                                   enum node_stat_item idx)
> > > +{
> > > +       if (lruvec_is_root(lruvec))
> > > +               return node_page_state(lruvec_pgdat(lruvec), idx);
> > > +
> > > +       return lruvec_page_state(lruvec, idx);
> > > +}
> > > +
> > > +/**
> > > + * lruvec_lru_page_state_monotonic - lruvec_lru_page_state() for delta sampling
> > > + * @lruvec: the LRU vector to read from
> > > + * @idx: the node_stat_item to read
> > > + *
> > > + * See lruvec_page_state_monotonic() and lruvec_lru_page_state().
> > > + */
> > > +unsigned long lruvec_lru_page_state_monotonic(const struct lruvec *lruvec,
> > > +                                             enum node_stat_item idx)
> > > +{
> > > +       if (lruvec_is_root(lruvec))
> > > +               return node_page_state_monotonic(lruvec_pgdat(lruvec), idx);
> > > +
> > > +       return lruvec_page_state_monotonic(lruvec, idx);
> > > +}
> >
> > Why not do this directly in lruvec_page_state() and
> > lruvec_page_state_monotonic()? They already fallback to node stats if
> > memcg is disabled, why not do the same for the root? Are there cases
> > where the node stats are not the right thing to do?
>
> Hi Yosry,
>
> Thanks for taking a look at this series! I don't think we're able to
> do this directly in lruvec_page_state()/lruvec_page_state_monotonic()
> because I think there's still some places where the node stat and the
> root memcg stat are not guaranteed to be equal.
>
> The stat I was looking at last week was slab's NR_SLAB_RECLAIMABLE_B
> and NR_SLAB_UNRECLAIMABLE_B where for those, the node counter value
> can contain memory/stats that wasn't actually charged to any memcg,
> which I think means for these cases, the node counter values can be
> larger than what reading the root memcg rstat value would give. The
> functions I was looking at were account_slab() and
> memcg_slab_post_alloc_hook() where the logic adds every new slab page
> to the node counter but the memcg stats only get updated if specific
> criteria are met (eg the cache had SLAB_ACCOUNT set or the allocation
> had the __GFP_ACCOUNT gfp flag set and the allocation was not from a
> root memcg).

If it's only the slab stats, it probably makes sense to special case
those instead of the rest of the readers, right? It would be nice if
the default behavior is reading the node stats that doesn't require a
flush.

Re: [PATCH v1 1/3] mm: zswap: fix shrinker count for the root memcg

[SJ Park]

On Mon,  2 Nov 2026 14:50:28 -0800 Joanne Koong <joannelkoong@gmail.com> wrote:

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

Nice catch, makes sense to me.

> 
> Fixes: b5ba474f3f51 ("zswap: shrink zswap pool based on memory pressure")
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>

Reviewed-by: SJ Park <sj@kernel.org>


Thanks,
SJ

[...]

Re: [PATCH v1 3/3] mm/damon/core: use node counters instead of flushing the root memcg

[SJ Park]

Hi Joanne,


On Mon,  2 Nov 2026 14:50:30 -0800 Joanne Koong <joannelkoong@gmail.com> wrote:

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

Just thinking loudly.  damos_get_node_memcg_used_bp() is called only once per
DAMOS quota reset interval.  It is usually recommended to set the interval to 1
second.  So a sort of rate limiting is applied in common use cases.  That said,
some users might using quite short interval.  And it is obviously better to
avoid unnecessary flushes.

> 
> For the root memcg, the node counters hold the same LRU sizes as the
> memcg rstat stats, and they are always up to date. Use
> lruvec_lru_page_state(), which reads the node counters for the root
> memcg, and skip the flush for the root memcg.

Again, loud thought.  For root memcg, users might use
DAMOS_QUOTA_NODE_MEM_{USED,FREE}_BP instead, which calls
damos_get_node_mem_bp().  That said, some users might use
DAMOS_QUOTA_NODE_MEMCG_{USED,FREE}_BP for root memcg (maybe it makes user space
tool writing easier?).

So, looks all good to me, thank you for this patch.

> 
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>

Assuming the discussion on patch 2 concludes to have lruvec_lru_page_state(),

Reviewed-by: SJ Park <sj@kernel.org>


Thanks,
SJ

[...]