Re: x86 copy performance regression

From: Linus Torvalds
Date: Fri May 26 2023 - 14:34:01 EST

On Fri, May 26, 2023 at 10:51 AM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> Hmmm
> [ 25.532236] RIP: 0010:0xffffffffa5a85134
> [ 25.536173] Code: Unable to access opcode bytes at 0xffffffffa5a8510a.

This was the other reason I really didn't want to use alternatives on
the conditional branch instructions. The relocations are really not
very natural, and we have odd rules for those things. So I suspect our
instruction rewriting simply gets this wrong, because that's such a
nasty pattern.

I really wanted my "just hardcode the instruction bytes" to work. Not
only did it get me the small 2-byte conditional jump, it meant that
there was no relocation on it. But objtool really hates not
understanding what the alternatives code does.

Which is fair enough, but it's frustrating here when it only results
in more problems.

Anyway, I guess *this* avoids all issues. It creates an extra jump to
a jump for the case where the CPU doesn't have ERMS, but I guess we
don't really care about those CPUs anyway.

And it avoids all the "alternative instructions have relocations"
issues. And it creates all small two-byte jumps, and the "rep movsb"
fits exactly on that same 2 bytes too. Which I guess all argues for
this being what I should have started with.

This time it *really* works.

Famous last words.

arch/x86/lib/copy_user_64.S | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
index 4fc5c2de2de4..01c5de4c279b 100644
--- a/arch/x86/lib/copy_user_64.S
+++ b/arch/x86/lib/copy_user_64.S
@@ -7,6 +7,8 @@

#include <linux/linkage.h>
+#include <asm/cpufeatures.h>
+#include <asm/alternative.h>
#include <asm/asm.h>
#include <asm/export.h>

@@ -29,7 +31,7 @@
cmpq $64,%rcx
- jae .Lunrolled
+ jae .Llarge

cmp $8,%ecx
jae .Lword
@@ -65,6 +67,12 @@ SYM_FUNC_START(rep_movs_alternative)
_ASM_EXTABLE_UA( 2b, .Lcopy_user_tail)
_ASM_EXTABLE_UA( 3b, .Lcopy_user_tail)

+0: ALTERNATIVE "jmp .Lunrolled", "rep movsb", X86_FEATURE_ERMS
+1: RET
+ _ASM_EXTABLE_UA( 0b, 1b)
.p2align 4
10: movq (%rsi),%r8