[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFULd4atO8uy_5PbjV8vw-yk0tv_=4kh_JwJ+6Xq-2myStCnmw@mail.gmail.com>
Date: Sun, 1 Oct 2023 23:47:58 +0200
From: Uros Bizjak <ubizjak@...il.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
Andy Lutomirski <luto@...nel.org>,
Ingo Molnar <mingo@...nel.org>, Nadav Amit <namit@...are.com>,
Brian Gerst <brgerst@...il.com>,
Denys Vlasenko <dvlasenk@...hat.com>,
"H . Peter Anvin" <hpa@...or.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Borislav Petkov <bp@...en8.de>,
Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [RFC PATCH 0/4] x86/percpu: Use segment qualifiers
On Sun, Oct 1, 2023 at 10:21 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Sun, 1 Oct 2023 at 12:53, Uros Bizjak <ubizjak@...il.com> wrote:
> >
> > Regarding x86 target specific code, the same functionality used for
> > explicit address space is used internally to handle __thread
> > qualifier.
>
> Ok, that's interesting, in that __thread is certainly widely used so
> it will have seen testing.
>
> > Even *if* there are some issues with aliasing, the kernel
> > is immune to them due to
> >
> > KBUILD_CFLAGS += -fno-strict-aliasing
>
> It's not aliasing I'd worry about. It's correctness.
>
> And indeed, the *very* first thing I tried shows that this is all very
> very buggy in gcc.
>
> What did I try? A simple memory copy with a structure assignment.
>
> Try to compile this:
>
> #include <string.h>
> struct a { long arr[30]; };
>
> __seg_fs struct a m;
> void foo(struct a *dst) { *dst = m; }
>
> using the kernel compiler options (it's the "don't use sse/avx" ones
> that matter):
>
> gcc -mno-avx -mno-sse -O2 -S t.c
>
> and look at the end result. It's complete and utter sh*t:
>
> foo:
> xorl %eax, %eax
> cmpq $240, %rax
> jnb .L5
> .L2:
> movzbl %fs:m(%rax), %edx
> movb %dl, (%rdi,%rax)
> addq $1, %rax
> cmpq $240, %rax
> jb .L2
> .L5:
> ret
>
> to the point that I can only go "WTF"?
>
> I mean, it's not just that it does the copy one byte at a time. It
> literally compares %rax to $240 just after it has cleared it. I look
> at that code, and I go "a five-year old with a crayon could have done
> better".
>
> In other words, no, we're not using this thing that generates that
> kind of garbage.
>
> Somebody needs to open a bugzilla entry for this kind of code generation.
Huh, this testcase triggers known issue with IVopts. I opened
PR111657, but the issue with IVopts is already reported in PR79649
[2].
> Clang isn't much better, but at least it doesn't generate bad code. It
> just crashes with an internal compiler error on the above trivial
> test-case:
>
> fatal error: error in backend: cannot lower memory intrinsic in
> address space 257
>
> which at least tells the user that they can't copy memory from that
> address space. But once again shows that no, this feature is not ready
> for prime-time.
>
> If literally the *first* thing I thought to test was this broken, what
> else is broken in this model?
>
> And no, the kernel doesn't currently do the above kinds of things.
> That's not the point. The point was "how well is this compiler support
> tested". The answer is "not at all".
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111657
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79649
Uros.
Powered by blists - more mailing lists