On Mon, Jul 27, 2026 at 08:34:27PM +0800, Li Zhe wrote:
> The new x86 memcpy_nontemporal() helper in this series maps to

s/in this series//

That's implicit.

> memcpy_flushcache(), and the ZONE_DEVICE fast path uses that primitive
> for constant-sized struct page template copies.
> 
> The immediately relevant x86_64 sizeof(struct page) values here are 64,
> 80, and 96 bytes. memcpy_flushcache() currently inlines only the 4, 8,
> and 16-byte cases, so even those constant-sized struct page copies fall
> through to __memcpy_flushcache().
> 
> Add 32, 48, 64, 80, and 96-byte MOVNTI sequences to the main fixed-size
> switch so those constant-sized copies can stay on the inline path.
> Factor the larger sequences into movnti_8()/16()/32()/64() helpers so
> the switch can reuse them without duplicating the inline assembly.
> While the ZONE_DEVICE template-copy path only needs 64/80/96-byte
> copies, keep 32-byte and 48-byte copies inline as well rather than
> sending those nearby fixed-size cases back to __memcpy_flushcache().

Do not explain the code - explain *why* those sizes. I already asked a bunch
of times.

> 
> These new fixed-size cases follow the existing memcpy_flushcache()
> fixed-size MOVNTI cases. The existing 4/8/16-byte cases already use
> MOVNTI without a separate destination alignment check.
> 
> The movnti_8()/16()/32()/64() helpers keep the "memory" clobber
> intentionally. The destination memory operand describes the MOVNTI
> destination, but it does not express the broader compiler-scheduling
> constraint that this path wants: keep the fixed-size copy as a compact
> sequence of streaming stores and avoid moving unrelated memory accesses
> into the middle of that sequence.

A lot of bla for its own sake.

> A microbenchmark compared two memcpy_flushcache()-style helpers that
> shared the same generic body and differed only in whether
> 32/48/64/80/96-byte copies stayed in the fixed-size switch or were
> redirected to __memcpy_flushcache() when the destination was not 8-byte
> aligned. The timed interval covered the copy loop only; wmb() was used
> only to separate rounds.

So we're doing this only for a microbenchmark's sake?

> In pseudo-code, the two variants were:
> 
> variant A:
>       switch (len) {
>               case 32:
>               case 48:
>               case 64:
>               case 80:
>               case 96:
>                       do fixed-size MOVNTI stores;
>                       return;
>               default:
>                       generic __memcpy_flushcache()-style body;
>       }
> 
> variant B:
>       switch (len) {
>               case 32:
>               case 48:
>               case 64:
>               case 80:
>               case 96:
>                       if (!IS_ALIGNED(dst, 8))
>                               goto generic_path;
>                       do fixed-size MOVNTI stores;
>                       return;
>               default:
>       generic_path:
>                       generic __memcpy_flushcache()-style body;
>       }
> 
> For offsets 1..7 averaged, keeping those sizes in the fixed-size
> switch took about 0.410/0.411/0.426/0.426/0.428 us per
> 32/48/64/80/96-byte copy, while redirecting the same cases to
> __memcpy_flushcache() took about 0.962/0.956/0.923/0.998/1.001 us.
> 
> The correctness rationale for not adding a separate destination
> alignment check is that these cases follow the existing 4/8/16-byte
> memcpy_flushcache() MOVNTI cases, and I did not find Intel SDM text
> requiring an 8-byte aligned destination for MOVNTI.
> 
> The microbenchmark is only the performance motivation for keeping
> these small constant-size copies in the inline path.

So in the end of the commit message, I still don't know why we need this code.

I mean, it should be pretty trivial to explain: I'm adding those additional
sizes because it brings this and that. Srsly.

If you cannot justify that, why are you even doing it?

So from going over this again, the reason why you're adding those other sizes
are because struct page can be that big. Yes?

How do you know? What's the proof?

I'd expect a justification along the lines of: in a somewhat normal Linux
installation, struct size can be of size <bla>. Adding MOVNTI helpers of size
<bla> brings so and so performance uplift.

How hard it is to write it this way?

> Signed-off-by: Li Zhe <[email protected]>
> ---
>  arch/x86/include/asm/string_64.h | 56 +++++++++++++++++++++++++++++++-
>  1 file changed, 55 insertions(+), 1 deletion(-)

...

at least the code is making a lot more sense now.

The fact that you had to axe off so much cruft off of it tells me that you
haven't really measured it right.

So why do I really want your patch?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Reply via email to