diff mbox series

[V11,06/17] riscv: qspinlock: Introduce combo spinlock

Message ID 20230910082911.3378782-7-guoren@kernel.org (mailing list archive)
State New, archived
Headers show
Series riscv: Add Native/Paravirt qspinlock support | expand

Commit Message

Guo Ren Sept. 10, 2023, 8:29 a.m. UTC
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 LR/SC)
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/spinlock.h | 78 ++++++++++++++++++++++++++++++-
 arch/riscv/kernel/setup.c         | 14 ++++++
 3 files changed, 98 insertions(+), 3 deletions(-)

Comments

Guo Ren Sept. 10, 2023, 11:06 a.m. UTC | #1
On Sun, Sep 10, 2023 at 04:29:00AM -0400, 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 LR/SC)
> 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/spinlock.h | 78 ++++++++++++++++++++++++++++++-
>  arch/riscv/kernel/setup.c         | 14 ++++++
>  3 files changed, 98 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 7f39bfc75744..4bcff2860f48 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -473,7 +473,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"
> @@ -485,6 +485,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/spinlock.h b/arch/riscv/include/asm/spinlock.h
> index c644a92d4548..8ea0fee80652 100644
> --- a/arch/riscv/include/asm/spinlock.h
> +++ b/arch/riscv/include/asm/spinlock.h
> @@ -7,11 +7,85 @@
>  #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 <linux/jump_label.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
Sorry, I forgot __no_arch_spinlock_redefine advice here. I would add it in v12.
https://lore.kernel.org/linux-riscv/4cc7113a-0e4e-763a-cba2-7963bcd26c7a@redhat.com/

> +
> +DECLARE_STATIC_KEY_TRUE(combo_qspinlock_key);
> +
> +static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
> +{
> +	if (static_branch_likely(&combo_qspinlock_key))
> +		queued_spin_lock(lock);
> +	else
> +		ticket_spin_lock(lock);
> +}
> +
> +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
> +{
> +	if (static_branch_likely(&combo_qspinlock_key))
> +		return queued_spin_trylock(lock);
> +	else
> +		return ticket_spin_trylock(lock);
> +}
> +
> +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
> +{
> +	if (static_branch_likely(&combo_qspinlock_key))
> +		queued_spin_unlock(lock);
> +	else
> +		ticket_spin_unlock(lock);
> +}
> +
> +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
> +{
> +	if (static_branch_likely(&combo_qspinlock_key))
> +		return queued_spin_value_unlocked(lock);
> +	else
> +		return ticket_spin_value_unlocked(lock);
> +}
> +
> +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
> +{
> +	if (static_branch_likely(&combo_qspinlock_key))
> +		return queued_spin_is_locked(lock);
> +	else
> +		return ticket_spin_is_locked(lock);
> +}
> +
> +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
> +{
> +	if (static_branch_likely(&combo_qspinlock_key))
> +		return queued_spin_is_contended(lock);
> +	else
> +		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/setup.c b/arch/riscv/kernel/setup.c
> index 32c2e1eb71bd..a447cf360a18 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -269,6 +269,18 @@ static void __init parse_dtb(void)
>  #endif
>  }
>  
> +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> +DEFINE_STATIC_KEY_TRUE(combo_qspinlock_key);
> +EXPORT_SYMBOL(combo_qspinlock_key);
> +#endif
> +
> +static void __init riscv_spinlock_init(void)
> +{
> +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> +	static_branch_disable(&combo_qspinlock_key);
> +#endif
> +}
> +
>  extern void __init init_rt_signal_env(void);
>  
>  void __init setup_arch(char **cmdline_p)
> @@ -317,6 +329,8 @@ void __init setup_arch(char **cmdline_p)
>  	    riscv_isa_extension_available(NULL, ZICBOM))
>  		riscv_noncoherent_supported();
>  	riscv_set_dma_cache_alignment();
> +
> +	riscv_spinlock_init();
>  }
>  
>  static int __init topology_init(void)
> -- 
> 2.36.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
Leonardo Bras Sept. 13, 2023, 8:37 p.m. UTC | #2
On Sun, Sep 10, 2023 at 07:06:23AM -0400, Guo Ren wrote:
> On Sun, Sep 10, 2023 at 04:29:00AM -0400, 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 LR/SC)
> > 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/spinlock.h | 78 ++++++++++++++++++++++++++++++-
> >  arch/riscv/kernel/setup.c         | 14 ++++++
> >  3 files changed, 98 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 7f39bfc75744..4bcff2860f48 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -473,7 +473,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"
> > @@ -485,6 +485,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/spinlock.h b/arch/riscv/include/asm/spinlock.h
> > index c644a92d4548..8ea0fee80652 100644
> > --- a/arch/riscv/include/asm/spinlock.h
> > +++ b/arch/riscv/include/asm/spinlock.h
> > @@ -7,11 +7,85 @@
> >  #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 <linux/jump_label.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
> Sorry, I forgot __no_arch_spinlock_redefine advice here. I would add it in v12.
> https://lore.kernel.org/linux-riscv/4cc7113a-0e4e-763a-cba2-7963bcd26c7a@redhat.com/
> 

Please check a reply to a previous patch I sent earlier: I think these 
#undef can be avoided.
 
> > +
> > +DECLARE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > +
> > +static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
> > +{
> > +	if (static_branch_likely(&combo_qspinlock_key))
> > +		queued_spin_lock(lock);
> > +	else
> > +		ticket_spin_lock(lock);
> > +}
> > +
> > +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
> > +{
> > +	if (static_branch_likely(&combo_qspinlock_key))
> > +		return queued_spin_trylock(lock);
> > +	else
> > +		return ticket_spin_trylock(lock);
> > +}
> > +
> > +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
> > +{
> > +	if (static_branch_likely(&combo_qspinlock_key))
> > +		queued_spin_unlock(lock);
> > +	else
> > +		ticket_spin_unlock(lock);
> > +}
> > +
> > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
> > +{
> > +	if (static_branch_likely(&combo_qspinlock_key))
> > +		return queued_spin_value_unlocked(lock);
> > +	else
> > +		return ticket_spin_value_unlocked(lock);
> > +}
> > +
> > +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
> > +{
> > +	if (static_branch_likely(&combo_qspinlock_key))
> > +		return queued_spin_is_locked(lock);
> > +	else
> > +		return ticket_spin_is_locked(lock);
> > +}
> > +
> > +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
> > +{
> > +	if (static_branch_likely(&combo_qspinlock_key))
> > +		return queued_spin_is_contended(lock);
> > +	else
> > +		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/setup.c b/arch/riscv/kernel/setup.c
> > index 32c2e1eb71bd..a447cf360a18 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -269,6 +269,18 @@ static void __init parse_dtb(void)
> >  #endif
> >  }
> >  
> > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > +DEFINE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > +EXPORT_SYMBOL(combo_qspinlock_key);
> > +#endif
> > +
> > +static void __init riscv_spinlock_init(void)
> > +{
> > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > +	static_branch_disable(&combo_qspinlock_key);
> > +#endif
> > +}
> > +
> >  extern void __init init_rt_signal_env(void);
> >  
> >  void __init setup_arch(char **cmdline_p)
> > @@ -317,6 +329,8 @@ void __init setup_arch(char **cmdline_p)
> >  	    riscv_isa_extension_available(NULL, ZICBOM))
> >  		riscv_noncoherent_supported();
> >  	riscv_set_dma_cache_alignment();
> > +
> > +	riscv_spinlock_init();
> >  }
> >  
> >  static int __init topology_init(void)
> > -- 
> > 2.36.1
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > 
>
Leonardo Bras Sept. 13, 2023, 8:49 p.m. UTC | #3
On Wed, Sep 13, 2023 at 05:37:01PM -0300, Leonardo Bras wrote:
> On Sun, Sep 10, 2023 at 07:06:23AM -0400, Guo Ren wrote:
> > On Sun, Sep 10, 2023 at 04:29:00AM -0400, 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 LR/SC)
> > > 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/spinlock.h | 78 ++++++++++++++++++++++++++++++-
> > >  arch/riscv/kernel/setup.c         | 14 ++++++
> > >  3 files changed, 98 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 7f39bfc75744..4bcff2860f48 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -473,7 +473,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"
> > > @@ -485,6 +485,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/spinlock.h b/arch/riscv/include/asm/spinlock.h
> > > index c644a92d4548..8ea0fee80652 100644
> > > --- a/arch/riscv/include/asm/spinlock.h
> > > +++ b/arch/riscv/include/asm/spinlock.h
> > > @@ -7,11 +7,85 @@
> > >  #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 <linux/jump_label.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
> > Sorry, I forgot __no_arch_spinlock_redefine advice here. I would add it in v12.
> > https://lore.kernel.org/linux-riscv/4cc7113a-0e4e-763a-cba2-7963bcd26c7a@redhat.com/
> > 
> 
> Please check a reply to a previous patch I sent earlier: I think these 
> #undef can be avoided.
>  
> > > +
> > > +DECLARE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > +
> > > +static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
> > > +{
> > > +	if (static_branch_likely(&combo_qspinlock_key))
> > > +		queued_spin_lock(lock);
> > > +	else
> > > +		ticket_spin_lock(lock);
> > > +}
> > > +
> > > +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
> > > +{
> > > +	if (static_branch_likely(&combo_qspinlock_key))
> > > +		return queued_spin_trylock(lock);
> > > +	else
> > > +		return ticket_spin_trylock(lock);
> > > +}
> > > +
> > > +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
> > > +{
> > > +	if (static_branch_likely(&combo_qspinlock_key))
> > > +		queued_spin_unlock(lock);
> > > +	else
> > > +		ticket_spin_unlock(lock);
> > > +}
> > > +
> > > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
> > > +{
> > > +	if (static_branch_likely(&combo_qspinlock_key))
> > > +		return queued_spin_value_unlocked(lock);
> > > +	else
> > > +		return ticket_spin_value_unlocked(lock);
> > > +}
> > > +
> > > +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
> > > +{
> > > +	if (static_branch_likely(&combo_qspinlock_key))
> > > +		return queued_spin_is_locked(lock);
> > > +	else
> > > +		return ticket_spin_is_locked(lock);
> > > +}
> > > +
> > > +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
> > > +{
> > > +	if (static_branch_likely(&combo_qspinlock_key))
> > > +		return queued_spin_is_contended(lock);
> > > +	else
> > > +		return ticket_spin_is_contended(lock);
> > > +}
> > > +#else /* CONFIG_RISCV_COMBO_SPINLOCKS */
> > > +

Also, those functions all reproduce the same behavior, so maybe it would be 
better to keep that behavior in a macro such as:

#define COMBO_SPINLOCK_DECLARE(f) 					\
static __always_inline int arch_spin_ ## f(arch_spinlock_t *lock) 	\
{									\
	if (static_branch_likely(&combo_qspinlock_key))			\
		return queued_spin_ ## f(lock);				\
	else								\
		return ticket_spin_ ## f(lock);				\
}

COMBO_SPINLOCK_DECLARE(lock)
COMBO_SPINLOCK_DECLARE(trylock)
COMBO_SPINLOCK_DECLARE(unlock)
COMBO_SPINLOCK_DECLARE(value_unlocked)
COMBO_SPINLOCK_DECLARE(is_locked)
COMBO_SPINLOCK_DECLARE(is_contended)

Does that make sense?

Thanks!
Leo


> > >  #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/setup.c b/arch/riscv/kernel/setup.c
> > > index 32c2e1eb71bd..a447cf360a18 100644
> > > --- a/arch/riscv/kernel/setup.c
> > > +++ b/arch/riscv/kernel/setup.c
> > > @@ -269,6 +269,18 @@ static void __init parse_dtb(void)
> > >  #endif
> > >  }
> > >  
> > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > +DEFINE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > +EXPORT_SYMBOL(combo_qspinlock_key);
> > > +#endif
> > > +
> > > +static void __init riscv_spinlock_init(void)
> > > +{
> > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > +	static_branch_disable(&combo_qspinlock_key);
> > > +#endif
> > > +}
> > > +
> > >  extern void __init init_rt_signal_env(void);
> > >  
> > >  void __init setup_arch(char **cmdline_p)
> > > @@ -317,6 +329,8 @@ void __init setup_arch(char **cmdline_p)
> > >  	    riscv_isa_extension_available(NULL, ZICBOM))
> > >  		riscv_noncoherent_supported();
> > >  	riscv_set_dma_cache_alignment();
> > > +
> > > +	riscv_spinlock_init();
> > >  }
> > >  
> > >  static int __init topology_init(void)
> > > -- 
> > > 2.36.1
> > > 
> > > 
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > 
> >
Guo Ren Sept. 14, 2023, 4:49 a.m. UTC | #4
On Thu, Sep 14, 2023 at 4:49 AM Leonardo Bras <leobras@redhat.com> wrote:
>
> On Wed, Sep 13, 2023 at 05:37:01PM -0300, Leonardo Bras wrote:
> > On Sun, Sep 10, 2023 at 07:06:23AM -0400, Guo Ren wrote:
> > > On Sun, Sep 10, 2023 at 04:29:00AM -0400, 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 LR/SC)
> > > > 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/spinlock.h | 78 ++++++++++++++++++++++++++++++-
> > > >  arch/riscv/kernel/setup.c         | 14 ++++++
> > > >  3 files changed, 98 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index 7f39bfc75744..4bcff2860f48 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -473,7 +473,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"
> > > > @@ -485,6 +485,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/spinlock.h b/arch/riscv/include/asm/spinlock.h
> > > > index c644a92d4548..8ea0fee80652 100644
> > > > --- a/arch/riscv/include/asm/spinlock.h
> > > > +++ b/arch/riscv/include/asm/spinlock.h
> > > > @@ -7,11 +7,85 @@
> > > >  #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 <linux/jump_label.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
> > > Sorry, I forgot __no_arch_spinlock_redefine advice here. I would add it in v12.
> > > https://lore.kernel.org/linux-riscv/4cc7113a-0e4e-763a-cba2-7963bcd26c7a@redhat.com/
> > >
> >
> > Please check a reply to a previous patch I sent earlier: I think these
> > #undef can be avoided.
> >
> > > > +
> > > > +DECLARE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > > +
> > > > +static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > +         queued_spin_lock(lock);
> > > > + else
> > > > +         ticket_spin_lock(lock);
> > > > +}
> > > > +
> > > > +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > +         return queued_spin_trylock(lock);
> > > > + else
> > > > +         return ticket_spin_trylock(lock);
> > > > +}
> > > > +
> > > > +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > +         queued_spin_unlock(lock);
> > > > + else
> > > > +         ticket_spin_unlock(lock);
> > > > +}
> > > > +
> > > > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > +         return queued_spin_value_unlocked(lock);
> > > > + else
> > > > +         return ticket_spin_value_unlocked(lock);
> > > > +}
> > > > +
> > > > +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > +         return queued_spin_is_locked(lock);
> > > > + else
> > > > +         return ticket_spin_is_locked(lock);
> > > > +}
> > > > +
> > > > +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
> > > > +{
> > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > +         return queued_spin_is_contended(lock);
> > > > + else
> > > > +         return ticket_spin_is_contended(lock);
> > > > +}
> > > > +#else /* CONFIG_RISCV_COMBO_SPINLOCKS */
> > > > +
>
> Also, those functions all reproduce the same behavior, so maybe it would be
> better to keep that behavior in a macro such as:
>
> #define COMBO_SPINLOCK_DECLARE(f)                                       \
> static __always_inline int arch_spin_ ## f(arch_spinlock_t *lock)       \
> {                                                                       \
>         if (static_branch_likely(&combo_qspinlock_key))                 \
>                 return queued_spin_ ## f(lock);                         \
>         else                                                            \
>                 return ticket_spin_ ## f(lock);                         \
> }
>
> COMBO_SPINLOCK_DECLARE(lock)
> COMBO_SPINLOCK_DECLARE(trylock)
> COMBO_SPINLOCK_DECLARE(unlock)
> COMBO_SPINLOCK_DECLARE(value_unlocked)
> COMBO_SPINLOCK_DECLARE(is_locked)
> COMBO_SPINLOCK_DECLARE(is_contended)
>
> Does that make sense?
Yeah, thanks. I would try. You improved my macro skills. :)

>
> Thanks!
> Leo
>
>
> > > >  #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/setup.c b/arch/riscv/kernel/setup.c
> > > > index 32c2e1eb71bd..a447cf360a18 100644
> > > > --- a/arch/riscv/kernel/setup.c
> > > > +++ b/arch/riscv/kernel/setup.c
> > > > @@ -269,6 +269,18 @@ static void __init parse_dtb(void)
> > > >  #endif
> > > >  }
> > > >
> > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > > +DEFINE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > > +EXPORT_SYMBOL(combo_qspinlock_key);
> > > > +#endif
> > > > +
> > > > +static void __init riscv_spinlock_init(void)
> > > > +{
> > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > > + static_branch_disable(&combo_qspinlock_key);
> > > > +#endif
> > > > +}
> > > > +
> > > >  extern void __init init_rt_signal_env(void);
> > > >
> > > >  void __init setup_arch(char **cmdline_p)
> > > > @@ -317,6 +329,8 @@ void __init setup_arch(char **cmdline_p)
> > > >       riscv_isa_extension_available(NULL, ZICBOM))
> > > >           riscv_noncoherent_supported();
> > > >   riscv_set_dma_cache_alignment();
> > > > +
> > > > + riscv_spinlock_init();
> > > >  }
> > > >
> > > >  static int __init topology_init(void)
> > > > --
> > > > 2.36.1
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-riscv mailing list
> > > > linux-riscv@lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > >
> > >
>
Leonardo Bras Sept. 14, 2023, 7:17 a.m. UTC | #5
On Thu, Sep 14, 2023 at 12:49:45PM +0800, Guo Ren wrote:
> On Thu, Sep 14, 2023 at 4:49 AM Leonardo Bras <leobras@redhat.com> wrote:
> >
> > On Wed, Sep 13, 2023 at 05:37:01PM -0300, Leonardo Bras wrote:
> > > On Sun, Sep 10, 2023 at 07:06:23AM -0400, Guo Ren wrote:
> > > > On Sun, Sep 10, 2023 at 04:29:00AM -0400, 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 LR/SC)
> > > > > 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/spinlock.h | 78 ++++++++++++++++++++++++++++++-
> > > > >  arch/riscv/kernel/setup.c         | 14 ++++++
> > > > >  3 files changed, 98 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > index 7f39bfc75744..4bcff2860f48 100644
> > > > > --- a/arch/riscv/Kconfig
> > > > > +++ b/arch/riscv/Kconfig
> > > > > @@ -473,7 +473,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"
> > > > > @@ -485,6 +485,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/spinlock.h b/arch/riscv/include/asm/spinlock.h
> > > > > index c644a92d4548..8ea0fee80652 100644
> > > > > --- a/arch/riscv/include/asm/spinlock.h
> > > > > +++ b/arch/riscv/include/asm/spinlock.h
> > > > > @@ -7,11 +7,85 @@
> > > > >  #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 <linux/jump_label.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
> > > > Sorry, I forgot __no_arch_spinlock_redefine advice here. I would add it in v12.
> > > > https://lore.kernel.org/linux-riscv/4cc7113a-0e4e-763a-cba2-7963bcd26c7a@redhat.com/
> > > >
> > >
> > > Please check a reply to a previous patch I sent earlier: I think these
> > > #undef can be avoided.
> > >
> > > > > +
> > > > > +DECLARE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > > > +
> > > > > +static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
> > > > > +{
> > > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > > +         queued_spin_lock(lock);
> > > > > + else
> > > > > +         ticket_spin_lock(lock);
> > > > > +}
> > > > > +
> > > > > +static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
> > > > > +{
> > > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > > +         return queued_spin_trylock(lock);
> > > > > + else
> > > > > +         return ticket_spin_trylock(lock);
> > > > > +}
> > > > > +
> > > > > +static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
> > > > > +{
> > > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > > +         queued_spin_unlock(lock);
> > > > > + else
> > > > > +         ticket_spin_unlock(lock);
> > > > > +}
> > > > > +
> > > > > +static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
> > > > > +{
> > > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > > +         return queued_spin_value_unlocked(lock);
> > > > > + else
> > > > > +         return ticket_spin_value_unlocked(lock);
> > > > > +}
> > > > > +
> > > > > +static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
> > > > > +{
> > > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > > +         return queued_spin_is_locked(lock);
> > > > > + else
> > > > > +         return ticket_spin_is_locked(lock);
> > > > > +}
> > > > > +
> > > > > +static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
> > > > > +{
> > > > > + if (static_branch_likely(&combo_qspinlock_key))
> > > > > +         return queued_spin_is_contended(lock);
> > > > > + else
> > > > > +         return ticket_spin_is_contended(lock);
> > > > > +}
> > > > > +#else /* CONFIG_RISCV_COMBO_SPINLOCKS */
> > > > > +
> >
> > Also, those functions all reproduce the same behavior, so maybe it would be
> > better to keep that behavior in a macro such as:
> >
> > #define COMBO_SPINLOCK_DECLARE(f)                                       \
> > static __always_inline int arch_spin_ ## f(arch_spinlock_t *lock)       \
> > {                                                                       \
> >         if (static_branch_likely(&combo_qspinlock_key))                 \
> >                 return queued_spin_ ## f(lock);                         \
> >         else                                                            \
> >                 return ticket_spin_ ## f(lock);                         \
> > }
> >
> > COMBO_SPINLOCK_DECLARE(lock)
> > COMBO_SPINLOCK_DECLARE(trylock)
> > COMBO_SPINLOCK_DECLARE(unlock)
> > COMBO_SPINLOCK_DECLARE(value_unlocked)
> > COMBO_SPINLOCK_DECLARE(is_locked)
> > COMBO_SPINLOCK_DECLARE(is_contended)
> >
> > Does that make sense?
> Yeah, thanks. I would try. You improved my macro skills. :)

Glad to be of any help :)

> 
> >
> > Thanks!
> > Leo
> >
> >
> > > > >  #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/setup.c b/arch/riscv/kernel/setup.c
> > > > > index 32c2e1eb71bd..a447cf360a18 100644
> > > > > --- a/arch/riscv/kernel/setup.c
> > > > > +++ b/arch/riscv/kernel/setup.c
> > > > > @@ -269,6 +269,18 @@ static void __init parse_dtb(void)
> > > > >  #endif
> > > > >  }
> > > > >
> > > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > > > +DEFINE_STATIC_KEY_TRUE(combo_qspinlock_key);
> > > > > +EXPORT_SYMBOL(combo_qspinlock_key);
> > > > > +#endif
> > > > > +
> > > > > +static void __init riscv_spinlock_init(void)
> > > > > +{
> > > > > +#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
> > > > > + static_branch_disable(&combo_qspinlock_key);
> > > > > +#endif
> > > > > +}
> > > > > +
> > > > >  extern void __init init_rt_signal_env(void);
> > > > >
> > > > >  void __init setup_arch(char **cmdline_p)
> > > > > @@ -317,6 +329,8 @@ void __init setup_arch(char **cmdline_p)
> > > > >       riscv_isa_extension_available(NULL, ZICBOM))
> > > > >           riscv_noncoherent_supported();
> > > > >   riscv_set_dma_cache_alignment();
> > > > > +
> > > > > + riscv_spinlock_init();
> > > > >  }
> > > > >
> > > > >  static int __init topology_init(void)
> > > > > --
> > > > > 2.36.1
> > > > >
> > > > >
> > > > > _______________________________________________
> > > > > linux-riscv mailing list
> > > > > linux-riscv@lists.infradead.org
> > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> > > > >
> > > >
> >
> 
> 
> -- 
> Best Regards
>  Guo Ren
>
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 7f39bfc75744..4bcff2860f48 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -473,7 +473,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"
@@ -485,6 +485,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/spinlock.h b/arch/riscv/include/asm/spinlock.h
index c644a92d4548..8ea0fee80652 100644
--- a/arch/riscv/include/asm/spinlock.h
+++ b/arch/riscv/include/asm/spinlock.h
@@ -7,11 +7,85 @@ 
 #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 <linux/jump_label.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
+
+DECLARE_STATIC_KEY_TRUE(combo_qspinlock_key);
+
+static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
+{
+	if (static_branch_likely(&combo_qspinlock_key))
+		queued_spin_lock(lock);
+	else
+		ticket_spin_lock(lock);
+}
+
+static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
+{
+	if (static_branch_likely(&combo_qspinlock_key))
+		return queued_spin_trylock(lock);
+	else
+		return ticket_spin_trylock(lock);
+}
+
+static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
+{
+	if (static_branch_likely(&combo_qspinlock_key))
+		queued_spin_unlock(lock);
+	else
+		ticket_spin_unlock(lock);
+}
+
+static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
+{
+	if (static_branch_likely(&combo_qspinlock_key))
+		return queued_spin_value_unlocked(lock);
+	else
+		return ticket_spin_value_unlocked(lock);
+}
+
+static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
+{
+	if (static_branch_likely(&combo_qspinlock_key))
+		return queued_spin_is_locked(lock);
+	else
+		return ticket_spin_is_locked(lock);
+}
+
+static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
+{
+	if (static_branch_likely(&combo_qspinlock_key))
+		return queued_spin_is_contended(lock);
+	else
+		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/setup.c b/arch/riscv/kernel/setup.c
index 32c2e1eb71bd..a447cf360a18 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -269,6 +269,18 @@  static void __init parse_dtb(void)
 #endif
 }
 
+#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
+DEFINE_STATIC_KEY_TRUE(combo_qspinlock_key);
+EXPORT_SYMBOL(combo_qspinlock_key);
+#endif
+
+static void __init riscv_spinlock_init(void)
+{
+#ifdef CONFIG_RISCV_COMBO_SPINLOCKS
+	static_branch_disable(&combo_qspinlock_key);
+#endif
+}
+
 extern void __init init_rt_signal_env(void);
 
 void __init setup_arch(char **cmdline_p)
@@ -317,6 +329,8 @@  void __init setup_arch(char **cmdline_p)
 	    riscv_isa_extension_available(NULL, ZICBOM))
 		riscv_noncoherent_supported();
 	riscv_set_dma_cache_alignment();
+
+	riscv_spinlock_init();
 }
 
 static int __init topology_init(void)