linux-kernel - Re: x86 copy performance regression

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whZ23EHnBG4ox9QpHFDeiCSrA2H1wrYrfyg3KP=zK5Sog@mail.gmail.com>
Date:   Fri, 26 May 2023 11:33:25 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>
Subject: Re: x86 copy performance regression

On Fri, May 26, 2023 at 10:51 AM Eric Dumazet <edumazet@...gle.com> 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.

                Linus

View attachment "patch.diff" of type "text/x-patch" (877 bytes)