From: Usama Arif <usama.arif@linux.dev>
To: David Laight <david.laight.linux@gmail.com>
Cc: 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,
torvalds@linux-foundation.org, kas@kernel.org,
hannes@cmpxchg.org, riel@surriel.com, shakeel.butt@linux.dev,
kernel-team@meta.com
Subject: Re: [PATCH v2] x86/lib: Use REP MOVSB for large memcpy() on ERMS CPUs without FSRM
Date: Thu, 5 Nov 2026 17:28:45 +0000 [thread overview]
Message-ID: <39da378e-eac3-49cf-9ac4-7bd02d70eb37@linux.dev> (raw)
In-Reply-To: <20261105081205.5a84e4cb@pumpkin>
On 05/11/2026 08:12, David Laight wrote:
> On Wed, 4 Nov 2026 14:28:06 -0800
> Usama Arif <usama.arif@linux.dev> wrote:
>
>> Since commit 68674f94ffc9 ("x86: don't use REP_GOOD or ERMS for small
>> memory copies"), memcpy() uses REP MOVSB only on FSRM CPUs, so ERMS CPUs
>> without FSRM, such as Skylake through Cooper Lake, copy every size with
>> the scalar loop. Large copies matter there: in Meta's x86 fleet, about a
>> third of the time in memcpy_orig() is btrfs zstd decompression copying
>> pages into and out of zstd's buffers.
>>
>> Use REP MOVSB from 2048 bytes on ERMS CPUs. On a 1.4 GHz Cooper Lake, an
>> aligned, cache-hot 4 KiB memcpy() then takes 66.8% less time (123.4 ns
>> instead of 371.7 ns), and btrfs zstd reads take 4% less time. Smaller
>> copies keep the scalar loop: using REP MOVSB from 32 bytes made
>> 32/64/128/256-byte copies 114.9%/54.4%/66.1%/23.2% slower.
>>
>> To keep the dispatch cheap, fold memcpy_orig() into __memcpy(). With
>> FSRM, "rep movsb; RET" replaces the 32-byte length check. With ERMS, a
>> shift of the length in RDX selects REP MOVSB; RCX already holds the
>> count. Copies below 32 bytes skip the check, which is a single NOP
>> without ERMS.
>>
>> The FSRM replacement now contains a RET. apply_returns() runs before
>> apply_alternatives(), so with the ITS return thunk it decides whether to
>> keep the thunk from the RET's address in .altinstr_replacement. Align the
>> replacement to a cache line so that the RET keeps it; the thunk is safe
>> wherever the replacement is copied.
>>
>> Signed-off-by: Usama Arif <usama.arif@linux.dev>
>> ---
>> v1 -> v2: https://lore.kernel.org/r/20261013100442.2658311-1-usama.arif@linux.dev
>> - Raise the threshold from 512 to 2048 bytes. REP MOVSB was slower than
>> the scalar loop for some misaligned copies of 512 to 640 bytes.
>
> Which cpu type was that?
Cooper Lake (Xeon Platinum 8321HC).
> Do I remember the older zen cpu falling back to byte copies?
I just checked, and Zen 1 and Zen 2 have neither ERMS nor FSRM, so
they keep the scalar loop. Zen 3 onwards has both, and FSRM already
uses REP MOVSB for every size.
> I wonder if it is possibly to do a run-time patch of the constant.
It might be possible. But I wanted to keep the patch usecase driven.
The current usecase is coming from zstd/btrfs. If we find a usecase
where it might be faster for smaller copies, it might become worth
looking into then?
> In all the tests I've done on Intel cpu (going back to sandy bridge)
> the only alignment that made any difference was 64 byte (32 on older cpu)
> alignment of the destination buffer.
>
Also I should have been clearer in the changelog, by "misaligned copies of
512 to 640 bytes", I had tested a 513-byte copy with the source at page offset
1 and destination at 63, it took on average 62 ns with REP MOVSB versus
49 ns with scalar. I had done a couple of other tests at 640 bytes as well.
> That could be 'for further study'.
>
> There are some very bad alignment pairs on some cpu.
> On zen5 if (IIRC) (src - dest) % PAGE_SIZE is between 1 and 127 then
> the both setup and per-byte cost of REP MOSVB are significantly higher
> (the byte alignment makes minimal difference throughout).
I am seeing similar on Cooper Lake.
> I don't remember finding anything similar on the Intel cpu I tested.
> But it is probably not worth checking for them because the cost of the
> comparison will be larger overall.
>
Yes, I agree.
Do you think this version of the patch seems acceptable?
I feel like we should only investigate for lower size if we can find a
usecase.
> David
>
>> - Fold memcpy_orig() into __memcpy(). With FSRM, "rep movsb; RET" now
>> replaces the 32-byte length check (David Laight, Linus Torvalds). In
>> v1, the 13-byte empty FSRM replacement became a jump that FSRM CPUs
>> took on every call (Kiryl Shutsemau).
>> - Do the ERMS check after the 32-byte check, so copies below 32 bytes
>> skip it (Kiryl Shutsemau), and keep the count in RCX so REP MOVSB
>> needs no reload (David Laight).
>> - Align the FSRM replacement so that its RET keeps the ITS return thunk.
>> - Fix the comment that called the return value the source input.
>> ---
>> arch/x86/lib/memcpy_64.S | 101 ++++++++++++++++++++++-----------------
>> 1 file changed, 57 insertions(+), 44 deletions(-)
>>
>> diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
>> index 12a23fa7c44c5..dc71b7d1bfd58 100644
>> --- a/arch/x86/lib/memcpy_64.S
>> +++ b/arch/x86/lib/memcpy_64.S
>> @@ -27,28 +27,33 @@
>> *
>> * The 'rep movsb' itself is small enough to replace the call, but the
>> * two register moves blow up the code. And one of them is "needed"
>> - * only for the return value that is the same as the source input,
>> + * only for the return value that is the same as the destination input,
>> * 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. Align the replacement
>> + * source so its return is in the first half of a cache line and ITS
>> + * retains its thunk. The copied return can then execute in either
>> + * half of a cache line.
>> + */
>> + .pushsection .altinstr_replacement, "ax"
>> + .balign 64
>> + .popsection
>> +#endif
>> + ALTERNATIVE __stringify(cmpq $0x20, %rcx; jb .Lhandle_tail), \
>> + __stringify(rep movsb; RET), X86_FEATURE_FSRM
>>
>> - cmpq $0x20, %rdx
>> - jb .Lhandle_tail
>> + /*
>> + * RCX holds the count for REP MOVSB and scalar copying; RDX is scratch.
>> + * Shift RDX to check length >= 2048 without destroying the REP count.
>> + * The 10-byte alternative fits a NOP without ERMS.
>> + */
>> + ALTERNATIVE "", __stringify(shrq $11, %rdx; jnz .Lerms), X86_FEATURE_ERMS
>>
>> /*
>> * We check whether memory false dependence could occur,
>> @@ -56,9 +61,9 @@ SYM_FUNC_START_LOCAL(memcpy_orig)
>> */
>> cmp %dil, %sil
>> jl .Lcopy_backward
>> - subq $0x20, %rdx
>> + subq $0x20, %rcx
>> .Lcopy_forward_loop:
>> - subq $0x20, %rdx
>> + subq $0x20, %rcx
>>
>> /*
>> * Move in blocks of 4x8 bytes:
>> @@ -75,23 +80,23 @@ SYM_FUNC_START_LOCAL(memcpy_orig)
>> movq %r11, 3*8(%rdi)
>> leaq 4*8(%rdi), %rdi
>> jae .Lcopy_forward_loop
>> - addl $0x20, %edx
>> + addl $0x20, %ecx
>> jmp .Lhandle_tail
>>
>> .Lcopy_backward:
>> /*
>> * Calculate copy position to tail.
>> */
>> - addq %rdx, %rsi
>> - addq %rdx, %rdi
>> - subq $0x20, %rdx
>> + addq %rcx, %rsi
>> + addq %rcx, %rdi
>> + subq $0x20, %rcx
>> /*
>> * At most 3 ALU operations in one cycle,
>> * so append NOPS in the same 16 bytes trunk.
>> */
>> .p2align 4
>> .Lcopy_backward_loop:
>> - subq $0x20, %rdx
>> + subq $0x20, %rcx
>> movq -1*8(%rsi), %r8
>> movq -2*8(%rsi), %r9
>> movq -3*8(%rsi), %r10
>> @@ -107,11 +112,11 @@ SYM_FUNC_START_LOCAL(memcpy_orig)
>> /*
>> * Calculate copy position to head.
>> */
>> - addl $0x20, %edx
>> - subq %rdx, %rsi
>> - subq %rdx, %rdi
>> + addl $0x20, %ecx
>> + subq %rcx, %rsi
>> + subq %rcx, %rdi
>> .Lhandle_tail:
>> - cmpl $16, %edx
>> + cmpl $16, %ecx
>> jb .Lless_16bytes
>>
>> /*
>> @@ -119,55 +124,63 @@ SYM_FUNC_START_LOCAL(memcpy_orig)
>> */
>> movq 0*8(%rsi), %r8
>> movq 1*8(%rsi), %r9
>> - movq -2*8(%rsi, %rdx), %r10
>> - movq -1*8(%rsi, %rdx), %r11
>> + movq -2*8(%rsi, %rcx), %r10
>> + movq -1*8(%rsi, %rcx), %r11
>> movq %r8, 0*8(%rdi)
>> movq %r9, 1*8(%rdi)
>> - movq %r10, -2*8(%rdi, %rdx)
>> - movq %r11, -1*8(%rdi, %rdx)
>> + movq %r10, -2*8(%rdi, %rcx)
>> + movq %r11, -1*8(%rdi, %rcx)
>> RET
>> .p2align 4
>> .Lless_16bytes:
>> - cmpl $8, %edx
>> + cmpl $8, %ecx
>> jb .Lless_8bytes
>> /*
>> * Move data from 8 bytes to 15 bytes.
>> */
>> movq 0*8(%rsi), %r8
>> - movq -1*8(%rsi, %rdx), %r9
>> + movq -1*8(%rsi, %rcx), %r9
>> movq %r8, 0*8(%rdi)
>> - movq %r9, -1*8(%rdi, %rdx)
>> + movq %r9, -1*8(%rdi, %rcx)
>> RET
>> .p2align 4
>> .Lless_8bytes:
>> - cmpl $4, %edx
>> + cmpl $4, %ecx
>> jb .Lless_3bytes
>>
>> /*
>> * Move data from 4 bytes to 7 bytes.
>> */
>> - movl (%rsi), %ecx
>> - movl -4(%rsi, %rdx), %r8d
>> - movl %ecx, (%rdi)
>> - movl %r8d, -4(%rdi, %rdx)
>> + movl (%rsi), %edx
>> + movl -4(%rsi, %rcx), %r8d
>> + movl %edx, (%rdi)
>> + movl %r8d, -4(%rdi, %rcx)
>> RET
>> .p2align 4
>> .Lless_3bytes:
>> - subl $1, %edx
>> + subl $1, %ecx
>> jb .Lend
>> /*
>> * Move data from 1 bytes to 3 bytes.
>> */
>> - movzbl (%rsi), %ecx
>> + movzbl (%rsi), %edx
>> jz .Lstore_1byte
>> movzbq 1(%rsi), %r8
>> - movzbq (%rsi, %rdx), %r9
>> + movzbq (%rsi, %rcx), %r9
>> movb %r8b, 1(%rdi)
>> - movb %r9b, (%rdi, %rdx)
>> + movb %r9b, (%rdi, %rcx)
>> .Lstore_1byte:
>> - movb %cl, (%rdi)
>> + movb %dl, (%rdi)
>>
>> .Lend:
>> RET
>> -SYM_FUNC_END(memcpy_orig)
>>
>> +.Lerms:
>> + rep movsb
>> + RET
>> +SYM_FUNC_END(__memcpy)
>> +EXPORT_SYMBOL(__memcpy)
>> +
>> +SYM_FUNC_ALIAS_MEMFUNC(memcpy, __memcpy)
>> +SYM_PIC_ALIAS(memcpy)
>> +EXPORT_SYMBOL(memcpy)
>
prev parent reply other threads:[~2026-11-05 17:28 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 22:28 Usama Arif
2026-11-05 8:12 ` David Laight
2026-11-05 17:28 ` Usama Arif [this message]
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=39da378e-eac3-49cf-9ac4-7bd02d70eb37@linux.dev \
--to=usama.arif@linux.dev \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=david.laight.linux@gmail.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=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®