On Thu, Nov 09, 2023 at 10:22:13PM -0800, Linus Torvalds wrote:
> On Thu, 9 Nov 2023 at 21:57, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > So something like this should fix lockref. ENTIRELY UNTESTED, except
> > now the code generation of lockref_put_return() looks much better,
> > without a pointless flush to the stack, and now it has no pointless
> > stack frame as a result.
>
> Heh. And because I was looking at Al's tree, I didn't notice that
> commit c6f4a9002252 ("asm-generic: ticket-lock: Optimize
> arch_spin_value_unlocked()") had solved the ticket spinlock part of
> this in this merge window in the meantime.
>
> The qspinlock implementation - which is what x86 uses - is still
> broken in mainline, though.
>
> So that part of my patch still stands. Now attached just the small
> one-liner part. Adding Ingo and Guo Ren, who did the ticket lock part
> (and looks to have done it very similarly to my suggested patch.
Not only generic ticket lock, I think Will Deacon recognized the lockref
problem of the arm32 ticket-lock in 2013.
After my patch merged, I think riscv could also select
ARCH_USE_CMPXCHG_LOCKREF in its Kconfig.
Ref:
commit 0cbad9c9dfe0c38e8ec7385b39087c005a6dee3e
Author: Will Deacon <will@xxxxxxxxxx>
Date: Wed Oct 9 17:19:22 2013 +0100
ARM: 7854/1: lockref: add support for lockless lockrefs using
cmpxchg64
Our spinlocks are only 32-bit (2x16-bit tickets) and, on processors
with 64-bit atomic instructions, cmpxchg64 makes use of the
double-word
exclusive accessors.
This patch wires up the cmpxchg-based lockless lockref
implementation
for ARM.
Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
Signed-off-by: Russell King <rmk+kernel@xxxxxxxxxxxxxxxx>
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1ad6fb6c094d..fc184bcd7848 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -5,6 +5,7 @@ config ARM
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAVE_CUSTOM_GPIO_H
+ select ARCH_USE_CMPXCHG_LOCKREF
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_EXTABLE_SORT if MMU
select CLONE_BACKWARDS
diff --git a/arch/arm/include/asm/spinlock.h
b/arch/arm/include/asm/spinlock.h
index 4f2c28060c9a..ed6c22919e47 100644
--- a/arch/arm/include/asm/spinlock.h
+++ b/arch/arm/include/asm/spinlock.h
@@ -127,10 +127,14 @@ static inline void
arch_spin_unlock(arch_spinlock_t *lock)
dsb_sev();
}
+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)
{
- struct __raw_tickets tickets = ACCESS_ONCE(lock->tickets);
- return tickets.owner != tickets.next;
+ return !arch_spin_value_unlocked(ACCESS_ONCE(*lock));
}
static inline int arch_spin_is_contended(arch_spinlock_t *lock)
>
> Ingo?
>
> Linus
> include/asm-generic/qspinlock.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
> index 995513fa2690..0655aa5b57b2 100644
> --- a/include/asm-generic/qspinlock.h
> +++ b/include/asm-generic/qspinlock.h
> @@ -70,7 +70,7 @@ static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
> */
> static __always_inline int queued_spin_value_unlocked(struct qspinlock lock)
> {
> - return !atomic_read(&lock.val);
> + return !lock.val.counter;
> }
>
> /**
