Re: [tip: x86/percpu] x86/percpu: Convert this_percpu_xchg_op() from asm() to C code, to generate better code

[Uros Bizjak]

On Wed, Mar 20, 2024 at 12:45 PM Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
>
> * tip-bot2 for Uros Bizjak <tip-bot2@xxxxxxxxxxxxx> wrote:
>
> > The following commit has been merged into the x86/percpu branch of tip:
> >
> > Commit-ID:     0539084639f3835c8d0b798e6659ec14a266b4f1
> > Gitweb:        https://git.kernel.org/tip/0539084639f3835c8d0b798e6659ec14a266b4f1
> > Author:        Uros Bizjak <ubizjak@xxxxxxxxx>
> > AuthorDate:    Wed, 20 Mar 2024 09:30:40 +01:00
> > Committer:     Ingo Molnar <mingo@xxxxxxxxxx>
> > CommitterDate: Wed, 20 Mar 2024 12:29:02 +01:00
> >
> > x86/percpu: Convert this_percpu_xchg_op() from asm() to C code, to generate 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 was
> wondering about:
>
> 1)
>
> Right now it's GCC-only:
>
>   config CC_HAS_NAMED_AS
>           def_bool CC_IS_GCC && GCC_VERSION >= 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@xxxxxxxxx/

> Clang claims to be compatible:
>
>   https://releases.llvm.org/9.0.0/tools/clang/docs/LanguageExtensions.html
>
>   "You can also use the GCC compatibility macros __seg_fs and __seg_gs for 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@xxxxxxxxx/

> 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@xxxxxxxxx/

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@xxxxxxxxx/

Thanks,
Uros.