Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] memcg: Misc cleanup and perf improvement for TCP.
@ 2026-10-29 19:44 Kuniyuki Iwashima
  2026-10-29 19:44 ` [PATCH v3 1/4] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-29 19:44 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 host w/ AMD EPYC 9B45 128-Core Processor (Zen 5 Turin,
2 sockets, 512 vCPUs), tcp_rr w/ 20000 flows throughput often
degrades by ~50%, 32M -> 17M~27M.

In the bad case, the number of involuntary context switches
was extremely high by 12x (241733 -> 3093367).

The root cause was that refill_stock() calls memcg_uncharge()
or drain_stock() while holding memcg_stock.lock.

Patch 1~3 are misc cleanup for better asm output and patch 4
moves heavy operations to out of local_lock(&memcg_stock.lock).

This series removes the flakiness and gives ~3% gain on the
512 vCPU platform.

In addition, on machine w/ AMD EPYC 7B12 64-Core Processor
(Zen 2 Rome, 2 sockets, 256 vCPU), tcp_rr w/ 10000 flows achieves
36% more throughput with lower latency.

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

  Metric                     |       Baseline |    Patched |    Diff
  ---------------------------+----------------+------------+--------
  Throughput (RPS)           |     20,884,499 | 28,453,051 | +36.24%
  Latency Mean (us)          |         478.46 |     351.07 | -26.63%
  Latency p50 (us)           |         453.96 |     343.04 | -24.43%
  Latency p90 (us)           |         669.00 |     479.57 | -28.32%
  Latency p99 (us)           |         969.38 |     593.92 | -38.73%


Changes:
  v3:
    * Drop MEMCG_SOCK batching patch
    * Patch 1: Be more specific in sentence 1
    * Patch 3: Mention Clang 23 and restore asm output
    * Patch 4: Add a new benchmark result without batching stats patch.

  v2: https://lore.kernel.org/linux-mm/20261028185034.3607746-1-kuniyu@google.com/
    * Rebased on mm.git/mm-new
    * Add patch 5 for flaky throughput issue
    * Patch 4:
      * Benchmark again after rebase and update numbers
      * Check in_nmi() in __refill_stock()

  v1: https://lore.kernel.org/linux-mm/20261016052811.3085635-1-kuniyu@google.com/


Kuniyuki Iwashima (4):
  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: Uncharge pages outside memcg_stock.lock in refill_stock().

 mm/memcontrol.c | 95 +++++++++++++++++++++++++++++--------------------
 1 file changed, 57 insertions(+), 38 deletions(-)

-- 
2.56.0.317.g454f8302e1-goog



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

* [PATCH v3 1/4] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock.
  2026-10-29 19:44 [PATCH v3 0/4] memcg: Misc cleanup and perf improvement for TCP Kuniyuki Iwashima
@ 2026-10-29 19:44 ` Kuniyuki Iwashima
  2026-10-29 19:58   ` Shakeel Butt
  2026-10-29 19:44 ` [PATCH v3 2/4] memcg: Reduce nesting in consume_stock() and refill_stock() Kuniyuki Iwashima
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-29 19:44 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)->{cached[],nr_pages[]} are 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>
---
v3: Be more specific in sentence 1
---
 mm/memcontrol.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 84f4a8fd4d16..8a401550dfaa 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2164,10 +2164,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);
@@ -2197,13 +2197,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);
@@ -2284,11 +2284,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;
 			if (stock_pages > MEMCG_STOCK_HIGH) {
 				memcg_uncharge(memcg,
 					       stock_pages - MEMCG_STOCK_LOW);
-- 
2.56.0.317.g454f8302e1-goog



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

* [PATCH v3 2/4] memcg: Reduce nesting in consume_stock() and refill_stock().
  2026-10-29 19:44 [PATCH v3 0/4] memcg: Misc cleanup and perf improvement for TCP Kuniyuki Iwashima
  2026-10-29 19:44 ` [PATCH v3 1/4] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
@ 2026-10-29 19:44 ` Kuniyuki Iwashima
  2026-10-29 19:44 ` [PATCH v3 3/4] memcg: Annotate slow paths " Kuniyuki Iwashima
  2026-10-29 19:44 ` [PATCH v3 4/4] memcg: Uncharge pages outside memcg_stock.lock in refill_stock() Kuniyuki Iwashima
  3 siblings, 0 replies; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-29 19:44 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>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
---
 mm/memcontrol.c | 64 ++++++++++++++++++++++++++-----------------------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8a401550dfaa..6666acc9cee8 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2168,15 +2168,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) {
-				WRITE_ONCE(stock->cached[i], NULL);
-				css_put(&memcg->css);
-			}
-			ret = true;
+		if (stock_pages < nr_pages)
+			break;
+
+		stock_pages -= nr_pages;
+		WRITE_ONCE(stock->nr_pages[i], stock_pages);
+		if (!stock_pages) {
+			WRITE_ONCE(stock->cached[i], NULL);
+			css_put(&memcg->css);
 		}
+
+		ret = true;
 		break;
 	}
 
@@ -2258,7 +2260,6 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 	struct memcg_stock_pcp *stock;
 	struct mem_cgroup *cached;
 	unsigned int stock_pages;
-	bool success = false;
 	int empty_slot = -1;
 	int i;
 
@@ -2287,32 +2288,35 @@ 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;
-			if (stock_pages > MEMCG_STOCK_HIGH) {
-				memcg_uncharge(memcg,
-					       stock_pages - MEMCG_STOCK_LOW);
-				stock_pages = MEMCG_STOCK_LOW;
-			}
-			WRITE_ONCE(stock->nr_pages[i], stock_pages);
-			success = true;
-			break;
+
+		if (memcg != cached)
+			continue;
+
+		stock_pages = stock->nr_pages[i] + nr_pages;
+		if (stock_pages > MEMCG_STOCK_HIGH) {
+			memcg_uncharge(memcg,
+				       stock_pages - MEMCG_STOCK_LOW);
+			stock_pages = MEMCG_STOCK_LOW;
 		}
+
+		WRITE_ONCE(stock->nr_pages[i], stock_pages);
+
+		local_unlock(&memcg_stock.lock);
+		return;
 	}
 
-	if (!success) {
-		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);
+	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.317.g454f8302e1-goog



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

* [PATCH v3 3/4] memcg: Annotate slow paths in consume_stock() and refill_stock().
  2026-10-29 19:44 [PATCH v3 0/4] memcg: Misc cleanup and perf improvement for TCP Kuniyuki Iwashima
  2026-10-29 19:44 ` [PATCH v3 1/4] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
  2026-10-29 19:44 ` [PATCH v3 2/4] memcg: Reduce nesting in consume_stock() and refill_stock() Kuniyuki Iwashima
@ 2026-10-29 19:44 ` Kuniyuki Iwashima
  2026-10-29 19:50   ` Shakeel Butt
  2026-10-29 19:44 ` [PATCH v3 4/4] memcg: Uncharge pages outside memcg_stock.lock in refill_stock() Kuniyuki Iwashima
  3 siblings, 1 reply; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-29 19:44 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_STOCK_HIGH (64) pages, so depleting a slot in
consume_stock() (!stock_pages) or trimming a slot in refill_stock()
(stock_pages > MEMCG_STOCK_HIGH) 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), Clang 23 treats
these branches as likely and inlines css_put() in consume_stock()
and memcg_uncharge() 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
memcg_uncharge() 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;
   7ab2: 43 0f b6 44 26 01       movzbl  0x1(%r14,%r12), %eax
   7ab8: 01 c6                   addl    %eax, %esi
  if (stock_pages > MEMCG_STOCK_HIGH) {
   7aba: 83 fe 41                cmpl    $0x41, %esi
   7abd: 72 1d                   jb      0x7adc <refill_stock+0x21c>
  memcg_uncharge(memcg, stock_pages - MEMCG_STOCK_LOW);   # inlined
   7abf: 41 89 f7                movl    %esi, %r15d
   7ac2: 41 83 c7 e0             addl    $-0x20, %r15d
   7ac6: 48 8d bb 00 01 00 00    leaq    0x100(%rbx), %rdi
   7acd: 4c 89 fe                movq    %r15, %rsi
   7ad0: e8 00 00 00 00          callq   <page_counter_uncharge>
   7ad5: be 20 00 00 00          movl    $0x20, %esi
   7ada: eb 41                   jmp     0x7b1d <refill_stock+0x25d>
  WRITE_ONCE(stock->nr_pages[i], stock_pages);
   7adc: 43 88 74 26 01          movb    %sil, 0x1(%r14,%r12)
  local_unlock(&memcg_stock.lock);
   7ae1: 65 48 8b 05 00 00 00 00 movq    %gs:(%rip), %rax
   7ae9: c6 80 00 00 00 00 00    movb    $0x0, (%rax)
   7af0: e9 03 fe ff ff          jmp     0x78f8 <refill_stock+0x38>

After (refill_stock):

  stock_pages = stock->nr_pages[i] + nr_pages;
   7b76: 42 0f b6 44 33 01       movzbl  0x1(%rbx,%r14), %eax
   7b7c: 01 c6                   addl    %eax, %esi
  if (unlikely(stock_pages > MEMCG_STOCK_HIGH)) {
   7b7e: 83 fe 41                cmpl    $0x41, %esi
   7b81: 73 20                   jae     0x7ba3 <refill_stock+0x233>
  WRITE_ONCE(stock->nr_pages[i], stock_pages);
   7b83: 42 88 74 33 01          movb    %sil, 0x1(%rbx,%r14)
  local_unlock(&memcg_stock.lock);
   7b88: 65 48 8b 05 00 00 00 00 movq    %gs:(%rip), %rax
   7b90: c6 80 00 00 00 00 00    movb    $0x0, (%rax)
  }
   7b97: 5b                      popq    %rbx
   7b98: 41 5e                   popq    %r14
   7b9a: 41 5f                   popq    %r15
   7b9c: 5d                      popq    %rbp
   7b9d: 2e e9 00 00 00 00       jmp     <__x86_return_thunk>
  memcg_uncharge(memcg, stock_pages - MEMCG_STOCK_LOW);
   7ba3: 83 c6 e0                addl    $-0x20, %esi
   7ba6: e8 15 f2 ff ff          callq   0x6dc0 <memcg_uncharge>
   7bab: be 20 00 00 00          movl    $0x20, %esi
   7bb0: eb d1                   jmp     0x7b83 <refill_stock+0x213>

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v3: Mention Clang 23 and restore asm output
---
 mm/memcontrol.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6666acc9cee8..a04b44656b38 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2157,8 +2157,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);
@@ -2168,12 +2168,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)) {
 			WRITE_ONCE(stock->cached[i], NULL);
 			css_put(&memcg->css);
 		}
@@ -2273,8 +2273,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.
@@ -2293,7 +2293,7 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 			continue;
 
 		stock_pages = stock->nr_pages[i] + nr_pages;
-		if (stock_pages > MEMCG_STOCK_HIGH) {
+		if (unlikely(stock_pages > MEMCG_STOCK_HIGH)) {
 			memcg_uncharge(memcg,
 				       stock_pages - MEMCG_STOCK_LOW);
 			stock_pages = MEMCG_STOCK_LOW;
-- 
2.56.0.317.g454f8302e1-goog



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

* [PATCH v3 4/4] memcg: Uncharge pages outside memcg_stock.lock in refill_stock().
  2026-10-29 19:44 [PATCH v3 0/4] memcg: Misc cleanup and perf improvement for TCP Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2026-10-29 19:44 ` [PATCH v3 3/4] memcg: Annotate slow paths " Kuniyuki Iwashima
@ 2026-10-29 19:44 ` Kuniyuki Iwashima
  2026-10-29 19:52   ` Shakeel Butt
  3 siblings, 1 reply; 8+ messages in thread
From: Kuniyuki Iwashima @ 2026-10-29 19:44 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 host w/ AMD EPYC 9B45 128-Core Processor (Zen 5 Turin,
2 sockets, 512 vCPUs), tcp_rr w/ 20000 flows throughput often
degrades by ~50%, 32M -> 17M~27M.

In the bad case, the number of involuntary context switches
was extremely high by 12x (241733 -> 3093367).

In perf record, 96% samples had the same pattern, where the
thread was interrupted during page_counter_uncharge() by softirq,
and later ksoftirqd was woken up:

  # perf record -C 0-15 -e sched:sched_wakeup -g \
      -o /tmp/sched_wakeup.data -- sleep 1
  # perf report -i /tmp/sched_wakeup.data --stdio --no-children
    96.36%  tcp_rr   [kernel.kallsyms]  [k] wake_up_process
            |
            ---wake_up_process
               |
                --96.00%--irq_exit_rcu
                          |
                           --95.82%--sysvec_call_function_single
                                     asm_sysvec_call_function_single
                                     |
                                      --95.58%--page_counter_uncharge
                                                mem_cgroup_sk_uncharge
                                                sock_rfree
                                                tcp_recvmsg
                                                __x64_sys_recvfrom
                                                do_syscall_64
                                                entry_SYSCALL_64_after_hwframe

  # perf script -i /tmp/sched_wakeup.data | grep -A 20 "comm=ksoftirqd"
  tcp_rr  101952 [001]  9679.340985: sched:sched_wakeup: comm=ksoftirqd/1 pid=23 prio=120 target_cpu=001
  	ffffffffb64cd0cc wake_up_process+0xe6c ([kernel.kallsyms])
  	ffffffffb64cb034 irq_exit_rcu+0x264 ([kernel.kallsyms])
  	ffffffffb68ad31c sysvec_call_function_single+0xcc ([kernel.kallsyms])
  	ffffffffb560158a asm_sysvec_call_function_single+0x1a ([kernel.kallsyms])
  	ffffffffb653c4ce page_counter_uncharge+0x2e ([kernel.kallsyms])
  	ffffffffb6541b28 mem_cgroup_sk_uncharge+0x3c8 ([kernel.kallsyms])
  	ffffffffb6561383 sock_rfree+0x83 ([kernel.kallsyms])
  	ffffffffb6577faf tcp_recvmsg+0x2ff ([kernel.kallsyms])
  	ffffffffb65604d9 __x64_sys_recvfrom+0x159 ([kernel.kallsyms])

The problem is that when stock_pages exceeds MEMCG_STOCK_HIGH
or all slots in struct memcg_stock_pcp are full, refill_stock()
calls memcg_uncharge() or drain_stock() while holding
memcg_stock.lock.

Because local_trylock() only disables preemption and leaves
interrupts and softirqs enabled, an RPS IPI arriving while
refill_stock() stalls on page_counter_uncharge() invokes
NET_RX_SOFTIRQ (process_backlog()) on the same CPU while
memcg_stock.lock is held.

Every incoming packet in process_backlog() then fails
local_trylock() in consume_stock() and falls back to
try_charge_memcg() and refill_stock(), hammering the global
page_counter atomic twice per packet, and slow processing
finally wakes up ksoftirqd.

Let's move memcg_uncharge() and css_put() outside
local_unlock(&memcg_stock.lock) in refill_stock().

With the change, the flakiness disappeared on the 512 vCPU platform.

In addition, on machine w/ AMD EPYC 7B12 64-Core Processor (Zen 2
Rome, 2 sockets, 256 vCPU), tcp_rr w/ 10000 flows achieves 36% more
throughput with lower latency.

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

  Metric                     |       Baseline |    Patched |    Diff
  ---------------------------+----------------+------------+--------
  Throughput (RPS)           |     20,884,499 | 28,453,051 | +36.24%
  Latency Mean (us)          |         478.46 |     351.07 | -26.63%
  Latency p50 (us)           |         453.96 |     343.04 | -24.43%
  Latency p90 (us)           |         669.00 |     479.57 | -28.32%
  Latency p99 (us)           |         969.38 |     593.92 | -38.73%

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v3: Add a new benchmark result without batching stats patch.
---
 mm/memcontrol.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a04b44656b38..ac43c895490a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2294,23 +2294,31 @@ static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
 
 		stock_pages = stock->nr_pages[i] + nr_pages;
 		if (unlikely(stock_pages > MEMCG_STOCK_HIGH)) {
-			memcg_uncharge(memcg,
-				       stock_pages - MEMCG_STOCK_LOW);
-			stock_pages = MEMCG_STOCK_LOW;
+			WRITE_ONCE(stock->nr_pages[i], MEMCG_STOCK_LOW);
+			stock_pages -= MEMCG_STOCK_LOW;
+		} else {
+			WRITE_ONCE(stock->nr_pages[i], stock_pages);
+			stock_pages = 0;
 		}
 
-		WRITE_ONCE(stock->nr_pages[i], stock_pages);
-
 		local_unlock(&memcg_stock.lock);
+
+		if (unlikely(stock_pages))
+			memcg_uncharge(memcg, stock_pages);
 		return;
 	}
 
+	cached = NULL;
+	stock_pages = 0;
+
 	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);
+
+		cached = stock->cached[i];
+		stock_pages = stock->nr_pages[i];
 	}
 
 	css_get(&memcg->css);
@@ -2318,6 +2326,13 @@ 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);
+
+	if (unlikely(cached)) {
+		if (stock_pages)
+			memcg_uncharge(cached, stock_pages);
+
+		css_put(&cached->css);
+	}
 }
 
 static bool is_memcg_drain_needed(struct memcg_stock_pcp *stock,
-- 
2.56.0.317.g454f8302e1-goog



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

* Re: [PATCH v3 3/4] memcg: Annotate slow paths in consume_stock() and refill_stock().
  2026-10-29 19:44 ` [PATCH v3 3/4] memcg: Annotate slow paths " Kuniyuki Iwashima
@ 2026-10-29 19:50   ` Shakeel Butt
  0 siblings, 0 replies; 8+ messages in thread
From: Shakeel Butt @ 2026-10-29 19:50 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Muchun Song, Kuniyuki Iwashima, linux-mm, cgroups

On Thu, Oct 29, 2026 at 07:44:14PM +0000, Kuniyuki Iwashima wrote:
> In consume_stock() and refill_stock(), each per-CPU stock slot
> holds up to MEMCG_STOCK_HIGH (64) pages, so depleting a slot in
> consume_stock() (!stock_pages) or trimming a slot in refill_stock()
> (stock_pages > MEMCG_STOCK_HIGH) 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), Clang 23 treats
> these branches as likely and inlines css_put() in consume_stock()
> and memcg_uncharge() 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
> memcg_uncharge() 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;
>    7ab2: 43 0f b6 44 26 01       movzbl  0x1(%r14,%r12), %eax
>    7ab8: 01 c6                   addl    %eax, %esi
>   if (stock_pages > MEMCG_STOCK_HIGH) {
>    7aba: 83 fe 41                cmpl    $0x41, %esi
>    7abd: 72 1d                   jb      0x7adc <refill_stock+0x21c>
>   memcg_uncharge(memcg, stock_pages - MEMCG_STOCK_LOW);   # inlined
>    7abf: 41 89 f7                movl    %esi, %r15d
>    7ac2: 41 83 c7 e0             addl    $-0x20, %r15d
>    7ac6: 48 8d bb 00 01 00 00    leaq    0x100(%rbx), %rdi
>    7acd: 4c 89 fe                movq    %r15, %rsi
>    7ad0: e8 00 00 00 00          callq   <page_counter_uncharge>
>    7ad5: be 20 00 00 00          movl    $0x20, %esi
>    7ada: eb 41                   jmp     0x7b1d <refill_stock+0x25d>
>   WRITE_ONCE(stock->nr_pages[i], stock_pages);
>    7adc: 43 88 74 26 01          movb    %sil, 0x1(%r14,%r12)
>   local_unlock(&memcg_stock.lock);
>    7ae1: 65 48 8b 05 00 00 00 00 movq    %gs:(%rip), %rax
>    7ae9: c6 80 00 00 00 00 00    movb    $0x0, (%rax)
>    7af0: e9 03 fe ff ff          jmp     0x78f8 <refill_stock+0x38>
> 
> After (refill_stock):
> 
>   stock_pages = stock->nr_pages[i] + nr_pages;
>    7b76: 42 0f b6 44 33 01       movzbl  0x1(%rbx,%r14), %eax
>    7b7c: 01 c6                   addl    %eax, %esi
>   if (unlikely(stock_pages > MEMCG_STOCK_HIGH)) {
>    7b7e: 83 fe 41                cmpl    $0x41, %esi
>    7b81: 73 20                   jae     0x7ba3 <refill_stock+0x233>
>   WRITE_ONCE(stock->nr_pages[i], stock_pages);
>    7b83: 42 88 74 33 01          movb    %sil, 0x1(%rbx,%r14)
>   local_unlock(&memcg_stock.lock);
>    7b88: 65 48 8b 05 00 00 00 00 movq    %gs:(%rip), %rax
>    7b90: c6 80 00 00 00 00 00    movb    $0x0, (%rax)
>   }
>    7b97: 5b                      popq    %rbx
>    7b98: 41 5e                   popq    %r14
>    7b9a: 41 5f                   popq    %r15
>    7b9c: 5d                      popq    %rbp
>    7b9d: 2e e9 00 00 00 00       jmp     <__x86_return_thunk>
>   memcg_uncharge(memcg, stock_pages - MEMCG_STOCK_LOW);
>    7ba3: 83 c6 e0                addl    $-0x20, %esi
>    7ba6: e8 15 f2 ff ff          callq   0x6dc0 <memcg_uncharge>
>    7bab: be 20 00 00 00          movl    $0x20, %esi
>    7bb0: eb d1                   jmp     0x7b83 <refill_stock+0x213>
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>



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

* Re: [PATCH v3 4/4] memcg: Uncharge pages outside memcg_stock.lock in refill_stock().
  2026-10-29 19:44 ` [PATCH v3 4/4] memcg: Uncharge pages outside memcg_stock.lock in refill_stock() Kuniyuki Iwashima
@ 2026-10-29 19:52   ` Shakeel Butt
  0 siblings, 0 replies; 8+ messages in thread
From: Shakeel Butt @ 2026-10-29 19:52 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Muchun Song, Kuniyuki Iwashima, linux-mm, cgroups

On Thu, Oct 29, 2026 at 07:44:15PM +0000, Kuniyuki Iwashima wrote:
> On host w/ AMD EPYC 9B45 128-Core Processor (Zen 5 Turin,
> 2 sockets, 512 vCPUs), tcp_rr w/ 20000 flows throughput often
> degrades by ~50%, 32M -> 17M~27M.
> 
> In the bad case, the number of involuntary context switches
> was extremely high by 12x (241733 -> 3093367).
> 
> In perf record, 96% samples had the same pattern, where the
> thread was interrupted during page_counter_uncharge() by softirq,
> and later ksoftirqd was woken up:
> 
>   # perf record -C 0-15 -e sched:sched_wakeup -g \
>       -o /tmp/sched_wakeup.data -- sleep 1
>   # perf report -i /tmp/sched_wakeup.data --stdio --no-children
>     96.36%  tcp_rr   [kernel.kallsyms]  [k] wake_up_process
>             |
>             ---wake_up_process
>                |
>                 --96.00%--irq_exit_rcu
>                           |
>                            --95.82%--sysvec_call_function_single
>                                      asm_sysvec_call_function_single
>                                      |
>                                       --95.58%--page_counter_uncharge
>                                                 mem_cgroup_sk_uncharge
>                                                 sock_rfree
>                                                 tcp_recvmsg
>                                                 __x64_sys_recvfrom
>                                                 do_syscall_64
>                                                 entry_SYSCALL_64_after_hwframe
> 
>   # perf script -i /tmp/sched_wakeup.data | grep -A 20 "comm=ksoftirqd"
>   tcp_rr  101952 [001]  9679.340985: sched:sched_wakeup: comm=ksoftirqd/1 pid=23 prio=120 target_cpu=001
>   	ffffffffb64cd0cc wake_up_process+0xe6c ([kernel.kallsyms])
>   	ffffffffb64cb034 irq_exit_rcu+0x264 ([kernel.kallsyms])
>   	ffffffffb68ad31c sysvec_call_function_single+0xcc ([kernel.kallsyms])
>   	ffffffffb560158a asm_sysvec_call_function_single+0x1a ([kernel.kallsyms])
>   	ffffffffb653c4ce page_counter_uncharge+0x2e ([kernel.kallsyms])
>   	ffffffffb6541b28 mem_cgroup_sk_uncharge+0x3c8 ([kernel.kallsyms])
>   	ffffffffb6561383 sock_rfree+0x83 ([kernel.kallsyms])
>   	ffffffffb6577faf tcp_recvmsg+0x2ff ([kernel.kallsyms])
>   	ffffffffb65604d9 __x64_sys_recvfrom+0x159 ([kernel.kallsyms])
> 
> The problem is that when stock_pages exceeds MEMCG_STOCK_HIGH
> or all slots in struct memcg_stock_pcp are full, refill_stock()
> calls memcg_uncharge() or drain_stock() while holding
> memcg_stock.lock.
> 
> Because local_trylock() only disables preemption and leaves
> interrupts and softirqs enabled, an RPS IPI arriving while
> refill_stock() stalls on page_counter_uncharge() invokes
> NET_RX_SOFTIRQ (process_backlog()) on the same CPU while
> memcg_stock.lock is held.
> 
> Every incoming packet in process_backlog() then fails
> local_trylock() in consume_stock() and falls back to
> try_charge_memcg() and refill_stock(), hammering the global
> page_counter atomic twice per packet, and slow processing
> finally wakes up ksoftirqd.
> 
> Let's move memcg_uncharge() and css_put() outside
> local_unlock(&memcg_stock.lock) in refill_stock().
> 
> With the change, the flakiness disappeared on the 512 vCPU platform.
> 
> In addition, on machine w/ AMD EPYC 7B12 64-Core Processor (Zen 2
> Rome, 2 sockets, 256 vCPU), tcp_rr w/ 10000 flows achieves 36% more
> throughput with lower latency.
> 
>   tcp_rr (10000 flows, 128 threads, 60s, average of 3 runs):
> 
>   Metric                     |       Baseline |    Patched |    Diff
>   ---------------------------+----------------+------------+--------
>   Throughput (RPS)           |     20,884,499 | 28,453,051 | +36.24%
>   Latency Mean (us)          |         478.46 |     351.07 | -26.63%
>   Latency p50 (us)           |         453.96 |     343.04 | -24.43%
>   Latency p90 (us)           |         669.00 |     479.57 | -28.32%
>   Latency p99 (us)           |         969.38 |     593.92 | -38.73%
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

* Re: [PATCH v3 1/4] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock.
  2026-10-29 19:44 ` [PATCH v3 1/4] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
@ 2026-10-29 19:58   ` Shakeel Butt
  0 siblings, 0 replies; 8+ messages in thread
From: Shakeel Butt @ 2026-10-29 19:58 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Roman Gushchin,
	Muchun Song, Kuniyuki Iwashima, linux-mm, cgroups

On Thu, Oct 29, 2026 at 07:44:12PM +0000, Kuniyuki Iwashima wrote:
> this_cpu_ptr(&memcg_stock)->{cached[],nr_pages[]} are 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>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


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

end of thread, other threads:[~2026-10-29 19:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-29 19:44 [PATCH v3 0/4] memcg: Misc cleanup and perf improvement for TCP Kuniyuki Iwashima
2026-10-29 19:44 ` [PATCH v3 1/4] memcg: Remove unnecessary READ_ONCE() under memcg_stock.lock Kuniyuki Iwashima
2026-10-29 19:58   ` Shakeel Butt
2026-10-29 19:44 ` [PATCH v3 2/4] memcg: Reduce nesting in consume_stock() and refill_stock() Kuniyuki Iwashima
2026-10-29 19:44 ` [PATCH v3 3/4] memcg: Annotate slow paths " Kuniyuki Iwashima
2026-10-29 19:50   ` Shakeel Butt
2026-10-29 19:44 ` [PATCH v3 4/4] memcg: Uncharge pages outside memcg_stock.lock in refill_stock() Kuniyuki Iwashima
2026-10-29 19:52   ` Shakeel Butt

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