| Message ID | 20220430153626.30660-6-palmer@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Generic Ticket Spinlocks | expand |
Am Samstag, 30. April 2022, 17:36:24 CEST schrieb Palmer Dabbelt: > From: Palmer Dabbelt <palmer@rivosinc.com> > > Our existing spinlocks aren't fair and replacing them has been on the > TODO list for a long time. This moves to the recently-introduced ticket > spinlocks, which are simple enough that they are likely to be correct > and fast on the vast majority of extant implementations. > > This introduces a horrible hack that allows us to split out the spinlock > conversion from the rwlock conversion. We have to do the spinlocks > first because qrwlock needs fair spinlocks, but we don't want to pollute > the asm-generic code to support the generic spinlocks without qrwlocks. > Thus we pollute the RISC-V code, but just until the next commit as it's > all going away. > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> on riscv64+riscv32 qemu, beaglev and d1-nezha Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Guo Ren <guoren@kernel.org> On Wed, May 4, 2022 at 8:02 PM Heiko Stübner <heiko@sntech.de> wrote: > > Am Samstag, 30. April 2022, 17:36:24 CEST schrieb Palmer Dabbelt: > > From: Palmer Dabbelt <palmer@rivosinc.com> > > > > Our existing spinlocks aren't fair and replacing them has been on the > > TODO list for a long time. This moves to the recently-introduced ticket > > spinlocks, which are simple enough that they are likely to be correct > > and fast on the vast majority of extant implementations. > > > > This introduces a horrible hack that allows us to split out the spinlock > > conversion from the rwlock conversion. We have to do the spinlocks > > first because qrwlock needs fair spinlocks, but we don't want to pollute > > the asm-generic code to support the generic spinlocks without qrwlocks. > > Thus we pollute the RISC-V code, but just until the next commit as it's > > all going away. > > > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > > on riscv64+riscv32 qemu, beaglev and d1-nezha > > Tested-by: Heiko Stuebner <heiko@sntech.de> > >
On 30/04/2022 16:36, Palmer Dabbelt wrote: > From: Palmer Dabbelt <palmer@rivosinc.com> > > Our existing spinlocks aren't fair and replacing them has been on the > TODO list for a long time. This moves to the recently-introduced ticket > spinlocks, which are simple enough that they are likely to be correct > and fast on the vast majority of extant implementations. > > This introduces a horrible hack that allows us to split out the spinlock > conversion from the rwlock conversion. We have to do the spinlocks > first because qrwlock needs fair spinlocks, but we don't want to pollute > the asm-generic code to support the generic spinlocks without qrwlocks. > Thus we pollute the RISC-V code, but just until the next commit as it's > all going away. > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> I am loathe to add a TB tag since I have not done much by way of testing any realistic use cases - but I have put it in our CI and have had a play around with it locally & nothing obviously broke for me. If you think that is sufficient: Tested-by: Conor Dooley <conor.dooley@microchip.com> Otherwise feel free to ignore the tag. Thanks, Conor. > --- > arch/riscv/include/asm/Kbuild | 2 ++ > arch/riscv/include/asm/spinlock.h | 44 +++---------------------- > arch/riscv/include/asm/spinlock_types.h | 9 +++-- > 3 files changed, 10 insertions(+), 45 deletions(-) > > diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild > index 5edf5b8587e7..c3f229ae8033 100644 > --- a/arch/riscv/include/asm/Kbuild > +++ b/arch/riscv/include/asm/Kbuild > @@ -3,5 +3,7 @@ generic-y += early_ioremap.h > generic-y += flat.h > generic-y += kvm_para.h > generic-y += parport.h > +generic-y += qrwlock.h > +generic-y += qrwlock_types.h > generic-y += user.h > generic-y += vmlinux.lds.h > diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h > index f4f7fa1b7ca8..88a4d5d0d98a 100644 > --- a/arch/riscv/include/asm/spinlock.h > +++ b/arch/riscv/include/asm/spinlock.h > @@ -7,49 +7,13 @@ > #ifndef _ASM_RISCV_SPINLOCK_H > #define _ASM_RISCV_SPINLOCK_H > > +/* This is horible, but the whole file is going away in the next commit. */ > +#define __ASM_GENERIC_QRWLOCK_H > + > #include <linux/kernel.h> > #include <asm/current.h> > #include <asm/fence.h> > - > -/* > - * Simple spin lock operations. These provide no fairness guarantees. > - */ > - > -/* FIXME: Replace this with a ticket lock, like MIPS. */ > - > -#define arch_spin_is_locked(x) (READ_ONCE((x)->lock) != 0) > - > -static inline void arch_spin_unlock(arch_spinlock_t *lock) > -{ > - smp_store_release(&lock->lock, 0); > -} > - > -static inline int arch_spin_trylock(arch_spinlock_t *lock) > -{ > - int tmp = 1, busy; > - > - __asm__ __volatile__ ( > - " amoswap.w %0, %2, %1\n" > - RISCV_ACQUIRE_BARRIER > - : "=r" (busy), "+A" (lock->lock) > - : "r" (tmp) > - : "memory"); > - > - return !busy; > -} > - > -static inline void arch_spin_lock(arch_spinlock_t *lock) > -{ > - while (1) { > - if (arch_spin_is_locked(lock)) > - continue; > - > - if (arch_spin_trylock(lock)) > - break; > - } > -} > - > -/***********************************************************/ > +#include <asm-generic/spinlock.h> > > static inline void arch_read_lock(arch_rwlock_t *lock) > { > diff --git a/arch/riscv/include/asm/spinlock_types.h b/arch/riscv/include/asm/spinlock_types.h > index 5a35a49505da..f2f9b5d7120d 100644 > --- a/arch/riscv/include/asm/spinlock_types.h > +++ b/arch/riscv/include/asm/spinlock_types.h > @@ -6,15 +6,14 @@ > #ifndef _ASM_RISCV_SPINLOCK_TYPES_H > #define _ASM_RISCV_SPINLOCK_TYPES_H > > +/* This is horible, but the whole file is going away in the next commit. */ > +#define __ASM_GENERIC_QRWLOCK_TYPES_H > + > #ifndef __LINUX_SPINLOCK_TYPES_RAW_H > # error "please don't include this file directly" > #endif > > -typedef struct { > - volatile unsigned int lock; > -} arch_spinlock_t; > - > -#define __ARCH_SPIN_LOCK_UNLOCKED { 0 } > +#include <asm-generic/spinlock_types.h> > > typedef struct { > volatile unsigned int lock;
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild index 5edf5b8587e7..c3f229ae8033 100644 --- a/arch/riscv/include/asm/Kbuild +++ b/arch/riscv/include/asm/Kbuild @@ -3,5 +3,7 @@ generic-y += early_ioremap.h generic-y += flat.h generic-y += kvm_para.h generic-y += parport.h +generic-y += qrwlock.h +generic-y += qrwlock_types.h generic-y += user.h generic-y += vmlinux.lds.h diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h index f4f7fa1b7ca8..88a4d5d0d98a 100644 --- a/arch/riscv/include/asm/spinlock.h +++ b/arch/riscv/include/asm/spinlock.h @@ -7,49 +7,13 @@ #ifndef _ASM_RISCV_SPINLOCK_H #define _ASM_RISCV_SPINLOCK_H +/* This is horible, but the whole file is going away in the next commit. */ +#define __ASM_GENERIC_QRWLOCK_H + #include <linux/kernel.h> #include <asm/current.h> #include <asm/fence.h> - -/* - * Simple spin lock operations. These provide no fairness guarantees. - */ - -/* FIXME: Replace this with a ticket lock, like MIPS. */ - -#define arch_spin_is_locked(x) (READ_ONCE((x)->lock) != 0) - -static inline void arch_spin_unlock(arch_spinlock_t *lock) -{ - smp_store_release(&lock->lock, 0); -} - -static inline int arch_spin_trylock(arch_spinlock_t *lock) -{ - int tmp = 1, busy; - - __asm__ __volatile__ ( - " amoswap.w %0, %2, %1\n" - RISCV_ACQUIRE_BARRIER - : "=r" (busy), "+A" (lock->lock) - : "r" (tmp) - : "memory"); - - return !busy; -} - -static inline void arch_spin_lock(arch_spinlock_t *lock) -{ - while (1) { - if (arch_spin_is_locked(lock)) - continue; - - if (arch_spin_trylock(lock)) - break; - } -} - -/***********************************************************/ +#include <asm-generic/spinlock.h> static inline void arch_read_lock(arch_rwlock_t *lock) { diff --git a/arch/riscv/include/asm/spinlock_types.h b/arch/riscv/include/asm/spinlock_types.h index 5a35a49505da..f2f9b5d7120d 100644 --- a/arch/riscv/include/asm/spinlock_types.h +++ b/arch/riscv/include/asm/spinlock_types.h @@ -6,15 +6,14 @@ #ifndef _ASM_RISCV_SPINLOCK_TYPES_H #define _ASM_RISCV_SPINLOCK_TYPES_H +/* This is horible, but the whole file is going away in the next commit. */ +#define __ASM_GENERIC_QRWLOCK_TYPES_H + #ifndef __LINUX_SPINLOCK_TYPES_RAW_H # error "please don't include this file directly" #endif -typedef struct { - volatile unsigned int lock; -} arch_spinlock_t; - -#define __ARCH_SPIN_LOCK_UNLOCKED { 0 } +#include <asm-generic/spinlock_types.h> typedef struct { volatile unsigned int lock;