[PATCH] powerpc/pseries: Rework lppaca_shared_proc() to avoid DEBUG_PREEMPT
From: Russell Currey <hidden>
Date: 2023-08-08 00:54:42
Subsystem:
cpu idle time management framework, linux for powerpc (32-bit and 64-bit), the rest · Maintainers:
"Rafael J. Wysocki", Daniel Lezcano, Madhavan Srinivasan, Linus Torvalds
lppaca_shared_proc() takes a pointer to the lppaca which is typically
accessed through get_lppaca(). With DEBUG_PREEMPT enabled, this leads
to checking if preemption is enabled, for example:
BUG: using smp_processor_id() in preemptible [00000000] code: grep/10693
caller is lparcfg_data+0x408/0x19a0
CPU: 4 PID: 10693 Comm: grep Not tainted 6.5.0-rc3 #2
Call Trace:
dump_stack_lvl+0x154/0x200 (unreliable)
check_preemption_disabled+0x214/0x220
lparcfg_data+0x408/0x19a0
...
This isn't actually a problem however, as it does not matter which
lppaca is accessed, the shared proc state will be the same.
vcpudispatch_stats_procfs_init() already works around this by disabling
preemption, but the lparcfg code does not, erroring any time
/proc/powerpc/lparcfg is accessed with DEBUG_PREEMPT enabled.
Instead of disabling preemption on the caller side, rework
lppaca_shared_proc() to not take a pointer and instead directly access
the lppaca, bypassing any potential preemption checks.
Fixes: f13c13a00512 ("powerpc: Stop using non-architected shared_proc field in lppaca")
Signed-off-by: Russell Currey <redacted>
---
Fixes tag might be a bit overkill.
---
arch/powerpc/include/asm/lppaca.h | 9 ++++++++-
arch/powerpc/include/asm/paca.h | 5 +++++
arch/powerpc/platforms/pseries/lpar.c | 10 +---------
arch/powerpc/platforms/pseries/lparcfg.c | 4 ++--
arch/powerpc/platforms/pseries/setup.c | 2 +-
drivers/cpuidle/cpuidle-pseries.c | 8 +-------
6 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/lppaca.h b/arch/powerpc/include/asm/lppaca.h
index 34d44cb17c87..c12e1a6e3595 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h@@ -127,7 +127,14 @@ struct lppaca { */ #define LPPACA_OLD_SHARED_PROC 2 -static inline bool lppaca_shared_proc(struct lppaca *l) +/* + * All CPUs should have the same shared proc value, so directly access the PACA + * to avoid false positives from DEBUG_PREEMPT. + * + * local_paca can't be referenced directly from lppaca.h, hence the macro. + */ +#define lppaca_shared_proc() (__lppaca_shared_proc(local_paca->lppaca_ptr)) +static inline bool __lppaca_shared_proc(struct lppaca *l) { if (!firmware_has_feature(FW_FEATURE_SPLPAR)) return false;
diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index cb325938766a..f77337b92ccf 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h@@ -49,6 +49,11 @@ extern unsigned int debug_smp_processor_id(void); /* from linux/smp.h */ #ifdef CONFIG_PPC_PSERIES #define get_lppaca() (get_paca()->lppaca_ptr) +/* + * All CPUs should have the same shared proc value, so directly access the PACA + * to avoid false positives from DEBUG_PREEMPT. + */ +#define lppaca_shared_proc() (__lppaca_shared_proc(local_paca->lppaca_ptr)) #endif #define get_slb_shadow() (get_paca()->slb_shadow_ptr)
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 2eab323f6970..cb2f1211f7eb 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c@@ -639,16 +639,8 @@ static const struct proc_ops vcpudispatch_stats_freq_proc_ops = { static int __init vcpudispatch_stats_procfs_init(void) { - /* - * Avoid smp_processor_id while preemptible. All CPUs should have - * the same value for lppaca_shared_proc. - */ - preempt_disable(); - if (!lppaca_shared_proc(get_lppaca())) { - preempt_enable(); + if (!lppaca_shared_proc()) return 0; - } - preempt_enable(); if (!proc_create("powerpc/vcpudispatch_stats", 0600, NULL, &vcpudispatch_stats_proc_ops))
diff --git a/arch/powerpc/platforms/pseries/lparcfg.c b/arch/powerpc/platforms/pseries/lparcfg.c
index 8acc70509520..1c151d77e74b 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c@@ -206,7 +206,7 @@ static void parse_ppp_data(struct seq_file *m) ppp_data.active_system_procs); /* pool related entries are appropriate for shared configs */ - if (lppaca_shared_proc(get_lppaca())) { + if (lppaca_shared_proc()) { unsigned long pool_idle_time, pool_procs; seq_printf(m, "pool=%d\n", ppp_data.pool_num);
@@ -560,7 +560,7 @@ static int pseries_lparcfg_data(struct seq_file *m, void *v) partition_potential_processors); seq_printf(m, "shared_processor_mode=%d\n", - lppaca_shared_proc(get_lppaca())); + lppaca_shared_proc()); #ifdef CONFIG_PPC_64S_HASH_MMU if (!radix_enabled())
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index e2a57cfa6c83..0ef2a7e014aa 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c@@ -847,7 +847,7 @@ static void __init pSeries_setup_arch(void) if (firmware_has_feature(FW_FEATURE_LPAR)) { vpa_init(boot_cpuid); - if (lppaca_shared_proc(get_lppaca())) { + if (lppaca_shared_proc()) { static_branch_enable(&shared_processor); pv_spinlocks_init(); #ifdef CONFIG_PARAVIRT_TIME_ACCOUNTING
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index a7d33f3ee01e..14db9b7d985d 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c@@ -414,13 +414,7 @@ static int __init pseries_idle_probe(void) return -ENODEV; if (firmware_has_feature(FW_FEATURE_SPLPAR)) { - /* - * Use local_paca instead of get_lppaca() since - * preemption is not disabled, and it is not required in - * fact, since lppaca_ptr does not need to be the value - * associated to the current CPU, it can be from any CPU. - */ - if (lppaca_shared_proc(local_paca->lppaca_ptr)) { + if (lppaca_shared_proc()) { cpuidle_state_table = shared_states; max_idle_state = ARRAY_SIZE(shared_states); } else {
--
2.41.0