Looks like feedback has been largely positive on this one. I think I
got everything from the v1 and v2, but it was a bit mixed up so sorry if
I missed something. I'm generally being conservative on the tags here,
as things have drifted around a bit. Specifically I dropped the
Tested-bys, as this is all based on 5.18-rc1 now and there's been a
touch of diff.
I've put this at palmer/tspinlock-v3, in case that helps anyone. This
generally looks good to me, but I'll wait for feedback before putting it
anywhere else. I'd default to doing a shared tag for the asm-generic
stuff and then let other arch folks pull in that (with their arch
support), but if you want me to take it via my tree then feel free to
just say so explicitly. What's on that branch right now definately
shouldn't be treated as stable, though, as I'll wait for at least an
official Ack/Review from the asm-generic folks (and of course there may
be more feedback).
This passes my standard tests, both as the whole thing and as just the
RISC-V spinlock change. That's just QEMU, though, so it's not all that
exciting.
Changes since v2 <[email protected]>:
* Picked up Peter's SOBs, which were posted on the v1.
* Re-ordered the first two patches, as they
* Re-worded the RISC-V qrwlock patch, as it was a bit mushy. I also
added a blurb in the qrwlock's top comment about this dependency.
* Picked up Stafford's fix for big-endian systems, which I have not
tested as I don't have one (at least easily availiable, I think the BE
MIPS systems are still in that pile in my garage).
* Call the generic version <asm-genenic/spinlock{_types}.h>, as there's
really no utility to the version that only errors out.
Changes since v1 <[email protected]>:
* Follow Arnd suggestion to make the patch series more generic.
* Add csky in the series.
* Combine RISC-V's two patches into one.
* Modify openrisc's patch to suit the new generic version.
From: Guo Ren <[email protected]>
There is no benefit from custom implementation for ticket-spinlock,
so move to generic ticket-spinlock for easy maintenance.
Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/csky/include/asm/Kbuild | 3 +
arch/csky/include/asm/spinlock.h | 89 --------------------------
arch/csky/include/asm/spinlock_types.h | 27 --------
3 files changed, 3 insertions(+), 116 deletions(-)
delete mode 100644 arch/csky/include/asm/spinlock.h
delete mode 100644 arch/csky/include/asm/spinlock_types.h
diff --git a/arch/csky/include/asm/Kbuild b/arch/csky/include/asm/Kbuild
index 888248235c23..103207a58f97 100644
--- a/arch/csky/include/asm/Kbuild
+++ b/arch/csky/include/asm/Kbuild
@@ -3,7 +3,10 @@ generic-y += asm-offsets.h
generic-y += extable.h
generic-y += gpio.h
generic-y += kvm_para.h
+generic-y += spinlock.h
+generic-y += spinlock_types.h
generic-y += qrwlock.h
+generic-y += qrwlock_types.h
generic-y += parport.h
generic-y += user.h
generic-y += vmlinux.lds.h
diff --git a/arch/csky/include/asm/spinlock.h b/arch/csky/include/asm/spinlock.h
deleted file mode 100644
index 69f5aa249c5f..000000000000
--- a/arch/csky/include/asm/spinlock.h
+++ /dev/null
@@ -1,89 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-#ifndef __ASM_CSKY_SPINLOCK_H
-#define __ASM_CSKY_SPINLOCK_H
-
-#include <linux/spinlock_types.h>
-#include <asm/barrier.h>
-
-/*
- * Ticket-based spin-locking.
- */
-static inline void arch_spin_lock(arch_spinlock_t *lock)
-{
- arch_spinlock_t lockval;
- u32 ticket_next = 1 << TICKET_NEXT;
- u32 *p = &lock->lock;
- u32 tmp;
-
- asm volatile (
- "1: ldex.w %0, (%2) \n"
- " mov %1, %0 \n"
- " add %0, %3 \n"
- " stex.w %0, (%2) \n"
- " bez %0, 1b \n"
- : "=&r" (tmp), "=&r" (lockval)
- : "r"(p), "r"(ticket_next)
- : "cc");
-
- while (lockval.tickets.next != lockval.tickets.owner)
- lockval.tickets.owner = READ_ONCE(lock->tickets.owner);
-
- smp_mb();
-}
-
-static inline int arch_spin_trylock(arch_spinlock_t *lock)
-{
- u32 tmp, contended, res;
- u32 ticket_next = 1 << TICKET_NEXT;
- u32 *p = &lock->lock;
-
- do {
- asm volatile (
- " ldex.w %0, (%3) \n"
- " movi %2, 1 \n"
- " rotli %1, %0, 16 \n"
- " cmpne %1, %0 \n"
- " bt 1f \n"
- " movi %2, 0 \n"
- " add %0, %0, %4 \n"
- " stex.w %0, (%3) \n"
- "1: \n"
- : "=&r" (res), "=&r" (tmp), "=&r" (contended)
- : "r"(p), "r"(ticket_next)
- : "cc");
- } while (!res);
-
- if (!contended)
- smp_mb();
-
- return !contended;
-}
-
-static inline void arch_spin_unlock(arch_spinlock_t *lock)
-{
- smp_mb();
- WRITE_ONCE(lock->tickets.owner, lock->tickets.owner + 1);
-}
-
-static inline int arch_spin_value_unlocked(arch_spinlock_t lock)
-{
- return lock.tickets.owner == lock.tickets.next;
-}
-
-static inline int arch_spin_is_locked(arch_spinlock_t *lock)
-{
- return !arch_spin_value_unlocked(READ_ONCE(*lock));
-}
-
-static inline int arch_spin_is_contended(arch_spinlock_t *lock)
-{
- struct __raw_tickets tickets = READ_ONCE(lock->tickets);
-
- return (tickets.next - tickets.owner) > 1;
-}
-#define arch_spin_is_contended arch_spin_is_contended
-
-#include <asm/qrwlock.h>
-
-#endif /* __ASM_CSKY_SPINLOCK_H */
diff --git a/arch/csky/include/asm/spinlock_types.h b/arch/csky/include/asm/spinlock_types.h
deleted file mode 100644
index db87a12c3827..000000000000
--- a/arch/csky/include/asm/spinlock_types.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-#ifndef __ASM_CSKY_SPINLOCK_TYPES_H
-#define __ASM_CSKY_SPINLOCK_TYPES_H
-
-#ifndef __LINUX_SPINLOCK_TYPES_RAW_H
-# error "please don't include this file directly"
-#endif
-
-#define TICKET_NEXT 16
-
-typedef struct {
- union {
- u32 lock;
- struct __raw_tickets {
- /* little endian */
- u16 owner;
- u16 next;
- } tickets;
- };
-} arch_spinlock_t;
-
-#define __ARCH_SPIN_LOCK_UNLOCKED { { 0 } }
-
-#include <asm-generic/qrwlock_types.h>
-
-#endif /* __ASM_CSKY_SPINLOCK_TYPES_H */
--
2.34.1
From: Palmer Dabbelt <[email protected]>
Now that we have fair spinlocks we can use the generic queued rwlocks,
so we might as well do so.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/Kbuild | 2 +
arch/riscv/include/asm/spinlock.h | 99 -------------------------
arch/riscv/include/asm/spinlock_types.h | 24 ------
4 files changed, 3 insertions(+), 123 deletions(-)
delete mode 100644 arch/riscv/include/asm/spinlock.h
delete mode 100644 arch/riscv/include/asm/spinlock_types.h
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 00fd9c548f26..f8a55d94016d 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -39,6 +39,7 @@ config RISCV
select ARCH_SUPPORTS_DEBUG_PAGEALLOC if MMU
select ARCH_SUPPORTS_HUGETLBFS if MMU
select ARCH_USE_MEMTEST
+ select ARCH_USE_QUEUED_RWLOCKS
select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU
select ARCH_WANT_FRAME_POINTERS
select ARCH_WANT_GENERAL_HUGETLB
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index c3f229ae8033..504f8b7e72d4 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -3,6 +3,8 @@ generic-y += early_ioremap.h
generic-y += flat.h
generic-y += kvm_para.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 += user.h
diff --git a/arch/riscv/include/asm/spinlock.h b/arch/riscv/include/asm/spinlock.h
deleted file mode 100644
index 88a4d5d0d98a..000000000000
--- a/arch/riscv/include/asm/spinlock.h
+++ /dev/null
@@ -1,99 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) 2015 Regents of the University of California
- * Copyright (C) 2017 SiFive
- */
-
-#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>
-#include <asm-generic/spinlock.h>
-
-static inline void arch_read_lock(arch_rwlock_t *lock)
-{
- int tmp;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bltz %1, 1b\n"
- " addi %1, %1, 1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- : "+A" (lock->lock), "=&r" (tmp)
- :: "memory");
-}
-
-static inline void arch_write_lock(arch_rwlock_t *lock)
-{
- int tmp;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bnez %1, 1b\n"
- " li %1, -1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- : "+A" (lock->lock), "=&r" (tmp)
- :: "memory");
-}
-
-static inline int arch_read_trylock(arch_rwlock_t *lock)
-{
- int busy;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bltz %1, 1f\n"
- " addi %1, %1, 1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- "1:\n"
- : "+A" (lock->lock), "=&r" (busy)
- :: "memory");
-
- return !busy;
-}
-
-static inline int arch_write_trylock(arch_rwlock_t *lock)
-{
- int busy;
-
- __asm__ __volatile__(
- "1: lr.w %1, %0\n"
- " bnez %1, 1f\n"
- " li %1, -1\n"
- " sc.w %1, %1, %0\n"
- " bnez %1, 1b\n"
- RISCV_ACQUIRE_BARRIER
- "1:\n"
- : "+A" (lock->lock), "=&r" (busy)
- :: "memory");
-
- return !busy;
-}
-
-static inline void arch_read_unlock(arch_rwlock_t *lock)
-{
- __asm__ __volatile__(
- RISCV_RELEASE_BARRIER
- " amoadd.w x0, %1, %0\n"
- : "+A" (lock->lock)
- : "r" (-1)
- : "memory");
-}
-
-static inline void arch_write_unlock(arch_rwlock_t *lock)
-{
- smp_store_release(&lock->lock, 0);
-}
-
-#endif /* _ASM_RISCV_SPINLOCK_H */
diff --git a/arch/riscv/include/asm/spinlock_types.h b/arch/riscv/include/asm/spinlock_types.h
deleted file mode 100644
index f2f9b5d7120d..000000000000
--- a/arch/riscv/include/asm/spinlock_types.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) 2015 Regents of the University of California
- */
-
-#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
-
-#include <asm-generic/spinlock_types.h>
-
-typedef struct {
- volatile unsigned int lock;
-} arch_rwlock_t;
-
-#define __ARCH_RW_LOCK_UNLOCKED { 0 }
-
-#endif /* _ASM_RISCV_SPINLOCK_TYPES_H */
--
2.34.1
From: Peter Zijlstra <[email protected]>
We have no indications that openrisc meets the qspinlock requirements,
so move to ticket-spinlock as that is more likey to be correct.
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/openrisc/Kconfig | 1 -
arch/openrisc/include/asm/Kbuild | 5 ++--
arch/openrisc/include/asm/spinlock.h | 27 ----------------------
arch/openrisc/include/asm/spinlock_types.h | 7 ------
4 files changed, 2 insertions(+), 38 deletions(-)
delete mode 100644 arch/openrisc/include/asm/spinlock.h
delete mode 100644 arch/openrisc/include/asm/spinlock_types.h
diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
index 0d68adf6e02b..99f0e4a4cbbd 100644
--- a/arch/openrisc/Kconfig
+++ b/arch/openrisc/Kconfig
@@ -30,7 +30,6 @@ config OPENRISC
select HAVE_DEBUG_STACKOVERFLOW
select OR1K_PIC
select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
- select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_USE_QUEUED_RWLOCKS
select OMPIC if SMP
select ARCH_WANT_FRAME_POINTERS
diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
index ca5987e11053..3386b9c1c073 100644
--- a/arch/openrisc/include/asm/Kbuild
+++ b/arch/openrisc/include/asm/Kbuild
@@ -1,9 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
generic-y += extable.h
generic-y += kvm_para.h
-generic-y += mcs_spinlock.h
-generic-y += qspinlock_types.h
-generic-y += qspinlock.h
+generic-y += spinlock_types.h
+generic-y += spinlock.h
generic-y += qrwlock_types.h
generic-y += qrwlock.h
generic-y += user.h
diff --git a/arch/openrisc/include/asm/spinlock.h b/arch/openrisc/include/asm/spinlock.h
deleted file mode 100644
index 264944a71535..000000000000
--- a/arch/openrisc/include/asm/spinlock.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * OpenRISC Linux
- *
- * Linux architectural port borrowing liberally from similar works of
- * others. All original copyrights apply as per the original source
- * declaration.
- *
- * OpenRISC implementation:
- * Copyright (C) 2003 Matjaz Breskvar <[email protected]>
- * Copyright (C) 2010-2011 Jonas Bonn <[email protected]>
- * et al.
- */
-
-#ifndef __ASM_OPENRISC_SPINLOCK_H
-#define __ASM_OPENRISC_SPINLOCK_H
-
-#include <asm/qspinlock.h>
-
-#include <asm/qrwlock.h>
-
-#define arch_spin_relax(lock) cpu_relax()
-#define arch_read_relax(lock) cpu_relax()
-#define arch_write_relax(lock) cpu_relax()
-
-
-#endif
diff --git a/arch/openrisc/include/asm/spinlock_types.h b/arch/openrisc/include/asm/spinlock_types.h
deleted file mode 100644
index 7c6fb1208c88..000000000000
--- a/arch/openrisc/include/asm/spinlock_types.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef _ASM_OPENRISC_SPINLOCK_TYPES_H
-#define _ASM_OPENRISC_SPINLOCK_TYPES_H
-
-#include <asm/qspinlock_types.h>
-#include <asm/qrwlock_types.h>
-
-#endif /* _ASM_OPENRISC_SPINLOCK_TYPES_H */
--
2.34.1
From: Palmer Dabbelt <[email protected]>
I could only find the fairness requirements documented as the C code,
this calls them out in a comment just to be a bit more explicit.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
include/asm-generic/qrwlock.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h
index 7ae0ece07b4e..24ae09c1db9f 100644
--- a/include/asm-generic/qrwlock.h
+++ b/include/asm-generic/qrwlock.h
@@ -2,6 +2,10 @@
/*
* Queue read/write lock
*
+ * These use generic atomic and locking routines, but depend on a fair spinlock
+ * implementation in order to be fair themselves. The implementation in
+ * asm-generic/spinlock.h meets these requirements.
+ *
* (C) Copyright 2013-2014 Hewlett-Packard Development Company, L.P.
*
* Authors: Waiman Long <[email protected]>
--
2.34.1
On Thu, Apr 14, 2022 at 03:02:11PM -0700, Palmer Dabbelt wrote:
> From: Peter Zijlstra <[email protected]>
>
> We have no indications that openrisc meets the qspinlock requirements,
> so move to ticket-spinlock as that is more likey to be correct.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> ---
> arch/openrisc/Kconfig | 1 -
> arch/openrisc/include/asm/Kbuild | 5 ++--
> arch/openrisc/include/asm/spinlock.h | 27 ----------------------
> arch/openrisc/include/asm/spinlock_types.h | 7 ------
> 4 files changed, 2 insertions(+), 38 deletions(-)
> delete mode 100644 arch/openrisc/include/asm/spinlock.h
> delete mode 100644 arch/openrisc/include/asm/spinlock_types.h
>
> diff --git a/arch/openrisc/Kconfig b/arch/openrisc/Kconfig
> index 0d68adf6e02b..99f0e4a4cbbd 100644
> --- a/arch/openrisc/Kconfig
> +++ b/arch/openrisc/Kconfig
> @@ -30,7 +30,6 @@ config OPENRISC
> select HAVE_DEBUG_STACKOVERFLOW
> select OR1K_PIC
> select CPU_NO_EFFICIENT_FFS if !OPENRISC_HAVE_INST_FF1
> - select ARCH_USE_QUEUED_SPINLOCKS
> select ARCH_USE_QUEUED_RWLOCKS
> select OMPIC if SMP
> select ARCH_WANT_FRAME_POINTERS
> diff --git a/arch/openrisc/include/asm/Kbuild b/arch/openrisc/include/asm/Kbuild
> index ca5987e11053..3386b9c1c073 100644
> --- a/arch/openrisc/include/asm/Kbuild
> +++ b/arch/openrisc/include/asm/Kbuild
> @@ -1,9 +1,8 @@
> # SPDX-License-Identifier: GPL-2.0
> generic-y += extable.h
> generic-y += kvm_para.h
> -generic-y += mcs_spinlock.h
> -generic-y += qspinlock_types.h
> -generic-y += qspinlock.h
> +generic-y += spinlock_types.h
> +generic-y += spinlock.h
> generic-y += qrwlock_types.h
> generic-y += qrwlock.h
> generic-y += user.h
> diff --git a/arch/openrisc/include/asm/spinlock.h b/arch/openrisc/include/asm/spinlock.h
> deleted file mode 100644
> index 264944a71535..000000000000
> --- a/arch/openrisc/include/asm/spinlock.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * OpenRISC Linux
> - *
> - * Linux architectural port borrowing liberally from similar works of
> - * others. All original copyrights apply as per the original source
> - * declaration.
> - *
> - * OpenRISC implementation:
> - * Copyright (C) 2003 Matjaz Breskvar <[email protected]>
> - * Copyright (C) 2010-2011 Jonas Bonn <[email protected]>
> - * et al.
> - */
> -
> -#ifndef __ASM_OPENRISC_SPINLOCK_H
> -#define __ASM_OPENRISC_SPINLOCK_H
> -
> -#include <asm/qspinlock.h>
> -
> -#include <asm/qrwlock.h>
> -
> -#define arch_spin_relax(lock) cpu_relax()
> -#define arch_read_relax(lock) cpu_relax()
> -#define arch_write_relax(lock) cpu_relax()
> -
> -
> -#endif
> diff --git a/arch/openrisc/include/asm/spinlock_types.h b/arch/openrisc/include/asm/spinlock_types.h
> deleted file mode 100644
> index 7c6fb1208c88..000000000000
> --- a/arch/openrisc/include/asm/spinlock_types.h
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -#ifndef _ASM_OPENRISC_SPINLOCK_TYPES_H
> -#define _ASM_OPENRISC_SPINLOCK_TYPES_H
> -
> -#include <asm/qspinlock_types.h>
> -#include <asm/qrwlock_types.h>
> -
> -#endif /* _ASM_OPENRISC_SPINLOCK_TYPES_H */
Thanks for this, a bit late but.
Acked-by: Stafford Horne <[email protected]>