Linux cgroups development
 help / color / mirror / Atom feed
* [PATCH 0/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK.
@ 2026-10-16  5:27 Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 1/5] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt
  Cc: Muchun Song, Kuniyuki Iwashima, Kuniyuki Iwashima, linux-mm,
	cgroups

On our production fleet, we saw mem_cgroup_sk_charge() spent over
3x the cycles of try_charge_memcg() although the former should be
a thin wrapper of the latter.

Looking at perf during tcp_rr, most of the cycles were spent in
mod_memcg_state(MEMCG_SOCK), which incurred two heavy memory stalls
due to cache line misses:

  mem_cgroup_sk_charge  vmlinux [Percent: local period]
  ...
     1.74 │       mov     cpu_number,%esi
     0.70 │       movslq  %ebx,%rcx
     0.31 │       mov     0x518(%r14),%rdx
     8.99 │       add     %rcx,%gs:0x18(%rdx,%rax,8)
                  ^-- this_cpu_add(memcg->vmstats_percpu->state[i], val);
                      in mod_memcg_state()
 >> 31.21 │       test    %ebx,%ebx
     0.05 │     ↓ je      97
          │       mov     %r14,%rdi
          │     → call    __css_rstat_updated
     0.31 │       mov     0x518(%r14),%rdx
          │ 72:   test    %rdx,%rdx
     0.06 │     ↓ je      97
          │       mov     this_cpu_off,%rsi
     0.17 │       mov     0x10(%rsi,%rdx,1),%rax
     6.59 │       mov     0xc18(%rax),%eax
                  ^-- atomic_read(&vmstats->stats_updates)
                      in memcg_vmstats_needs_flush()
 >> 26.02 │       mov     __num_online_cpus,%ecx
 ...
     0.65 │ e5:   mov     %r14,%rdi
          │       mov     %edx,%esi
     1.88 │       mov     %ebx,%edx
          │     → call    try_charge_memcg
     1.56 │       xor     %ebp,%ebp
     0.18 │       test    %eax,%eax
     0.22 │     ↑ jne     9f
          │     ↑ jmp     31

This series adds nr_sock[] to struct memcg_stock_pcp to batch
mod_memcg_state() for MEMCG_SOCK.

With this change, tcp_rr w/ 20000 flows achieves ~9% more
throughput and the TCP stack spends far fewer cycles on memcg.

  tcp_rr (20000 flows, 128 threads, 60s, average of 3 runs):

  Metric                     |       Baseline |        Patched |    Diff
  ---------------------------+----------------+----------------+--------
  Throughput (RPS)           |     22,408,523 |     24,360,262 |  +8.71%
  Latency Mean (us)          |         891.92 |         820.31 |  -8.03%
  Latency p50 (us)           |         877.22 |         744.10 | -15.18%
  Latency p90 (us)           |       1,085.43 |       1,082.02 |  -0.31%

  perf cycles (20000 flows, 10s, average of 3 runs):

  Server (M cycles / %)      |           Baseline |            Patched |    Diff
  ---------------------------+--------------------+--------------------+--------
  Host Total Cycles          |        4,994,342 M |        4,793,242 M |   -4.0%
  tcp_rcv_established()      | 213,968 M (4.283%) | 210,418 M (4.390%) |   -1.7%
  mem_cgroup_sk_charge()     |  12,322 M (0.247%) |   6,390 M (0.133%) |  -48.1%
  try_charge_memcg()         |   8,326 M (0.167%) |       0 M (0.000%) | -100.0%
  mem_cgroup_sk_uncharge()   |   7,492 M (0.150%) |   5,752 M (0.120%) |  -23.2%
  refill_stock()             |   7,988 M (0.160%) |       0 M (0.000%) | -100.0%
  Charge Total               |  20,648 M (0.413%) |   6,390 M (0.133%) |  -69.1%
  Uncharge Total             |  15,480 M (0.310%) |   5,752 M (0.120%) |  -62.8%
  Memcg Sock Total           |  36,128 M (0.723%) |  12,142 M (0.253%) |  -66.4%


Kuniyuki Iwashima (5):
  memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock.
  memcg: Reduce nesting in consume_stock() and refill_stock().
  memcg: Annotate slow paths in consume_stock() and refill_stock().
  memcg: Batch mod_memcg_state() for MEMCG_SOCK.
  selftests: cgroup: Deflake test_memcg_sock() in test_memcontrol.c.

 mm/memcontrol.c                               | 155 +++++++++++++-----
 .../selftests/cgroup/test_memcontrol.c        |   9 +-
 2 files changed, 118 insertions(+), 46 deletions(-)

-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/5] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock.
  2026-10-16  5:27 [PATCH 0/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
@ 2026-10-16  5:27 ` Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 2/5] memcg: Reduce nesting in consume_stock() and refill_stock() Kuniyuki Iwashima
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt
  Cc: Muchun Song, Kuniyuki Iwashima, Kuniyuki Iwashima, linux-mm,
	cgroups

this_cpu_ptr(&memcg_stock) is always modified under
local_lock(&memcg_stock.lock).

Unlike is_memcg_drain_needed() under RCU, consume_stock(),
drain_stock(), and refill_stock() do not need READ_ONCE().

Especially, in refill_stock(), READ_ONCE() is used twice for
stock->cached[i], which forces the compiler to emit two volatile
loads from stock->cached[i].

Let's remove the unnecessary READ_ONCE() and eliminate the
redundant loads in refill_stock():

Before:

  cached = READ_ONCE(stock->cached[i]);
   4c 8b 80 88 d7 bd 84 	movq	-0x7b422878(%rax), %r8
  if (memcg == READ_ONCE(stock->cached[i])) {
   48 3b 98 88 d7 bd 84 	cmpq	-0x7b422878(%rax), %rbx
   0f 84 86 01 00 00    	je	0xffffffff817d01e0 <refill_stock+0x210>

After:

  cached = stock->cached[i];
   4c 8b 88 88 d7 bd 84 	movq	-0x7b422878(%rax), %r9
  if (memcg == stock->cached[i]) {
   4c 39 cb             	cmpq	%r9, %rbx
   0f 84 7a 01 00 00    	je	0xffffffff817d01d0 <refill_stock+0x200>

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 mm/memcontrol.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 856a7d07586c..c268a7864547 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2135,10 +2135,10 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 	stock = this_cpu_ptr(&memcg_stock);
 
 	for (i = 0; i < NR_MEMCG_STOCK; ++i) {
-		if (memcg != READ_ONCE(stock->cached[i]))
+		if (memcg != stock->cached[i])
 			continue;
 
-		stock_pages = READ_ONCE(stock->nr_pages[i]);
+		stock_pages = stock->nr_pages[i];
 		if (stock_pages >= nr_pages) {
 			stock_pages -= nr_pages;
 			WRITE_ONCE(stock->nr_pages[i], stock_pages);
@@ -2168,13 +2168,13 @@ static void memcg_uncharge(struct mem_cgroup *memcg, unsigned int nr_pages)
  */
 static void drain_stock(struct memcg_stock_pcp *stock, int i)
 {
-	struct mem_cgroup *old = READ_ONCE(stock->cached[i]);
+	struct mem_cgroup *old = stock->cached[i];
 	uint8_t stock_pages;
 
 	if (!old)
 		return;
 
-	stock_pages = READ_ONCE(stock->nr_pages[i]);
+	stock_pages = stock->nr_pages[i];
 	if (stock_pages) {
 		memcg_uncharge(old, stock_pages);
 		WRITE_ONCE(stock->nr_pages[i], 0);
@@ -2254,11 +2254,11 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 
 	stock = this_cpu_ptr(&memcg_stock);
 	for (i = 0; i < NR_MEMCG_STOCK; ++i) {
-		cached = READ_ONCE(stock->cached[i]);
+		cached = stock->cached[i];
 		if (!cached && empty_slot == -1)
 			empty_slot = i;
-		if (memcg == READ_ONCE(stock->cached[i])) {
-			stock_pages = READ_ONCE(stock->nr_pages[i]) + nr_pages;
+		if (memcg == stock->cached[i]) {
+			stock_pages = stock->nr_pages[i] + nr_pages;
 			WRITE_ONCE(stock->nr_pages[i], stock_pages);
 			if (stock_pages > MEMCG_CHARGE_BATCH)
 				drain_stock(stock, i);
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/5] memcg: Reduce nesting in consume_stock() and refill_stock().
  2026-10-16  5:27 [PATCH 0/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 1/5] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
@ 2026-10-16  5:27 ` Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 3/5] memcg: Annotate slow paths " Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt
  Cc: Muchun Song, Kuniyuki Iwashima, Kuniyuki Iwashima, linux-mm,
	cgroups

A later patch adds more code to the most nested block in
consume_stock() and refill_stock().

Let's reduce the nesting to make the later change cleaner.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 mm/memcontrol.c | 59 ++++++++++++++++++++++++++-----------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c268a7864547..7c722a593cb5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2139,15 +2139,17 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 			continue;
 
 		stock_pages = stock->nr_pages[i];
-		if (stock_pages >= nr_pages) {
-			stock_pages -= nr_pages;
-			WRITE_ONCE(stock->nr_pages[i], stock_pages);
-			if (!stock_pages) {
-				css_put(&memcg->css);
-				WRITE_ONCE(stock->cached[i], NULL);
-			}
-			ret = true;
+		if (stock_pages < nr_pages)
+			break;
+
+		stock_pages -= nr_pages;
+		WRITE_ONCE(stock->nr_pages[i], stock_pages);
+		if (!stock_pages) {
+			css_put(&memcg->css);
+			WRITE_ONCE(stock->cached[i], NULL);
 		}
+
+		ret = true;
 		break;
 	}
 
@@ -2229,7 +2231,6 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 	struct memcg_stock_pcp *stock;
 	struct mem_cgroup *cached;
 	uint8_t stock_pages;
-	bool success = false;
 	int empty_slot = -1;
 	int i;
 
@@ -2257,29 +2258,31 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 		cached = stock->cached[i];
 		if (!cached && empty_slot == -1)
 			empty_slot = i;
-		if (memcg == stock->cached[i]) {
-			stock_pages = stock->nr_pages[i] + nr_pages;
-			WRITE_ONCE(stock->nr_pages[i], stock_pages);
-			if (stock_pages > MEMCG_CHARGE_BATCH)
-				drain_stock(stock, i);
-			success = true;
-			break;
-		}
-	}
 
-	if (!success) {
-		i = empty_slot;
-		if (i == -1) {
-			i = stock->drain_idx++;
-			if (stock->drain_idx == NR_MEMCG_STOCK)
-				stock->drain_idx = 0;
+		if (memcg != cached)
+			continue;
+
+		stock_pages = stock->nr_pages[i] + nr_pages;
+		WRITE_ONCE(stock->nr_pages[i], stock_pages);
+		if (stock_pages > MEMCG_CHARGE_BATCH)
 			drain_stock(stock, i);
-		}
-		css_get(&memcg->css);
-		WRITE_ONCE(stock->cached[i], memcg);
-		WRITE_ONCE(stock->nr_pages[i], nr_pages);
+
+		local_unlock(&memcg_stock.lock);
+		return;
 	}
 
+	i = empty_slot;
+	if (i == -1) {
+		i = stock->drain_idx++;
+		if (stock->drain_idx == NR_MEMCG_STOCK)
+			stock->drain_idx = 0;
+		drain_stock(stock, i);
+	}
+
+	css_get(&memcg->css);
+	WRITE_ONCE(stock->cached[i], memcg);
+	WRITE_ONCE(stock->nr_pages[i], nr_pages);
+
 	local_unlock(&memcg_stock.lock);
 }
 
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/5] memcg: Annotate slow paths in consume_stock() and refill_stock().
  2026-10-16  5:27 [PATCH 0/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 1/5] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 2/5] memcg: Reduce nesting in consume_stock() and refill_stock() Kuniyuki Iwashima
@ 2026-10-16  5:27 ` Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 4/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 5/5] selftests: cgroup: Deflake test_memcg_sock() in test_memcontrol.c Kuniyuki Iwashima
  4 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt
  Cc: Muchun Song, Kuniyuki Iwashima, Kuniyuki Iwashima, linux-mm,
	cgroups

In consume_stock() and refill_stock(), each per-CPU stock slot
holds up to MEMCG_CHARGE_BATCH (64) pages, so depleting a slot
in consume_stock() (!stock_pages) or overflowing a slot in
refill_stock() (stock_pages > MEMCG_CHARGE_BATCH) happens at most
once per batch.

Similarly, requests larger than MEMCG_CHARGE_BATCH, local_trylock()
failure, and insufficient cached pages (stock_pages < nr_pages) are
all slow paths.

Without unlikely() annotations (when building without FDO, or when
an FDO profile does not cover a caller), the compiler treats these
branches as likely and inlines css_put() in consume_stock() and
drain_stock() in refill_stock() right into the middle of the hot path.

Consequently, the fast path has to jump over the inlined slow path
and jump again to the function epilogue after local_unlock().

Let's annotate the slow paths in consume_stock() and refill_stock()
with unlikely() so that the compiler keeps css_put() and
drain_stock() out-of-line at the cold tail and lets local_unlock()
and the function epilogue fall through on the fast path:

Before (refill_stock):

  stock_pages = stock->nr_pages[i] + nr_pages;
   b242: 43 02 74 3c 01          addb    0x1(%r12,%r15), %sil
  WRITE_ONCE(stock->nr_pages[i], stock_pages);
   b247: 43 88 74 3c 01          movb    %sil, 0x1(%r12,%r15)
  if (stock_pages > MEMCG_CHARGE_BATCH)
   b24c: 40 80 fe 41             cmpb    $0x41, %sil
   b250: 72 4f                   jb      0xb2a1 <refill_stock+0x2a1>
  struct mem_cgroup *old = stock->cached[i];   # drain_stock() inlined
   b252: 4b 8b 5c fc 08          movq    0x8(%r12,%r15,8), %rbx
   ...
   b276: e8 00 00 00 00          callq   <page_counter_uncharge>
   ...
  local_unlock(&memcg_stock.lock);
   b2a1: 65 48 8b 05 00 00 00 00 movq    %gs:(%rip), %rax
   b2a9: c6 80 00 00 00 00 00    movb    $0x0, (%rax)
   b2b0: e9 93 fd ff ff          jmp     0xb048 <refill_stock+0x48>

After (refill_stock):

  stock_pages = stock->nr_pages[i] + nr_pages;
   b278: 40 02 74 03 01          addb    0x1(%rbx,%rax), %sil
  WRITE_ONCE(stock->nr_pages[i], stock_pages);
   b27d: 40 88 74 03 01          movb    %sil, 0x1(%rbx,%rax)
  if (unlikely(stock_pages > MEMCG_CHARGE_BATCH))
   b282: 40 80 fe 41             cmpb    $0x41, %sil
   b286: 73 23                   jae     0xb2ab <refill_stock+0x27b>
  local_unlock(&memcg_stock.lock);
   b288: 65 48 8b 05 00 00 00 00 movq    %gs:(%rip), %rax
   b290: c6 80 00 00 00 00 00    movb    $0x0, (%rax)
  }
   b297: 48 83 c4 08             addq    $0x8, %rsp
   b29b: 5b                      popq    %rbx
   b29c: 41 5c                   popq    %r12
   b29e: 41 5d                   popq    %r13
   b2a0: 41 5e                   popq    %r14
   b2a2: 41 5f                   popq    %r15
   b2a4: 5d                      popq    %rbp
   b2a5: 2e e9 00 00 00 00       jmp     <__x86_return_thunk>
  drain_stock(stock, i);
   b2ab: 48 89 df                movq    %rbx, %rdi
   b2ae: 89 c6                   movl    %eax, %esi
   b2b0: e8 2b 16 00 00          callq   0xc8e0 <drain_stock>
   b2b5: eb d1                   jmp     0xb288 <refill_stock+0x258>

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 mm/memcontrol.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7c722a593cb5..cc3d78938b34 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2128,8 +2128,8 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 	bool ret = false;
 	int i;
 
-	if (nr_pages > MEMCG_CHARGE_BATCH ||
-	    !local_trylock(&memcg_stock.lock))
+	if (unlikely(nr_pages > MEMCG_CHARGE_BATCH ||
+		     !local_trylock(&memcg_stock.lock)))
 		return ret;
 
 	stock = this_cpu_ptr(&memcg_stock);
@@ -2139,12 +2139,12 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 			continue;
 
 		stock_pages = stock->nr_pages[i];
-		if (stock_pages < nr_pages)
+		if (unlikely(stock_pages < nr_pages))
 			break;
 
 		stock_pages -= nr_pages;
 		WRITE_ONCE(stock->nr_pages[i], stock_pages);
-		if (!stock_pages) {
+		if (unlikely(!stock_pages)) {
 			css_put(&memcg->css);
 			WRITE_ONCE(stock->cached[i], NULL);
 		}
@@ -2243,8 +2243,8 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 
 	VM_WARN_ON_ONCE(mem_cgroup_is_root(memcg));
 
-	if (nr_pages > MEMCG_CHARGE_BATCH ||
-	    !local_trylock(&memcg_stock.lock)) {
+	if (unlikely(nr_pages > MEMCG_CHARGE_BATCH ||
+		     !local_trylock(&memcg_stock.lock))) {
 		/*
 		 * In case of larger than batch refill or unlikely failure to
 		 * lock the percpu memcg_stock.lock, uncharge memcg directly.
@@ -2264,7 +2264,7 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 
 		stock_pages = stock->nr_pages[i] + nr_pages;
 		WRITE_ONCE(stock->nr_pages[i], stock_pages);
-		if (stock_pages > MEMCG_CHARGE_BATCH)
+		if (unlikely(stock_pages > MEMCG_CHARGE_BATCH))
 			drain_stock(stock, i);
 
 		local_unlock(&memcg_stock.lock);
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK.
  2026-10-16  5:27 [PATCH 0/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2026-10-16  5:27 ` [PATCH 3/5] memcg: Annotate slow paths " Kuniyuki Iwashima
@ 2026-10-16  5:27 ` Kuniyuki Iwashima
  2026-10-16  5:27 ` [PATCH 5/5] selftests: cgroup: Deflake test_memcg_sock() in test_memcontrol.c Kuniyuki Iwashima
  4 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt
  Cc: Muchun Song, Kuniyuki Iwashima, Kuniyuki Iwashima, linux-mm,
	cgroups

On our production fleet, we saw mem_cgroup_sk_charge() spent over
3x the cycles of try_charge_memcg() although the former should be
a thin wrapper of the latter.

Looking at perf during tcp_rr, most of the cycles were spent in
mod_memcg_state(MEMCG_SOCK), which incurred two heavy memory stalls
due to cache line misses:

  mem_cgroup_sk_charge  vmlinux [Percent: local period]
  ...
     1.74 │       mov     cpu_number,%esi
     0.70 │       movslq  %ebx,%rcx
     0.31 │       mov     0x518(%r14),%rdx
     8.99 │       add     %rcx,%gs:0x18(%rdx,%rax,8)
                  ^-- this_cpu_add(memcg->vmstats_percpu->state[i], val);
                      in mod_memcg_state()
 >> 31.21 │       test    %ebx,%ebx
     0.05 │     ↓ je      97
          │       mov     %r14,%rdi
          │     → call    __css_rstat_updated
     0.31 │       mov     0x518(%r14),%rdx
          │ 72:   test    %rdx,%rdx
     0.06 │     ↓ je      97
          │       mov     this_cpu_off,%rsi
     0.17 │       mov     0x10(%rsi,%rdx,1),%rax
     6.59 │       mov     0xc18(%rax),%eax
                  ^-- atomic_read(&vmstats->stats_updates)
                      in memcg_vmstats_needs_flush()
 >> 26.02 │       mov     __num_online_cpus,%ecx
 ...
     0.65 │ e5:   mov     %r14,%rdi
          │       mov     %edx,%esi
     1.88 │       mov     %ebx,%edx
          │     → call    try_charge_memcg
     1.56 │       xor     %ebp,%ebp
     0.18 │       test    %eax,%eax
     0.22 │     ↑ jne     9f
          │     ↑ jmp     31

Even worse, after the 2nd cache line miss, the subsequent
atomic_add() for statc->vmstats->stats_updates invalidates the
cache for other CPUs.

Moreover, try_charge_memcg() returned just after consume_stock()
in most cases, but it was not inlined even with FDO.

Usually, skb is charged and uncharged immediately, and caching
the counter in struct memcg_stock_pcp helps.

Let's reduce NR_MEMCG_STOCK from 7 to 6 to fit nr_sock[] in the
single cacheline of struct memcg_stock_pcp, inline the stock
fast path, and batch mod_memcg_state() for MEMCG_SOCK (-64, 64).

With this change, tcp_rr w/ 20000 flows achieves ~9% more
throughput and the TCP stack spends far fewer cycles on memcg.
Note that p90 latency improves only slightly because batching
amortises mod_memcg_state() and drain_stock() into every 64th
page.

  tcp_rr (20000 flows, 128 threads, 60s, average of 3 runs):

  Metric                     |       Baseline |        Patched |    Diff
  ---------------------------+----------------+----------------+--------
  Throughput (RPS)           |     22,408,523 |     24,360,262 |  +8.71%
  Latency Mean (us)          |         891.92 |         820.31 |  -8.03%
  Latency p50 (us)           |         877.22 |         744.10 | -15.18%
  Latency p90 (us)           |       1,085.43 |       1,082.02 |  -0.31%

  perf cycles (20000 flows, 10s, average of 3 runs):

  Server (M cycles / %)      |           Baseline |            Patched |    Diff
  ---------------------------+--------------------+--------------------+--------
  Host Total Cycles          |        4,994,342 M |        4,793,242 M |   -4.0%
  tcp_rcv_established()      | 213,968 M (4.283%) | 210,418 M (4.390%) |   -1.7%
  mem_cgroup_sk_charge()     |  12,322 M (0.247%) |   6,390 M (0.133%) |  -48.1%
  try_charge_memcg()         |   8,326 M (0.167%) |       0 M (0.000%) | -100.0%
  mem_cgroup_sk_uncharge()   |   7,492 M (0.150%) |   5,752 M (0.120%) |  -23.2%
  refill_stock()             |   7,988 M (0.160%) |       0 M (0.000%) | -100.0%
  Charge Total               |  20,648 M (0.413%) |   6,390 M (0.133%) |  -69.1%
  Uncharge Total             |  15,480 M (0.310%) |   5,752 M (0.120%) |  -62.8%
  Memcg Sock Total           |  36,128 M (0.723%) |  12,142 M (0.253%) |  -66.4%

As a side effect, "sock" in memory.stat can be off by up to
MEMCG_CHARGE_BATCH pages per CPU until the slot is drained,
like memory.current with nr_pages[].  This is acceptable, as
flushing nr_sock[] every 2 seconds (FLUSH_TIME) across all CPUs
would rather cause cache line invalidation and hurt performance.

Note that MEMCG_CHARGE_BATCH is unsigned int, so (int) cast is
needed, and stock_sock is declared as int rather than int8_t
to avoid potential overflow for MEMCG_CHARGE_BATCH > 64 and to
generate better asm without 8-bit partial-register operations
or extra sign-extension before mod_memcg_state().

Also, __always_inline is needed for both __consume_stock() and
__refill_stock(), otherwise, FDO branch weight is calculated
from two callers with mixed states (nr_sock == 0 vs nr_sock != 0).

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 mm/memcontrol.c | 84 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 73 insertions(+), 11 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index cc3d78938b34..52831ae7fd0f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2045,13 +2045,14 @@ void mem_cgroup_print_oom_group(struct mem_cgroup *memcg)
 
 /*
  * The value of NR_MEMCG_STOCK is selected to keep the cached memcgs and their
- * nr_pages in a single cacheline. This may change in future.
+ * nr_pages and nr_sock in a single cacheline. This may change in future.
  */
-#define NR_MEMCG_STOCK 7
+#define NR_MEMCG_STOCK 6
 #define FLUSHING_CACHED_CHARGE	0
 struct memcg_stock_pcp {
 	local_trylock_t lock;
 	uint8_t nr_pages[NR_MEMCG_STOCK];
+	int8_t nr_sock[NR_MEMCG_STOCK];
 	struct mem_cgroup *cached[NR_MEMCG_STOCK];
 
 	struct work_struct work;
@@ -2111,9 +2112,10 @@ static bool obj_stock_flush_required(struct obj_stock_pcp *stock,
 				     struct mem_cgroup *root_memcg);
 
 /**
- * consume_stock: Try to consume stocked charge on this cpu.
+ * __consume_stock: Try to consume stocked charge on this cpu.
  * @memcg: memcg to consume from.
  * @nr_pages: how many pages to charge.
+ * @nr_sock: how many pages to account for MEMCG_SOCK.
  *
  * Consume the cached charge if enough nr_pages are present otherwise return
  * failure. Also return failure for charge request larger than
@@ -2121,10 +2123,13 @@ static bool obj_stock_flush_required(struct obj_stock_pcp *stock,
  *
  * returns true if successful, false otherwise.
  */
-static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
+static __always_inline bool __consume_stock(struct mem_cgroup *memcg,
+					    unsigned int nr_pages,
+					    unsigned int nr_sock)
 {
 	struct memcg_stock_pcp *stock;
 	uint8_t stock_pages;
+	int stock_sock = 0;
 	bool ret = false;
 	int i;
 
@@ -2144,7 +2149,19 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 
 		stock_pages -= nr_pages;
 		WRITE_ONCE(stock->nr_pages[i], stock_pages);
-		if (unlikely(!stock_pages)) {
+
+		if (nr_sock) {
+			stock_sock = stock->nr_sock[i] + nr_sock;
+
+			if (likely(stock_sock < (int)MEMCG_CHARGE_BATCH)) {
+				WRITE_ONCE(stock->nr_sock[i], stock_sock);
+				stock_sock = 0;
+			} else {
+				WRITE_ONCE(stock->nr_sock[i], 0);
+			}
+		}
+
+		if (unlikely(!stock_pages && !stock->nr_sock[i])) {
 			css_put(&memcg->css);
 			WRITE_ONCE(stock->cached[i], NULL);
 		}
@@ -2155,9 +2172,17 @@ static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 
 	local_unlock(&memcg_stock.lock);
 
+	if (unlikely(stock_sock))
+		mod_memcg_state(memcg, MEMCG_SOCK, stock_sock);
+
 	return ret;
 }
 
+static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
+{
+	return __consume_stock(memcg, nr_pages, 0);
+}
+
 static void memcg_uncharge(struct mem_cgroup *memcg, unsigned int nr_pages)
 {
 	page_counter_uncharge(&memcg->memory, nr_pages);
@@ -2172,6 +2197,7 @@ static void drain_stock(struct memcg_stock_pcp *stock, int i)
 {
 	struct mem_cgroup *old = stock->cached[i];
 	uint8_t stock_pages;
+	int stock_sock;
 
 	if (!old)
 		return;
@@ -2182,6 +2208,12 @@ static void drain_stock(struct memcg_stock_pcp *stock, int i)
 		WRITE_ONCE(stock->nr_pages[i], 0);
 	}
 
+	stock_sock = stock->nr_sock[i];
+	if (stock_sock) {
+		mod_memcg_state(old, MEMCG_SOCK, stock_sock);
+		WRITE_ONCE(stock->nr_sock[i], 0);
+	}
+
 	css_put(&old->css);
 	WRITE_ONCE(stock->cached[i], NULL);
 }
@@ -2226,12 +2258,15 @@ static void drain_local_obj_stock(struct work_struct *dummy)
 	local_unlock(&obj_stock.lock);
 }
 
-static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
+static __always_inline void __refill_stock(struct mem_cgroup *memcg,
+					   unsigned int nr_pages,
+					   unsigned int nr_sock)
 {
 	struct memcg_stock_pcp *stock;
 	struct mem_cgroup *cached;
 	uint8_t stock_pages;
 	int empty_slot = -1;
+	int stock_sock = 0;
 	int i;
 
 	/*
@@ -2250,7 +2285,7 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 		 * lock the percpu memcg_stock.lock, uncharge memcg directly.
 		 */
 		memcg_uncharge(memcg, nr_pages);
-		return;
+		goto update_sock;
 	}
 
 	stock = this_cpu_ptr(&memcg_stock);
@@ -2264,10 +2299,26 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 
 		stock_pages = stock->nr_pages[i] + nr_pages;
 		WRITE_ONCE(stock->nr_pages[i], stock_pages);
+
+		if (nr_sock) {
+			stock_sock = stock->nr_sock[i] - nr_sock;
+
+			if (likely(stock_sock > -(int)MEMCG_CHARGE_BATCH)) {
+				WRITE_ONCE(stock->nr_sock[i], stock_sock);
+				stock_sock = 0;
+			} else {
+				WRITE_ONCE(stock->nr_sock[i], 0);
+			}
+		}
+
 		if (unlikely(stock_pages > MEMCG_CHARGE_BATCH))
 			drain_stock(stock, i);
 
 		local_unlock(&memcg_stock.lock);
+
+		if (unlikely(stock_sock))
+			mod_memcg_state(memcg, MEMCG_SOCK, stock_sock);
+
 		return;
 	}
 
@@ -2284,6 +2335,15 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 	WRITE_ONCE(stock->nr_pages[i], nr_pages);
 
 	local_unlock(&memcg_stock.lock);
+
+update_sock:
+	if (nr_sock)
+		mod_memcg_state(memcg, MEMCG_SOCK, -nr_sock);
+}
+
+static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
+{
+	__refill_stock(memcg, nr_pages, 0);
 }
 
 static bool is_memcg_drain_needed(struct memcg_stock_pcp *stock,
@@ -2299,7 +2359,8 @@ static bool is_memcg_drain_needed(struct memcg_stock_pcp *stock,
 		if (!memcg)
 			continue;
 
-		if (READ_ONCE(stock->nr_pages[i]) &&
+		if ((READ_ONCE(stock->nr_pages[i]) ||
+		     READ_ONCE(stock->nr_sock[i])) &&
 		    mem_cgroup_is_descendant(memcg, root_memcg)) {
 			flush = true;
 			break;
@@ -5629,6 +5690,9 @@ bool mem_cgroup_sk_charge(const struct sock *sk, unsigned int nr_pages,
 	if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
 		return memcg1_charge_skmem(memcg, nr_pages, gfp_mask);
 
+	if (likely(__consume_stock(memcg, nr_pages, nr_pages)))
+		return true;
+
 	if (try_charge_memcg(memcg, gfp_mask, nr_pages) == 0) {
 		mod_memcg_state(memcg, MEMCG_SOCK, nr_pages);
 		return true;
@@ -5651,9 +5715,7 @@ void mem_cgroup_sk_uncharge(const struct sock *sk, unsigned int nr_pages)
 		return;
 	}
 
-	mod_memcg_state(memcg, MEMCG_SOCK, -nr_pages);
-
-	refill_stock(memcg, nr_pages);
+	__refill_stock(memcg, nr_pages, nr_pages);
 }
 
 void mem_cgroup_flush_workqueue(void)
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 5/5] selftests: cgroup: Deflake test_memcg_sock() in test_memcontrol.c.
  2026-10-16  5:27 [PATCH 0/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
                   ` (3 preceding siblings ...)
  2026-10-16  5:27 ` [PATCH 4/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
@ 2026-10-16  5:27 ` Kuniyuki Iwashima
  4 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Shakeel Butt
  Cc: Muchun Song, Kuniyuki Iwashima, Kuniyuki Iwashima, linux-mm,
	cgroups

With MEMCG_SOCK batched in per-CPU memcg_stock, test_memcg_sock()
can fail in two places:

1. In tcp_client(), reading memory.stat only flushes rstat
   synchronously when vmstats->stats_updates exceeds
   MEMCG_CHARGE_BATCH * num_online_cpus().  Because mod_memcg_state()
   is now called less frequently, 16 read() iterations (0x10) on a
   64-CPU system can finish in a few milliseconds before
   stats_updates reaches the 4096-page threshold or the 2-second
   periodic flusher (FLUSH_TIME) runs, leaving "sock " at 0.

2. After the TCP connection is closed, up to MEMCG_CHARGE_BATCH - 1
   pages of MEMCG_SOCK delta can remain cached in per-CPU
   stock->nr_sock[] (e.g. when the initial charge published +n via
   try_charge_memcg() and mod_memcg_state(), while the uncharge was
   absorbed into stock->nr_sock[] as -n without overflowing
   stock->nr_pages[]).  Since the periodic rstat flusher does not
   drain memcg_stock, "sock " in memory.stat can stay non-zero across
   cg_read_key_long_poll().

Let's increase retries in tcp_client() from 0x10 to 0x40 and write
"0" to memory.high before polling memory.stat at the end of
test_memcg_sock() to trigger drain_all_stock() and flush
stock->nr_sock[].

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 tools/testing/selftests/cgroup/test_memcontrol.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 0ed82347044e..1311aa5bc522 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -1333,7 +1333,7 @@ static int tcp_client(const char *cgroup, unsigned short port)
 	const char server[] = "localhost";
 	struct addrinfo *ai;
 	char servport[6];
-	int retries = 0x10; /* nice round number */
+	int retries = 0x40; /* nice round number */
 	int sk, ret;
 	long allocated;
 
@@ -1447,6 +1447,13 @@ static int test_memcg_sock(const char *root)
 	if (cg_read_long(memcg, "memory.current") < 0)
 		goto cleanup;
 
+	/*
+	 * Drain per-CPU memcg_stock so cached MEMCG_SOCK deltas in
+	 * stock->nr_sock[] are flushed to vmstats.
+	 */
+	if (cg_write(memcg, "memory.high", "0"))
+		goto cleanup;
+
 	/*
 	 * memory.stat is updated asynchronously via the memcg rstat
 	 * flushing worker, which runs periodically (every 2 seconds,
-- 
2.56.0.212.g129ede20d9-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-16  5:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16  5:27 [PATCH 0/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
2026-10-16  5:27 ` [PATCH 1/5] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
2026-10-16  5:27 ` [PATCH 2/5] memcg: Reduce nesting in consume_stock() and refill_stock() Kuniyuki Iwashima
2026-10-16  5:27 ` [PATCH 3/5] memcg: Annotate slow paths " Kuniyuki Iwashima
2026-10-16  5:27 ` [PATCH 4/5] memcg: Batch mod_memcg_state() for MEMCG_SOCK Kuniyuki Iwashima
2026-10-16  5:27 ` [PATCH 5/5] selftests: cgroup: Deflake test_memcg_sock() in test_memcontrol.c Kuniyuki Iwashima

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox