| Message ID | 20230802164701.192791-6-guoren@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | riscv: Add Native/Paravirt/CNA qspinlock support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes, riscv/for-next or riscv/master |
On 8/2/23 12:46, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > Combo spinlock could support queued and ticket in one Linux Image and > select them during boot time via errata mechanism. Here is the func > size (Bytes) comparison table below: > > TYPE : COMBO | TICKET | QUEUED > arch_spin_lock : 106 | 60 | 50 > arch_spin_unlock : 54 | 36 | 26 > arch_spin_trylock : 110 | 72 | 54 > arch_spin_is_locked : 48 | 34 | 20 > arch_spin_is_contended : 56 | 40 | 24 > rch_spin_value_unlocked : 48 | 34 | 24 > > One example of disassemble combo arch_spin_unlock: > 0xffffffff8000409c <+14>: nop # detour slot > 0xffffffff800040a0 <+18>: fence rw,w # queued spinlock start > 0xffffffff800040a4 <+22>: sb zero,0(a4) # queued spinlock end > 0xffffffff800040a8 <+26>: ld s0,8(sp) > 0xffffffff800040aa <+28>: addi sp,sp,16 > 0xffffffff800040ac <+30>: ret > 0xffffffff800040ae <+32>: lw a5,0(a4) # ticket spinlock start > 0xffffffff800040b0 <+34>: sext.w a5,a5 > 0xffffffff800040b2 <+36>: fence rw,w > 0xffffffff800040b6 <+40>: addiw a5,a5,1 > 0xffffffff800040b8 <+42>: slli a5,a5,0x30 > 0xffffffff800040ba <+44>: srli a5,a5,0x30 > 0xffffffff800040bc <+46>: sh a5,0(a4) # ticket spinlock end > 0xffffffff800040c0 <+50>: ld s0,8(sp) > 0xffffffff800040c2 <+52>: addi sp,sp,16 > 0xffffffff800040c4 <+54>: ret > > The qspinlock is smaller and faster than ticket-lock when all are in > fast-path, and combo spinlock could provide a compatible Linux Image > for different micro-arch design (weak/strict fwd guarantee) processors. > > Signed-off-by: Guo Ren <guoren@kernel.org> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > --- > arch/riscv/Kconfig | 9 +++- > arch/riscv/include/asm/hwcap.h | 1 + > arch/riscv/include/asm/spinlock.h | 87 ++++++++++++++++++++++++++++++- > arch/riscv/kernel/cpufeature.c | 10 ++++ > 4 files changed, 104 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index e89a3bea3dc1..119e774a3dcf 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -440,7 +440,7 @@ config NODES_SHIFT > > choice > prompt "RISC-V spinlock type" > - default RISCV_TICKET_SPINLOCKS > + default RISCV_COMBO_SPINLOCKS > > config RISCV_TICKET_SPINLOCKS > bool "Using ticket spinlock" > @@ -452,6 +452,13 @@ config RISCV_QUEUED_SPINLOCKS > help > Make sure your micro arch LL/SC has a strong forward progress guarantee. > Otherwise, stay at ticket-lock. > + > +config RISCV_COMBO_SPINLOCKS > + bool "Using combo spinlock" > + depends on SMP && MMU > + select ARCH_USE_QUEUED_SPINLOCKS > + help > + Select queued spinlock or ticket-lock via errata. > endchoice > > config RISCV_ALTERNATIVE > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > index f041bfa7f6a0..08ae75a694c2 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -54,6 +54,7 @@ > #define RISCV_ISA_EXT_ZIFENCEI 41 > #define RISCV_ISA_EXT_ZIHPM 42 > > +#define RISCV_ISA_EXT_XTICKETLOCK 63 > #define RISCV_ISA_EXT_MAX 64 > #define RISCV_ISA_EXT_NAME_LEN_MAX 32 > > diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h > index c644a92d4548..9eb3ad31e564 100644 > --- a/arch/riscv/include/asm/spinlock.h > +++ b/arch/riscv/include/asm/spinlock.h > @@ -7,11 +7,94 @@ > #define _Q_PENDING_LOOPS (1 << 9) > #endif > I see why you separated the _Q_PENDING_LOOPS out. > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS > +#include <asm-generic/ticket_spinlock.h> > + > +#undef arch_spin_is_locked > +#undef arch_spin_is_contended > +#undef arch_spin_value_unlocked > +#undef arch_spin_lock > +#undef arch_spin_trylock > +#undef arch_spin_unlock > + > +#include <asm-generic/qspinlock.h> > +#include <asm/hwcap.h> > + > +#undef arch_spin_is_locked > +#undef arch_spin_is_contended > +#undef arch_spin_value_unlocked > +#undef arch_spin_lock > +#undef arch_spin_trylock > +#undef arch_spin_unlock Perhaps you can add a macro like __no_arch_spinlock_redefine to disable the various arch_spin_* definition in qspinlock.h and ticket_spinlock.h. > + > +#define COMBO_DETOUR \ > + asm_volatile_goto(ALTERNATIVE( \ > + "nop", \ > + "j %l[ticket_spin_lock]", \ > + 0, \ > + RISCV_ISA_EXT_XTICKETLOCK, \ > + CONFIG_RISCV_COMBO_SPINLOCKS) \ > + : : : : ticket_spin_lock); > + > +static __always_inline void arch_spin_lock(arch_spinlock_t *lock) > +{ > + COMBO_DETOUR > + queued_spin_lock(lock); > + return; > +ticket_spin_lock: > + ticket_spin_lock(lock); > +} > + > +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock) > +{ > + COMBO_DETOUR > + return queued_spin_trylock(lock); > +ticket_spin_lock: > + return ticket_spin_trylock(lock); > +} > + > +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) > +{ > + COMBO_DETOUR > + queued_spin_unlock(lock); > + return; > +ticket_spin_lock: > + ticket_spin_unlock(lock); > +} > + > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) > +{ > + COMBO_DETOUR > + return queued_spin_value_unlocked(lock); > +ticket_spin_lock: > + return ticket_spin_value_unlocked(lock); > +} > + > +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) > +{ > + COMBO_DETOUR > + return queued_spin_is_locked(lock); > +ticket_spin_lock: > + return ticket_spin_is_locked(lock); > +} > + > +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) > +{ > + COMBO_DETOUR > + return queued_spin_is_contended(lock); > +ticket_spin_lock: > + return ticket_spin_is_contended(lock); > +} > +#else /* CONFIG_RISCV_COMBO_SPINLOCKS */ > + > #ifdef CONFIG_QUEUED_SPINLOCKS > #include <asm/qspinlock.h> > -#include <asm/qrwlock.h> > #else > -#include <asm-generic/spinlock.h> > +#include <asm-generic/ticket_spinlock.h> > #endif > > +#endif /* CONFIG_RISCV_COMBO_SPINLOCKS */ > + > +#include <asm/qrwlock.h> > + > #endif /* __ASM_RISCV_SPINLOCK_H */ > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index bdcf460ea53d..e65b0e54152d 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -324,6 +324,16 @@ void __init riscv_fill_hwcap(void) > set_bit(RISCV_ISA_EXT_ZICSR, isainfo->isa); > set_bit(RISCV_ISA_EXT_ZIFENCEI, isainfo->isa); > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS > + /* > + * The RISC-V Linux used queued spinlock at first; then, we used ticket_lock > + * as default or queued spinlock by choice. Because ticket_lock would dirty > + * spinlock value, the only way is to change from queued_spinlock to > + * ticket_spinlock, but can not be vice. The phrase "but can not be vice" is confusing. I think you mean "but not vice versa". Right? Cheers, Longman
On Sat, Aug 12, 2023 at 3:51 AM Waiman Long <longman@redhat.com> wrote: > > On 8/2/23 12:46, guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > Combo spinlock could support queued and ticket in one Linux Image and > > select them during boot time via errata mechanism. Here is the func > > size (Bytes) comparison table below: > > > > TYPE : COMBO | TICKET | QUEUED > > arch_spin_lock : 106 | 60 | 50 > > arch_spin_unlock : 54 | 36 | 26 > > arch_spin_trylock : 110 | 72 | 54 > > arch_spin_is_locked : 48 | 34 | 20 > > arch_spin_is_contended : 56 | 40 | 24 > > rch_spin_value_unlocked : 48 | 34 | 24 > > > > One example of disassemble combo arch_spin_unlock: > > 0xffffffff8000409c <+14>: nop # detour slot > > 0xffffffff800040a0 <+18>: fence rw,w # queued spinlock start > > 0xffffffff800040a4 <+22>: sb zero,0(a4) # queued spinlock end > > 0xffffffff800040a8 <+26>: ld s0,8(sp) > > 0xffffffff800040aa <+28>: addi sp,sp,16 > > 0xffffffff800040ac <+30>: ret > > 0xffffffff800040ae <+32>: lw a5,0(a4) # ticket spinlock start > > 0xffffffff800040b0 <+34>: sext.w a5,a5 > > 0xffffffff800040b2 <+36>: fence rw,w > > 0xffffffff800040b6 <+40>: addiw a5,a5,1 > > 0xffffffff800040b8 <+42>: slli a5,a5,0x30 > > 0xffffffff800040ba <+44>: srli a5,a5,0x30 > > 0xffffffff800040bc <+46>: sh a5,0(a4) # ticket spinlock end > > 0xffffffff800040c0 <+50>: ld s0,8(sp) > > 0xffffffff800040c2 <+52>: addi sp,sp,16 > > 0xffffffff800040c4 <+54>: ret > > > > The qspinlock is smaller and faster than ticket-lock when all are in > > fast-path, and combo spinlock could provide a compatible Linux Image > > for different micro-arch design (weak/strict fwd guarantee) processors. > > > > Signed-off-by: Guo Ren <guoren@kernel.org> > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > --- > > arch/riscv/Kconfig | 9 +++- > > arch/riscv/include/asm/hwcap.h | 1 + > > arch/riscv/include/asm/spinlock.h | 87 ++++++++++++++++++++++++++++++- > > arch/riscv/kernel/cpufeature.c | 10 ++++ > > 4 files changed, 104 insertions(+), 3 deletions(-) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index e89a3bea3dc1..119e774a3dcf 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -440,7 +440,7 @@ config NODES_SHIFT > > > > choice > > prompt "RISC-V spinlock type" > > - default RISCV_TICKET_SPINLOCKS > > + default RISCV_COMBO_SPINLOCKS > > > > config RISCV_TICKET_SPINLOCKS > > bool "Using ticket spinlock" > > @@ -452,6 +452,13 @@ config RISCV_QUEUED_SPINLOCKS > > help > > Make sure your micro arch LL/SC has a strong forward progress guarantee. > > Otherwise, stay at ticket-lock. > > + > > +config RISCV_COMBO_SPINLOCKS > > + bool "Using combo spinlock" > > + depends on SMP && MMU > > + select ARCH_USE_QUEUED_SPINLOCKS > > + help > > + Select queued spinlock or ticket-lock via errata. > > endchoice > > > > config RISCV_ALTERNATIVE > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > > index f041bfa7f6a0..08ae75a694c2 100644 > > --- a/arch/riscv/include/asm/hwcap.h > > +++ b/arch/riscv/include/asm/hwcap.h > > @@ -54,6 +54,7 @@ > > #define RISCV_ISA_EXT_ZIFENCEI 41 > > #define RISCV_ISA_EXT_ZIHPM 42 > > > > +#define RISCV_ISA_EXT_XTICKETLOCK 63 > > #define RISCV_ISA_EXT_MAX 64 > > #define RISCV_ISA_EXT_NAME_LEN_MAX 32 > > > > diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h > > index c644a92d4548..9eb3ad31e564 100644 > > --- a/arch/riscv/include/asm/spinlock.h > > +++ b/arch/riscv/include/asm/spinlock.h > > @@ -7,11 +7,94 @@ > > #define _Q_PENDING_LOOPS (1 << 9) > > #endif > > > > I see why you separated the _Q_PENDING_LOOPS out. haha, yes, I even forget this, :). > > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS > > +#include <asm-generic/ticket_spinlock.h> > > + > > +#undef arch_spin_is_locked > > +#undef arch_spin_is_contended > > +#undef arch_spin_value_unlocked > > +#undef arch_spin_lock > > +#undef arch_spin_trylock > > +#undef arch_spin_unlock > > + > > +#include <asm-generic/qspinlock.h> > > +#include <asm/hwcap.h> > > + > > +#undef arch_spin_is_locked > > +#undef arch_spin_is_contended > > +#undef arch_spin_value_unlocked > > +#undef arch_spin_lock > > +#undef arch_spin_trylock > > +#undef arch_spin_unlock > Perhaps you can add a macro like __no_arch_spinlock_redefine to disable > the various arch_spin_* definition in qspinlock.h and ticket_spinlock.h. That's great; I will try it in the next version. > > + > > +#define COMBO_DETOUR \ > > + asm_volatile_goto(ALTERNATIVE( \ > > + "nop", \ > > + "j %l[ticket_spin_lock]", \ > > + 0, \ > > + RISCV_ISA_EXT_XTICKETLOCK, \ > > + CONFIG_RISCV_COMBO_SPINLOCKS) \ > > + : : : : ticket_spin_lock); > > + > > +static __always_inline void arch_spin_lock(arch_spinlock_t *lock) > > +{ > > + COMBO_DETOUR > > + queued_spin_lock(lock); > > + return; > > +ticket_spin_lock: > > + ticket_spin_lock(lock); > > +} > > + > > +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock) > > +{ > > + COMBO_DETOUR > > + return queued_spin_trylock(lock); > > +ticket_spin_lock: > > + return ticket_spin_trylock(lock); > > +} > > + > > +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) > > +{ > > + COMBO_DETOUR > > + queued_spin_unlock(lock); > > + return; > > +ticket_spin_lock: > > + ticket_spin_unlock(lock); > > +} > > + > > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) > > +{ > > + COMBO_DETOUR > > + return queued_spin_value_unlocked(lock); > > +ticket_spin_lock: > > + return ticket_spin_value_unlocked(lock); > > +} > > + > > +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) > > +{ > > + COMBO_DETOUR > > + return queued_spin_is_locked(lock); > > +ticket_spin_lock: > > + return ticket_spin_is_locked(lock); > > +} > > + > > +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) > > +{ > > + COMBO_DETOUR > > + return queued_spin_is_contended(lock); > > +ticket_spin_lock: > > + return ticket_spin_is_contended(lock); > > +} > > +#else /* CONFIG_RISCV_COMBO_SPINLOCKS */ > > + > > #ifdef CONFIG_QUEUED_SPINLOCKS > > #include <asm/qspinlock.h> > > -#include <asm/qrwlock.h> > > #else > > -#include <asm-generic/spinlock.h> > > +#include <asm-generic/ticket_spinlock.h> > > #endif > > > > +#endif /* CONFIG_RISCV_COMBO_SPINLOCKS */ > > + > > +#include <asm/qrwlock.h> > > + > > #endif /* __ASM_RISCV_SPINLOCK_H */ > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > index bdcf460ea53d..e65b0e54152d 100644 > > --- a/arch/riscv/kernel/cpufeature.c > > +++ b/arch/riscv/kernel/cpufeature.c > > @@ -324,6 +324,16 @@ void __init riscv_fill_hwcap(void) > > set_bit(RISCV_ISA_EXT_ZICSR, isainfo->isa); > > set_bit(RISCV_ISA_EXT_ZIFENCEI, isainfo->isa); > > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS > > + /* > > + * The RISC-V Linux used queued spinlock at first; then, we used ticket_lock > > + * as default or queued spinlock by choice. Because ticket_lock would dirty > > + * spinlock value, the only way is to change from queued_spinlock to > > + * ticket_spinlock, but can not be vice. > > The phrase "but can not be vice" is confusing. I think you mean "but not > vice versa". Right? Yes, thx for the grammar correction. > > Cheers, > Longman >
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index e89a3bea3dc1..119e774a3dcf 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -440,7 +440,7 @@ config NODES_SHIFT choice prompt "RISC-V spinlock type" - default RISCV_TICKET_SPINLOCKS + default RISCV_COMBO_SPINLOCKS config RISCV_TICKET_SPINLOCKS bool "Using ticket spinlock" @@ -452,6 +452,13 @@ config RISCV_QUEUED_SPINLOCKS help Make sure your micro arch LL/SC has a strong forward progress guarantee. Otherwise, stay at ticket-lock. + +config RISCV_COMBO_SPINLOCKS + bool "Using combo spinlock" + depends on SMP && MMU + select ARCH_USE_QUEUED_SPINLOCKS + help + Select queued spinlock or ticket-lock via errata. endchoice config RISCV_ALTERNATIVE diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index f041bfa7f6a0..08ae75a694c2 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -54,6 +54,7 @@ #define RISCV_ISA_EXT_ZIFENCEI 41 #define RISCV_ISA_EXT_ZIHPM 42 +#define RISCV_ISA_EXT_XTICKETLOCK 63 #define RISCV_ISA_EXT_MAX 64 #define RISCV_ISA_EXT_NAME_LEN_MAX 32 diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h index c644a92d4548..9eb3ad31e564 100644 --- a/arch/riscv/include/asm/spinlock.h +++ b/arch/riscv/include/asm/spinlock.h @@ -7,11 +7,94 @@ #define _Q_PENDING_LOOPS (1 << 9) #endif +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS +#include <asm-generic/ticket_spinlock.h> + +#undef arch_spin_is_locked +#undef arch_spin_is_contended +#undef arch_spin_value_unlocked +#undef arch_spin_lock +#undef arch_spin_trylock +#undef arch_spin_unlock + +#include <asm-generic/qspinlock.h> +#include <asm/hwcap.h> + +#undef arch_spin_is_locked +#undef arch_spin_is_contended +#undef arch_spin_value_unlocked +#undef arch_spin_lock +#undef arch_spin_trylock +#undef arch_spin_unlock + +#define COMBO_DETOUR \ + asm_volatile_goto(ALTERNATIVE( \ + "nop", \ + "j %l[ticket_spin_lock]", \ + 0, \ + RISCV_ISA_EXT_XTICKETLOCK, \ + CONFIG_RISCV_COMBO_SPINLOCKS) \ + : : : : ticket_spin_lock); + +static __always_inline void arch_spin_lock(arch_spinlock_t *lock) +{ + COMBO_DETOUR + queued_spin_lock(lock); + return; +ticket_spin_lock: + ticket_spin_lock(lock); +} + +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock) +{ + COMBO_DETOUR + return queued_spin_trylock(lock); +ticket_spin_lock: + return ticket_spin_trylock(lock); +} + +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) +{ + COMBO_DETOUR + queued_spin_unlock(lock); + return; +ticket_spin_lock: + ticket_spin_unlock(lock); +} + +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock) +{ + COMBO_DETOUR + return queued_spin_value_unlocked(lock); +ticket_spin_lock: + return ticket_spin_value_unlocked(lock); +} + +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock) +{ + COMBO_DETOUR + return queued_spin_is_locked(lock); +ticket_spin_lock: + return ticket_spin_is_locked(lock); +} + +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) +{ + COMBO_DETOUR + return queued_spin_is_contended(lock); +ticket_spin_lock: + return ticket_spin_is_contended(lock); +} +#else /* CONFIG_RISCV_COMBO_SPINLOCKS */ + #ifdef CONFIG_QUEUED_SPINLOCKS #include <asm/qspinlock.h> -#include <asm/qrwlock.h> #else -#include <asm-generic/spinlock.h> +#include <asm-generic/ticket_spinlock.h> #endif +#endif /* CONFIG_RISCV_COMBO_SPINLOCKS */ + +#include <asm/qrwlock.h> + #endif /* __ASM_RISCV_SPINLOCK_H */ diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index bdcf460ea53d..e65b0e54152d 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -324,6 +324,16 @@ void __init riscv_fill_hwcap(void) set_bit(RISCV_ISA_EXT_ZICSR, isainfo->isa); set_bit(RISCV_ISA_EXT_ZIFENCEI, isainfo->isa); +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS + /* + * The RISC-V Linux used queued spinlock at first; then, we used ticket_lock + * as default or queued spinlock by choice. Because ticket_lock would dirty + * spinlock value, the only way is to change from queued_spinlock to + * ticket_spinlock, but can not be vice. + */ + set_bit(RISCV_ISA_EXT_XTICKETLOCK, isainfo->isa); +#endif + /* * These ones were as they were part of the base ISA when the * port & dt-bindings were upstreamed, and so can be set