Re: [PATCH 1/3] powerpc/spinlocks: Refactor SHARED_PROCESSOR

Christopher M Riedl [off-list ref] writes:

quoted

On July 30, 2019 at 4:31 PM Thiago Jung Bauermann [off-list ref] wrote:



Christopher M. Riedl [off-list ref] writes:

quoted

Determining if a processor is in shared processor mode is not a constant
so don't hide it behind a #define.

Signed-off-by: Christopher M. Riedl <redacted>
---
 arch/powerpc/include/asm/spinlock.h | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index a47f827bc5f1..8631b0b4e109 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h

@@ -101,15 +101,24 @@ static inline int arch_spin_trylock(arch_spinlock_t *lock)

 #if defined(CONFIG_PPC_SPLPAR)
 /* We only yield to the hypervisor if we are in shared processor mode */
-#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca->lppaca_ptr))
 extern void __spin_yield(arch_spinlock_t *lock);
 extern void __rw_yield(arch_rwlock_t *lock);
 #else /* SPLPAR */
 #define __spin_yield(x)	barrier()
 #define __rw_yield(x)	barrier()
-#define SHARED_PROCESSOR	0
 #endif

+static inline bool is_shared_processor(void)
+{
+/* Only server processors have an lppaca struct */
+#ifdef CONFIG_PPC_BOOK3S
+	return (IS_ENABLED(CONFIG_PPC_SPLPAR) &&
+		lppaca_shared_proc(local_paca->lppaca_ptr));
+#else
+	return false;
+#endif
+}
+

CONFIG_PPC_SPLPAR depends on CONFIG_PPC_PSERIES, which depends on
CONFIG_PPC_BOOK3S so the #ifdef above is unnecessary:

if CONFIG_PPC_BOOK3S is unset then CONFIG_PPC_SPLPAR will be unset as
well and the return expression should short-circuit to false.

Agreed, but the #ifdef is necessary to compile platforms which include
this header but do not implement lppaca_shared_proc(...) and friends.
I can reword the comment if that helps.

Ah, indeed. Yes, if you could mention that in the commit I think it
would help. These #ifdefs are becoming démodé so it's good to know why
they're there.

Another alternative is to provide a dummy lppaca_shared_proc() which
always returns false when CONFIG_PPC_BOOK3S isn't set (just mentioning
it, I don't have a preference).

--
Thiago Jung Bauermann
IBM Linux Technology Center