All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.