Re: [PATCH V5] riscv: Add qspinlock support

From: Conor.Dooley
Date: Mon Jun 20 2022 - 11:58:44 EST




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 */