lkml.org 
[lkml]   [2023]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()
Date

> On Oct 18, 2023, at 4:15 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>
>>
>> Looks like another case of underspecified functionality where both
>> compilers differ. Luckily, both DTRT when aliases are hidden in
>> another TU.
>
> Attached is the prototype patch that works for me (together with
> Linus' FPU switching patch).

In general looks good. See some minor issues below.

> --- a/arch/x86/include/asm/current.h
> +++ b/arch/x86/include/asm/current.h
> @@ -36,10 +36,23 @@ static_assert(sizeof(struct pcpu_hot) == 64);
>
> DECLARE_PER_CPU_ALIGNED(struct pcpu_hot, pcpu_hot);
>
> +/*
> + *
> + */

Obviously some further comments to clarify why struct pcpu_hot is
defined in percpu-hot.c (the GCC manual says: "It is an error if
the alias target is not defined in the same translation unit as the
alias” which can be used as part of the explanation.)

> +DECLARE_PER_CPU_ALIGNED(const struct pcpu_hot __percpu_seg_override,
> + const_pcpu_hot);
> +
> +#ifdef CONFIG_USE_X86_SEG_SUPPORT
> +static __always_inline struct task_struct *get_current(void)
> +{
> + return const_pcpu_hot.current_task;
> +}
> +#else
> static __always_inline struct task_struct *get_current(void)
> {
> return this_cpu_read_stable(pcpu_hot.current_task);
> }
> +#endif


Please consider using IS_ENABLED() to avoid the ifdef’ry.

So this would turn to be:

static __always_inline struct task_struct *get_current(void)
{
if (IS_ENABLED(CONFIG_USE_X86_SEG_SUPPORT))
return const_pcpu_hot.current_task;

return this_cpu_read_stable(pcpu_hot.current_task);
}


\
 
 \ /
  Last update: 2023-10-18 16:47    [W:0.142 / U:2.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site