[PATCH] memcg: keep propagating stats updates to ancestors

[PATCH] memcg: keep propagating stats updates to ancestors

[Runli]

memcg_rstat_updated() stops walking the ancestors when a memcg exceeds
the flush threshold. A concurrent flush can leave an ancestor below the
threshold, so the early exit prevents it from receiving further updates.

Consider root and leaf on a 4-CPU system, with a threshold of 256.
CPU 0 is flushing another CPU's stats after CPU 1's stats have already
been flushed. The counts below are shared stats_updates counters:

  CPU 0 (root flush)                CPU 1 (leaf updates)
  ------------------               --------------------
  clear leaf->stats_updates
                                   add 64 to leaf and root
  clear root->stats_updates
  finish flush
                                   leaf = 64, root = 0
                                   4 more batches of 64:
                                     leaf = 320, root = 256
                                   next update:
                                     leaf > threshold
                                     break; root is not updated

Root remains at the threshold, which is insufficient to trigger a flush,
while further leaf updates keep taking the early exit. The periodic
forced flush restores progress, but until then the root's aggregated
statistics can fall far behind. For example, page cache grows at rate of
1 GiB/s, a two-second wait can leave the reported usage about 2 GiB below
the actual usage.

Skip only the flushable node and continue walking its ancestors, so each
ancestor can accumulate updates until it exceeds the flush threshold.

Fixes: 60cada258dfe ("memcg: optimize memcg_rstat_updated")
Signed-off-by: Runli <mingyu.he@shopee.com>
---
Testing:
Based on commit 893e11787f78, with and without this patch,
  using 32 concurrent instances of:
  netperf -H 127.0.0.1 -p 12875 -t TCP_STREAM -l 60 \
          -T <client_cpu>,<server_cpu> -P 0 -v 0 -f m -- -m 1024

Both netperf and netserver ran in the same leaf memcg, directly below
root (root -> leaf), with memory bound to NUMA node 0.
Each kernel was tested for five 60-second runs after a 10-second warmup.

Median aggregate throughput decreased from 107.26 to 105.23 Gbit/s
(1.9%) with this change.

I think fixing the correctness issue is worth the roughly 2% throughput
cost.

 mm/memcontrol.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 1271d390b617..4600d9c9244e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -729,12 +729,11 @@ static inline void memcg_rstat_updated(struct mem_cgroup *memcg, long val,
 	for (; statc_pcpu; statc_pcpu = statc->parent_pcpu) {
 		statc = this_cpu_ptr(statc_pcpu);
 		/*
-		 * If @memcg is already flushable then all its ancestors are
-		 * flushable as well and also there is no need to increase
-		 * stats_updates.
+		 * A concurrent flush may have reset an ancestor's counter.
+		 * Skip this node if flushable, but keep walking the ancestors.
 		 */
 		if (memcg_vmstats_needs_flush(statc->vmstats))
-			break;
+			continue;
 
 		stats_updates = this_cpu_add_return(statc_pcpu->stats_updates,
 						    abs(val));
-- 
2.43.0

Re: [PATCH] memcg: keep propagating stats updates to ancestors

[Shakeel Butt]

On Fri, Sep 11, 2026 at 11:29:05AM +0800, Runli wrote:
> memcg_rstat_updated() stops walking the ancestors when a memcg exceeds
> the flush threshold. A concurrent flush can leave an ancestor below the
> threshold, so the early exit prevents it from receiving further updates.
> 
> Consider root and leaf on a 4-CPU system, with a threshold of 256.
> CPU 0 is flushing another CPU's stats after CPU 1's stats have already
> been flushed. The counts below are shared stats_updates counters:
> 
>   CPU 0 (root flush)                CPU 1 (leaf updates)
>   ------------------               --------------------
>   clear leaf->stats_updates
>                                    add 64 to leaf and root
>   clear root->stats_updates
>   finish flush
>                                    leaf = 64, root = 0
>                                    4 more batches of 64:
>                                      leaf = 320, root = 256
>                                    next update:
>                                      leaf > threshold
>                                      break; root is not updated
> 
> Root remains at the threshold, which is insufficient to trigger a flush,
> while further leaf updates keep taking the early exit. The periodic
> forced flush restores progress, but until then the root's aggregated
> statistics can fall far behind. For example, page cache grows at rate of
> 1 GiB/s, a two-second wait can leave the reported usage about 2 GiB below
> the actual usage.
> 
> Skip only the flushable node and continue walking its ancestors, so each
> ancestor can accumulate updates until it exceeds the flush threshold.
> 
> Fixes: 60cada258dfe ("memcg: optimize memcg_rstat_updated")
> Signed-off-by: Runli <mingyu.he@shopee.com>
> ---
> Testing:
> Based on commit 893e11787f78, with and without this patch,
>   using 32 concurrent instances of:
>   netperf -H 127.0.0.1 -p 12875 -t TCP_STREAM -l 60 \
>           -T <client_cpu>,<server_cpu> -P 0 -v 0 -f m -- -m 1024
> 
> Both netperf and netserver ran in the same leaf memcg, directly below
> root (root -> leaf), with memory bound to NUMA node 0.
> Each kernel was tested for five 60-second runs after a 10-second warmup.
> 
> Median aggregate throughput decreased from 107.26 to 105.23 Gbit/s
> (1.9%) with this change.
> 
> I think fixing the correctness issue is worth the roughly 2% throughput
> cost.

Do you have any real world or production data to show that this is an issue? I
would not change anything unless there is real data. Also we have periodic flush
which will fix this (I have not looked deeper into this to see if this is a real
issue). In addition we have bpf interface for the stats for users who really
care about accuracy and efficiency.

Re: [PATCH] memcg: keep propagating stats updates to ancestors

[Mingyu He]

Hi Shakeel,

  This came from a reproducible workload with a custom in-kernel consumer,
  not a conjecture or an AI analysis. I uses aggregated file pages statistics to
  decide whether reclamation is needed.

  Tracing showed leaf stats_updates at 6656 while root stayed at 6144
  (the threshold). Further updates stopped at the leaf, and root flush
  requests were skipped.
  In one reclamation check, memcg_page_state(leaf, NR_FILE_PAGES) returned
  about 2.64 GiB from the aggregated statistics. In the same invocation,
  a bpftrace probe sequentially summed the leaf's per-CPU NR_FILE_PAGES
  counters and obtained about 6.58 GiB. This sum was not an atomic snapshot,
  but it exposed a large discrepancy with the value used by the check.

  The periodic flush does restore progress. But issue for this consumer is
  the missed reclamation decisions before that flush, not permanently lost
  accounting.

  Regarding the BPF interface, the implementation in the tree I found is:

  __bpf_kfunc void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg)
  {
          mem_cgroup_flush_stats(memcg);
  }
  This bpf function still uses memcg_rstat_updated() benethe.
  It doesn't handle the issue.

  I originally called mem_cgroup_flush_stats() before reading the counters.
  After observing this issue, I switched the consumer to css_rstat_flush()
  on the relevant subtree.

  If waiting for the 2 seconds periodic flush is acceptable for this
function even
  under continued updates, I agree there is no need to change it.

  However, the comment in memcg_rstat_updated() states:

          If @memcg is already flushable then all its ancestors are
          flushable as well

  Concurrent counter resets can invalidate that assumption, as the trace
  shows. If the early exit is retained as an acceptable trade-off, could
  we update the comment to acknowledge this and the reliance on a later
  flush to restore progress?

  Thanks,
  Runli

Re: [PATCH] memcg: keep propagating stats updates to ancestors

[Shakeel Butt]

On Mon, Oct 12, 2026 at 04:17:46PM +0800, Mingyu He wrote:
> Hi Shakeel,
> 
>   This came from a reproducible workload with a custom in-kernel consumer,

Please share your custom in-kernel consumer. Let's not make changes in the
kernel for things which are not in upstreame kernel.

[...]
> 
>   Regarding the BPF interface, the implementation in the tree I found is:
> 
>   __bpf_kfunc void bpf_mem_cgroup_flush_stats(struct mem_cgroup *memcg)
>   {
>           mem_cgroup_flush_stats(memcg);

Let's make this force flush.