| Message ID | 20230508020120.218494-7-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: KCSAN fix warnings and mark accesses | expand |
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
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.
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. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- arch/powerpc/kernel/idle.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)