Re: [PATCH V5] riscv: Add qspinlock support

[Conor.Dooley]

On 20/06/2022 16:54, guoren@xxxxxxxxxx wrote:
> From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> 
> Enable qspinlock and meet the requirements mentioned in a8ad07e5240c9
> ("asm-generic: qspinlock: Indicate the use of mixed-size atomics").
> 
>  - RISC-V atomic_*_release()/atomic_*_acquire() are implemented with
>    own relaxed version plus acquire/release_fence for RCsc
>    synchronization.
> 
>  - RISC-V LR/SC pairs could provide a strong/weak forward guarantee
>    that depends on micro-architecture. And RISC-V ISA spec has given
>    out several limitations to let hardware support strict forward
>    guarantee (RISC-V User ISA - 8.3 Eventual Success of
>    Store-Conditional Instructions). Some riscv cores such as BOOMv3
>    & XiangShan could provide strict & strong forward guarantee (The
>    cache line would be kept in an exclusive state for Backoff cycles,
>    and only this core's interrupt could break the LR/SC pair).
> 
>  - RISC-V could provide cheap atomic_fetch_or_acquire() with RCsc.
> 
>  - RISC-V only provides relaxed xhg16 to support qspinlock.
> 
> The first version of patch was made in 2019.1 [1]. The second version
> was made in 2020.11 [2].
> 
> [1] https://lore.kernel.org/linux-riscv/20190211043829.30096-1-michaeljclark@xxxxxxx/#r
> [2] https://lore.kernel.org/linux-riscv/1606225437-22948-2-git-send-email-guoren@xxxxxxxxxx/
> 
> Change V5:
>  - Update comment with RISC-V forward guarantee feature.
>  - Back to V3 direction and optimize asm code.
> 
> Change V4:
>  - Remove custom sub-word xchg implementation
>  - Add ARCH_USE_QUEUED_SPINLOCKS_XCHG32 in locking/qspinlock
> 
> Change V3:
>  - Coding convention by Peter Zijlstra's advices
> 
> Change V2:
>  - Coding convention in cmpxchg.h
>  - Re-implement short xchg
>  - Remove char & cmpxchg implementations

Hey,
How come the changelog is inside the commit message?
I assume its a copy paste error..
Thanks,
Conor.

> 
> Signed-off-by: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Guo Ren <guoren@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Waiman Long <longman@xxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
> ---
>  arch/riscv/Kconfig                      |  9 +++++++++
>  arch/riscv/include/asm/Kbuild           |  4 ++--
>  arch/riscv/include/asm/cmpxchg.h        | 16 ++++++++++++++++
>  arch/riscv/include/asm/spinlock.h       | 12 ++++++++++++
>  arch/riscv/include/asm/spinlock_types.h | 12 ++++++++++++
>  5 files changed, 51 insertions(+), 2 deletions(-)
>  create mode 100644 arch/riscv/include/asm/spinlock.h
>  create mode 100644 arch/riscv/include/asm/spinlock_types.h
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 32ffef9f6e5b..3b0b117b4e95 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -333,6 +333,15 @@ config NODES_SHIFT
>  	  Specify the maximum number of NUMA Nodes available on the target
>  	  system.  Increases memory reserved to accommodate various tables.
>  
> +config RISCV_USE_QUEUED_SPINLOCKS
> +	bool "Using queued spinlock instead of ticket-lock"
> +	depends on SMP && MMU
> +	select ARCH_USE_QUEUED_SPINLOCKS
> +	default y if NUMA
> +	help
> +	  Make sure your micro arch LL/SC has a strong forward progress guarantee.
> +	  Otherwise, stay at ticket-lock.
> +
>  config RISCV_ALTERNATIVE
>  	bool
>  	depends on !XIP_KERNEL
> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
> index 504f8b7e72d4..e066ccab6417 100644
> --- a/arch/riscv/include/asm/Kbuild
> +++ b/arch/riscv/include/asm/Kbuild
> @@ -2,10 +2,10 @@
>  generic-y += early_ioremap.h
>  generic-y += flat.h
>  generic-y += kvm_para.h
> +generic-y += mcs_spinlock.h
>  generic-y += parport.h
> -generic-y += spinlock.h
> -generic-y += spinlock_types.h
>  generic-y += qrwlock.h
>  generic-y += qrwlock_types.h
> +generic-y += qspinlock.h
>  generic-y += user.h
>  generic-y += vmlinux.lds.h
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 12debce235e5..f7f8e359d3ac 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -17,6 +17,22 @@
>  	__typeof__(new) __new = (new);					\
>  	__typeof__(*(ptr)) __ret;					\
>  	switch (size) {							\
> +	case 2:								\
> +		u32 temp;						\
> +		u32 shif = ((ulong)__ptr & 2) ? 16 : 0;			\
> +		u32 mask = 0xffff << shif;				\
> +		__ptr = (__typeof__(ptr))((ulong)__ptr & ~(ulong)2);	\
> +		__asm__ __volatile__ (					\
> +			"0:	lr.w %0, %2\n"				\
> +			"	and  %1, %0, %z3\n"			\
> +			"	or   %1, %1, %z4\n"			\
> +			"	sc.w %1, %1, %2\n"			\
> +			"	bnez %1, 0b\n"				\
> +			: "=&r" (__ret), "=&r" (temp), "+A" (*__ptr)	\
> +			: "rJ" (~mask), "rJ" (__new << shif)		\
> +			: "memory");					\
> +		__ret = (__ret & mask) >> shif;				\
> +		break;							\
>  	case 4:								\
>  		__asm__ __volatile__ (					\
>  			"	amoswap.w %0, %2, %1\n"			\
> diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h
> new file mode 100644
> index 000000000000..fd3fd09cff52
> --- /dev/null
> +++ b/arch/riscv/include/asm/spinlock.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_SPINLOCK_H
> +#define __ASM_SPINLOCK_H
> +
> +#ifdef CONFIG_ARCH_USE_QUEUED_SPINLOCKS
> +#include <asm/qspinlock.h>
> +#include <asm/qrwlock.h>
> +#else
> +#include <asm-generic/spinlock.h>
> +#endif
> +
> +#endif /* __ASM_SPINLOCK_H */
> diff --git a/arch/riscv/include/asm/spinlock_types.h b/arch/riscv/include/asm/spinlock_types.h
> new file mode 100644
> index 000000000000..9281237b5f4e
> --- /dev/null
> +++ b/arch/riscv/include/asm/spinlock_types.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_SPINLOCK_TYPES_H
> +#define __ASM_SPINLOCK_TYPES_H
> +
> +#ifdef CONFIG_ARCH_USE_QUEUED_SPINLOCKS
> +#include <asm-generic/qspinlock_types.h>
> +#include <asm-generic/qrwlock_types.h>
> +#else
> +#include <asm-generic/spinlock_types.h>
> +#endif
> +
> +#endif /* __ASM_SPINLOCK_TYPES_H */