mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Usama Arif <usama.arif@linux.dev>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Kiryl Shutsemau <kas@kernel.org>,
	bp@alien8.de, dave.hansen@linux.intel.com, dsterba@suse.com,
	hpa@zytor.com, linux-kernel@vger.kernel.org, mingo@redhat.com,
	terrelln@fb.com, tglx@kernel.org, x86@kernel.org,
	hannes@cmpxchg.org, riel@surriel.com, shakeel.butt@linux.dev,
	kernel-team@meta.com
Subject: Re: [PATCH] x86/lib: Use REP MOVSB for large memcpy() on ERMS CPUs without FSRM
Date: Tue, 3 Nov 2026 08:32:38 +0000	[thread overview]
Message-ID: <20261103083238.672cdfa5@pumpkin> (raw)
In-Reply-To: <5f6fa235-55bc-45c6-90a8-20f75152e9f2@linux.dev>

On Mon, 2 Nov 2026 23:03:39 +0000
Usama Arif <usama.arif@linux.dev> wrote:

> On 22/10/2026 10:49, David Laight wrote:
> > On Wed, 21 Oct 2026 08:18:25 -1000
> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >   
> >> On Tue, 20 Oct 2026 at 23:55, Kiryl Shutsemau <kas@kernel.org> wrote:  
> >>>
> >>> Moving MOVQ inside ALTERNATIVE_2 is ugly...    
> >>
> >> Yeah, maybe it would make sense to make that function be
> >>
> >>         movq %rdi, %rax
> >>         movq %rdx, %rcx
> >>         ALTERNATIVE "jmp memcpy_orig", "rep movsb", X86_FEATURE_FSRM
> >>         RET
> >>
> >> instead? No nop instructions at all, and both alternatives are two bytes.
> >>
> >> Then do any ERMS logic at the top of memcpy_orig, which now has the
> >> registers ready for string instructions anyway.  
> > 
> > Is it possibly to just patch "rep movsb; RET" over whatever gets generated
> > for memcpy_orig (after the two register moves) ?
> > Then there are neither nops nor a jmp.
> > 
> > David
> >   
> Hi,
> 
> Sorry for the late reply. I had taken time off for the past couple of weeks
> and didnt have access to my work station.
> 
> I implemented Davids suggestion and I think it looks good. The scalar body is
> now part of __memcpy(), and FSRM replaces the existing 32-byte check with
> "rep movsb; RET". Non-FSRM calls fall straight into scalar code, and FSRM
> returns before any padding.
> 
> I have retained the 2048-byte ERMS cutoff, since selecting REP from 32 bytes
> regressed short copies in the Cooper Lake tests.
> 
> I asked LLM to review it and it indicated a problem with ITS: returns are
> patched before alternatives are copied, so the return decision uses the
> replacement's source address. I align replacement storage outside the
> copied range so ITS retains the return thunk and relocation preserves its
> target. This adds no executed padding.
> 
> Any thoughts on below? I will send it as a proper patch for review
> if it looks acceptable.
> 
> 
> From 9baffefb71c7223702c7bcd96d43b9a2828f3939 Mon Sep 17 00:00:00 2001
> From: Usama Arif <usama.arif@linux.dev>
> Date: Thu, 1 Oct 2026 14:15:12 -0700
> Subject: [PATCH] x86/lib: Use REP MOVSB for large memcpy() on ERMS CPUs
>  without FSRM
> 
> Commit 68674f94ffc9 ("x86: don't use REP_GOOD or ERMS for small
> memory copies") left ERMS CPUs without FSRM, including Cooper Lake and
> Skylake, using scalar code for all out-of-line copies. Btrfs/zstd input
> staging and output flushing accounted for about 34% of memcpy_orig()'s
> sampled weight in Meta's x86 fleet profile.
> 
> Use REP MOVSB from 2048 bytes on ERMS CPUs without FSRM, keeping scalar
> copies below that threshold. FSRM continues to use REP for all sizes.
> 
> Merge scalar code into __memcpy() and overlay its existing 32-byte check
> with REP MOVSB and RET on FSRM, removing the non-FSRM entry jump. Copies
> below 32 bytes bypass the ERMS gate. SHR11/JNZ fits one ten-byte NOP
> without ERMS; the REP arm reloads the original count from RDX.
> 
> With ITS configured, align replacement storage before its start label so
> the raw FSRM return retains its thunk before relocation. The alignment
> padding is outside the copied range.
> 
> On Cooper Lake (boost disabled, capped at 1.401 GHz), plain-RET userspace
> replicas with aligned warmed buffers and entry offset zero modulo 64
> measured 4 KiB copies at 371.7 ns originally and 123.4 ns with this patch:
> a 66.8% paired reduction across five process runs. A separate sweep using
> REP from 32 bytes increased 32/64/128/256-byte times by
> 114.9%/54.4%/66.1%/23.2%, respectively.
> 
> Small-copy costs remain: a warmed aligned zero-byte copy at entry offset
> 48 modulo 64 took a paired 0.75 ns more (+7.9%) than the original, in all
> five runs. These microbenchmarks do not establish a kernel workload gain
> or the best threshold across CPUs.
> 
> Direct copy checks and memcpy KUnit passed with all four ERMS/FSRM
> combinations and with ITS forced on; the copied return target was verified.
> 
> Signed-off-by: Usama Arif <usama.arif@linux.dev>
> ---
>  arch/x86/lib/memcpy_64.S | 42 ++++++++++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
> index 12a23fa7c44c5..407b226d4de2f 100644
> --- a/arch/x86/lib/memcpy_64.S
> +++ b/arch/x86/lib/memcpy_64.S
> @@ -31,24 +31,27 @@
>   * which the compiler could/should do much better anyway.
>   */
>  SYM_TYPED_FUNC_START(__memcpy)
> -	ALTERNATIVE "jmp memcpy_orig", "", X86_FEATURE_FSRM
> -
>  	movq %rdi, %rax
>  	movq %rdx, %rcx
> -	rep movsb
> -	RET
> -SYM_FUNC_END(__memcpy)
> -EXPORT_SYMBOL(__memcpy)
> -
> -SYM_FUNC_ALIAS_MEMFUNC(memcpy, __memcpy)
> -SYM_PIC_ALIAS(memcpy)
> -EXPORT_SYMBOL(memcpy)
>  
> -SYM_FUNC_START_LOCAL(memcpy_orig)
> -	movq %rdi, %rax
> +#ifdef CONFIG_MITIGATION_ITS
> +	/*
> +	 * Returns are patched before alternatives. Keep the replacement
> +	 * return in the first half of a cache line so ITS retains its thunk
> +	 * when the instruction is copied to its final address.

I'm not 100% sure what that is about, but does it mean that the cache line
alignment of the code here also matters?

> +	 */
> +	.pushsection .altinstr_replacement, "ax"
> +	.balign 64
> +	.popsection
> +#endif
> +	ALTERNATIVE __stringify(cmpq $0x20, %rdx; jb .Lhandle_tail), \
> +		    __stringify(rep movsb; RET), X86_FEATURE_FSRM
>  
> -	cmpq $0x20, %rdx
> -	jb .Lhandle_tail
> +	/*
> +	 * Prepared RCX is scratch for checking length >= 2048.
> +	 * The 10-byte alternative fits a NOP without ERMS.
> +	 */
> +	ALTERNATIVE "", __stringify(shrq $11, %rcx; jnz .Lerms), X86_FEATURE_ERMS

Can you use %rdx here, preserving %rcx for the 'rep movsb'?
You might need to swap cx<=>dx in the rest of the function.
(I'm sure we could use #defines to give names to registers....)

David

>  	/*
>  	 * We check whether memory false dependence could occur,
> @@ -169,5 +172,14 @@ SYM_FUNC_START_LOCAL(memcpy_orig)
>  
>  .Lend:
>  	RET
> -SYM_FUNC_END(memcpy_orig)
>  
> +.Lerms:
> +	movq %rdx, %rcx
> +	rep movsb
> +	RET
> +SYM_FUNC_END(__memcpy)
> +EXPORT_SYMBOL(__memcpy)
> +
> +SYM_FUNC_ALIAS_MEMFUNC(memcpy, __memcpy)
> +SYM_PIC_ALIAS(memcpy)
> +EXPORT_SYMBOL(memcpy)


  reply	other threads:[~2026-11-03  8:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13 10:04 Usama Arif
2026-10-15 10:39 ` Kiryl Shutsemau
2026-10-17 19:27   ` Usama Arif
2026-10-21  9:54     ` Kiryl Shutsemau
2026-10-21 18:18       ` Linus Torvalds
2026-10-22  8:49         ` David Laight
2026-11-02 23:03           ` Usama Arif
2026-11-03  8:32             ` David Laight [this message]
2026-11-03 14:14               ` Usama Arif
2026-10-17 21:42 ` David Laight

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=20261103083238.672cdfa5@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dsterba@suse.com \
    --cc=hannes@cmpxchg.org \
    --cc=hpa@zytor.com \
    --cc=kas@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=riel@surriel.com \
    --cc=shakeel.butt@linux.dev \
    --cc=terrelln@fb.com \
    --cc=tglx@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=usama.arif@linux.dev \
    --cc=x86@kernel.org \
    /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

all inboxes | Powered by JetHome®