Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()

[Nadav Amit]

> On Oct 18, 2023, at 4:15 PM, Uros Bizjak <ubizjak@xxxxxxxxx> 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);
}