Re: [PATCH 06/12] powerpc: Mark accesses to power_save callback in arch_cpu_idle
From: "Nicholas Piggin" <npiggin@gmail.com>
Date: 2023-05-09 02:22:55
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
quoted hunk ↗ jump to hunk
Signed-off-by: Rohan McLure <redacted> --- 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