Re: [PATCH v2 5/6] powerpc/pseries: implement paravirt qspinlocks for SPLPAR

From: Waiman Long
Date: Sun Jul 05 2020 - 15:09:06 EST


On 7/3/20 3:35 AM, Nicholas Piggin wrote:
Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>
---
arch/powerpc/include/asm/paravirt.h | 28 ++++++++++
arch/powerpc/include/asm/qspinlock.h | 55 +++++++++++++++++++
arch/powerpc/include/asm/qspinlock_paravirt.h | 5 ++
arch/powerpc/platforms/pseries/Kconfig | 5 ++
arch/powerpc/platforms/pseries/setup.c | 6 +-
include/asm-generic/qspinlock.h | 2 +
6 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/include/asm/qspinlock_paravirt.h

diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
index 7a8546660a63..f2d51f929cf5 100644
--- a/arch/powerpc/include/asm/paravirt.h
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -29,6 +29,16 @@ static inline void yield_to_preempted(int cpu, u32 yield_count)
{
plpar_hcall_norets(H_CONFER, get_hard_smp_processor_id(cpu), yield_count);
}
+
+static inline void prod_cpu(int cpu)
+{
+ plpar_hcall_norets(H_PROD, get_hard_smp_processor_id(cpu));
+}
+
+static inline void yield_to_any(void)
+{
+ plpar_hcall_norets(H_CONFER, -1, 0);
+}
#else
static inline bool is_shared_processor(void)
{
@@ -45,6 +55,19 @@ static inline void yield_to_preempted(int cpu, u32 yield_count)
{
___bad_yield_to_preempted(); /* This would be a bug */
}
+
+extern void ___bad_yield_to_any(void);
+static inline void yield_to_any(void)
+{
+ ___bad_yield_to_any(); /* This would be a bug */
+}
+
+extern void ___bad_prod_cpu(void);
+static inline void prod_cpu(int cpu)
+{
+ ___bad_prod_cpu(); /* This would be a bug */
+}
+
#endif
#define vcpu_is_preempted vcpu_is_preempted
@@ -57,5 +80,10 @@ static inline bool vcpu_is_preempted(int cpu)
return false;
}
+static inline bool pv_is_native_spin_unlock(void)
+{
+ return !is_shared_processor();
+}
+
#endif /* __KERNEL__ */
#endif /* __ASM_PARAVIRT_H */
diff --git a/arch/powerpc/include/asm/qspinlock.h b/arch/powerpc/include/asm/qspinlock.h
index c49e33e24edd..0960a0de2467 100644
--- a/arch/powerpc/include/asm/qspinlock.h
+++ b/arch/powerpc/include/asm/qspinlock.h
@@ -3,9 +3,36 @@
#define _ASM_POWERPC_QSPINLOCK_H
#include <asm-generic/qspinlock_types.h>
+#include <asm/paravirt.h>
#define _Q_PENDING_LOOPS (1 << 9) /* not tuned */
+#ifdef CONFIG_PARAVIRT_SPINLOCKS
+extern void native_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+extern void __pv_queued_spin_lock_slowpath(struct qspinlock *lock, u32 val);
+
+static __always_inline void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
+{
+ if (!is_shared_processor())
+ native_queued_spin_lock_slowpath(lock, val);
+ else
+ __pv_queued_spin_lock_slowpath(lock, val);
+}

In a previous mail, I said that:

You may need to match the use of __pv_queued_spin_lock_slowpath() with the corresponding __pv_queued_spin_unlock(), e.g.

#define queued_spin_unlock queued_spin_unlock
static inline queued_spin_unlock(struct qspinlock *lock)
{
ÂÂÂÂÂÂÂ if (!is_shared_processor())
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ smp_store_release(&lock->locked, 0);
ÂÂÂÂÂÂÂ else
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ __pv_queued_spin_unlock(lock);
}

Otherwise, pv_kick() will never be called.

Maybe PowerPC HMT is different that the shared cpus can still process instruction, though slower, that cpu kicking like what was done in kvm is not really necessary. If that is the case, I think we should document that.

Cheers,
Longman