From: Michael Ellerman <mpe@ellerman.id.au>
To: <linuxppc-dev@lists.ozlabs.org>
Cc: ruscur@russell.cc
Subject: [PATCH v2 4/4] powerpc/pseries: Rework lppaca_shared_proc() to avoid DEBUG_PREEMPT
Date: Wed, 23 Aug 2023 15:53:17 +1000 [thread overview]
Message-ID: <20230823055317.751786-4-mpe@ellerman.id.au> (raw)
In-Reply-To: <20230823055317.751786-1-mpe@ellerman.id.au>
From: Russell Currey <ruscur@russell.cc>
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 <ruscur@russell.cc>
[mpe: Rework to avoid needing a definition in paca.h and lppaca.h]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://msgid.link/20230808005317.20374-1-ruscur@russell.cc
---
arch/powerpc/include/asm/lppaca.h | 11 +++++++++--
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 +-------
5 files changed, 14 insertions(+), 21 deletions(-)
v2: mpe, rework based on the preceeding header untangling patches.
diff --git a/arch/powerpc/include/asm/lppaca.h b/arch/powerpc/include/asm/lppaca.h
index b6a63fa0965f..61ec2447dabf 100644
--- a/arch/powerpc/include/asm/lppaca.h
+++ b/arch/powerpc/include/asm/lppaca.h
@@ -23,6 +23,7 @@
#include <asm/types.h>
#include <asm/mmu.h>
#include <asm/firmware.h>
+#include <asm/paca.h>
/*
* The lppaca is the "virtual processor area" registered with the hypervisor,
@@ -105,14 +106,20 @@ struct lppaca {
*/
#define LPPACA_OLD_SHARED_PROC 2
-static inline bool lppaca_shared_proc(struct lppaca *l)
+#ifdef CONFIG_PPC_PSERIES
+/*
+ * All CPUs should have the same shared proc value, so directly access the PACA
+ * to avoid false positives from DEBUG_PREEMPT.
+ */
+static inline bool lppaca_shared_proc(void)
{
+ struct lppaca *l = local_paca->lppaca_ptr;
+
if (!firmware_has_feature(FW_FEATURE_SPLPAR))
return false;
return !!(l->__old_status & LPPACA_OLD_SHARED_PROC);
}
-#ifdef CONFIG_PPC_PSERIES
#define get_lppaca() (get_paca()->lppaca_ptr)
#endif
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 27fb656bd6ba..f2cb62148f36 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -640,16 +640,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 bb0a9aeb50f9..ecea85c74c43 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -849,7 +849,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
next prev parent reply other threads:[~2023-08-23 5:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-23 5:53 [PATCH 1/4] powerpc/pseries: Move VPHN constants into vphn.h Michael Ellerman
2023-08-23 5:53 ` [PATCH 2/4] powerpc/pseries: Move hcall_vphn() prototype " Michael Ellerman
2023-08-23 5:53 ` [PATCH 3/4] powerpc: Don't include lppaca.h in paca.h Michael Ellerman
2023-08-23 5:53 ` Michael Ellerman [this message]
2023-08-31 4:02 ` (subset) [PATCH v2 4/4] powerpc/pseries: Rework lppaca_shared_proc() to avoid DEBUG_PREEMPT Michael Ellerman
2023-08-31 4:02 ` [PATCH 1/4] powerpc/pseries: Move VPHN constants into vphn.h Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230823055317.751786-4-mpe@ellerman.id.au \
--to=mpe@ellerman.id.au \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=ruscur@russell.cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.