* [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* 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
* [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* 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
* [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 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