Re: [PATCH 06/12] powerpc: Mark accesses to power_save callback in arch_cpu_idle

["Nicholas Piggin" <npiggin@gmail.com>]

On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> The power_save callback can be overwritten by another core at boot time.
> Specifically, null values will be replaced exactly once with the callback
> suitable for the particular platform (PowerNV / pseries lpars). Mark
> reads to this variable with READ_ONCE to signal to KCSAN that this race
> is acceptable, as well as to rule-out the possibility for compiler reorderings
> leading to calling a null pointer.

Is ppc_md readonly after init? Might be a good candidate if it is...
Maybe KCSAN doesn't recognise that though.

Unless the places that assign ppc_md.power_save need to be converted
to use WRITE_ONCE, you could just annotate this with data_race and
comment it's not really a race because it won't be called before the
structure is set up.

Thanks,
Nick
>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
>  arch/powerpc/kernel/idle.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
> index b1c0418b25c8..a1589bb97c98 100644
> --- a/arch/powerpc/kernel/idle.c
> +++ b/arch/powerpc/kernel/idle.c
> @@ -43,10 +43,12 @@ __setup("powersave=off", powersave_off);
>  
>  void arch_cpu_idle(void)
>  {
> +	void (*power_save)(void) = READ_ONCE(ppc_md.power_save);
> +
>  	ppc64_runlatch_off();
>  
> -	if (ppc_md.power_save) {
> -		ppc_md.power_save();
> +	if (power_save) {
> +		power_save();
>  		/*
>  		 * Some power_save functions return with
>  		 * interrupts enabled, some don't.
> -- 
> 2.37.2