Received-SPF: pass (google.com: domain of linux-kernel+bounces-108926-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240320083127.493250-1-ubizjak@gmail.com> <171093476000.10875.14076471223590027773.tip-bot2@tip-bot2>
 <ZfrMcyZXCBQD/sE8@gmail.com>
In-Reply-To: <ZfrMcyZXCBQD/sE8@gmail.com>
From: Uros Bizjak <ubizjak@gmail.com>
Date: Wed, 20 Mar 2024 14:12:14 +0100
Message-ID: <CAFULd4bNETbtP3VTGao4o3mtfpw6d=rhcWp5N+pnzp-f3fjXAw@mail.gmail.com>
Subject: Re: [tip: x86/percpu] x86/percpu: Convert this_percpu_xchg_op() from
 asm() to C code, to generate better code
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-tip-commits@vger.kernel.org, 
	"H. Peter Anvin" <hpa@zytor.com>, Linus Torvalds <torvalds@linux-foundation.org>, x86@kernel.org, 
	Nathan Chancellor <nathan@kernel.org>, Kees Cook <keescook@chromium.org>, 
	Josh Poimboeuf <jpoimboe@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 20, 2024 at 12:45=E2=80=AFPM Ingo Molnar <mingo@kernel.org> wro=
te:
>
>
> * tip-bot2 for Uros Bizjak <tip-bot2@linutronix.de> wrote:
>
> > The following commit has been merged into the x86/percpu branch of tip:
> >
> > Commit-ID:     0539084639f3835c8d0b798e6659ec14a266b4f1
> > Gitweb:        https://git.kernel.org/tip/0539084639f3835c8d0b798e6659e=
c14a266b4f1
> > Author:        Uros Bizjak <ubizjak@gmail.com>
> > AuthorDate:    Wed, 20 Mar 2024 09:30:40 +01:00
> > Committer:     Ingo Molnar <mingo@kernel.org>
> > CommitterDate: Wed, 20 Mar 2024 12:29:02 +01:00
> >
> > x86/percpu: Convert this_percpu_xchg_op() from asm() to C code, to gene=
rate better code
> >
> > Rewrite percpu_xchg_op() using generic percpu primitives instead
> > of using asm. The new implementation is similar to local_xchg() and
> > allows the compiler to perform various optimizations: e.g. the
> > compiler is able to create fast path through the loop, according
> > to likely/unlikely annotations in percpu_try_cmpxchg_op().
>
> So, while at it, there's two other x86 percpu code generation details I w=
as
> wondering about:
>
> 1)
>
> Right now it's GCC-only:
>
>   config CC_HAS_NAMED_AS
>           def_bool CC_IS_GCC && GCC_VERSION >=3D 120100
>
> Because we wanted to create a stable core of known-working functionality.
>
> I suppose we have already established that with the current merge window,
> so it might be time to expand it.

Please note the KASAN incompatibility issue with GCC < 13.3. This
issue was fixed in the meantime, so I have posted a patch to re-enable
the named AS feature for gcc-13.3+ [1].

[1] https://lore.kernel.org/lkml/20240320124603.566923-1-ubizjak@gmail.com/

> Clang claims to be compatible:
>
>   https://releases.llvm.org/9.0.0/tools/clang/docs/LanguageExtensions.htm=
l
>
>   "You can also use the GCC compatibility macros __seg_fs and __seg_gs fo=
r the
>    same purpose. The preprocessor symbols __SEG_FS and __SEG_GS indicate =
their
>    support."
>
> I haven't tried it yet though.

In the RFC submission, the support was determined by the functional
check [2]. Perhaps we should re-introduce this instead of checking for
known compiler versions:

+config CC_HAS_NAMED_AS
+ def_bool $(success,echo 'int __seg_fs fs; int __seg_gs gs;' | $(CC)
-x c - -c -o /dev/null)

[2] https://lore.kernel.org/lkml/20231001131620.112484-3-ubizjak@gmail.com/

> 2)
>
> Also, is the GCC_VERSION cutoff accurate - are previous GCC versions
> known-buggy, or was it primarily a risk-reduction cutoff?

This approach was chosen from our discussion [3]. The version cutoff
is arbitrary, it was later set to gcc-12.1, because it is the version
of the compiler you used at the time ;) I have also tried gcc-11 and
gcc-10 in the past, and the compiler produced bootable image. Saying
that, the usage of named address spaces in the kernel is somehow basic
(from the compiler PoV), so I think we could try the above approach
with the functional check and see if and what breaks. We can always
disable the USE_X86_SEG_SUPPORT config variable for known bad compiler
versions.

[3] https://lore.kernel.org/lkml/ZRwZOtANkcwtL+5B@gmail.com/

BTW: Related to percpu series is the patch that fixes the issue with
%rip-relative addressing for PER_CPU_VAR in BPF. IMHO, this issue
should be fixed before rc1, otherwise call thunks will be unusable
with BPF.

[4] https://lore.kernel.org/lkml/20240316232104.368561-1-joanbrugueram@gmai=
l.com/

Thanks,
Uros.