Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeel.butt@linux.dev>
To: Kuniyuki Iwashima <kuniyu@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	Muchun Song <muchun.song@linux.dev>,
	 Kuniyuki Iwashima <kuni1840@gmail.com>,
	linux-mm@kvack.org, cgroups@vger.kernel.org
Subject: Re: [PATCH v3 3/4] memcg: Annotate slow paths in consume_stock() and refill_stock().
Date: Thu, 29 Oct 2026 12:50:15 -0700	[thread overview]
Message-ID: <auOjX_O3vwou-lJ6@linux.dev> (raw)
In-Reply-To: <20261029194452.3189365-4-kuniyu@google.com>

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>



  reply	other threads:[~2026-10-29 19:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auOjX_O3vwou-lJ6@linux.dev \
    --to=shakeel.butt@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kuni1840@gmail.com \
    --cc=kuniyu@google.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=muchun.song@linux.dev \
    --cc=roman.gushchin@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox