Messages in this thread | | | Date | Tue, 12 Jul 2022 10:06:52 +1000 | From | Nicholas Piggin <> | Subject | Re: [PATCH 02/13] locking/qspinlock: inline mcs_spinlock functions into qspinlock |
| |
Excerpts from Peter Zijlstra's message of July 6, 2022 2:57 am: > On Tue, Jul 05, 2022 at 12:38:09AM +1000, Nicholas Piggin wrote: >> qspinlock uses mcs_spinlock for the struct type (.next, .locked, and the >> misplaced .count), and arch_mcs_spin_{un}lock_contended(). These can be >> trivially inlined into qspinlock, and the unused mcs_spinlock code >> removed. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- > >> arch/arm/include/asm/mcs_spinlock.h | 24 ------ > >> --- a/arch/arm/include/asm/mcs_spinlock.h >> +++ /dev/null >> @@ -1,24 +0,0 @@ >> -/* SPDX-License-Identifier: GPL-2.0 */ >> -#ifndef __ASM_MCS_LOCK_H >> -#define __ASM_MCS_LOCK_H >> - >> -#ifdef CONFIG_SMP >> -#include <asm/spinlock.h> >> - >> -/* MCS spin-locking. */ >> -#define arch_mcs_spin_lock_contended(lock) \ >> -do { \ >> - /* Ensure prior stores are observed before we enter wfe. */ \ >> - smp_mb(); \ >> - while (!(smp_load_acquire(lock))) \ >> - wfe(); \ >> -} while (0) \ >> - >> -#define arch_mcs_spin_unlock_contended(lock) \ >> -do { \ >> - smp_store_release(lock, 1); \ >> - dsb_sev(); \ >> -} while (0) >> - >> -#endif /* CONFIG_SMP */ >> -#endif /* __ASM_MCS_LOCK_H */ > >> @@ -475,7 +476,8 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val) >> WRITE_ONCE(prev->next, node); >> >> pv_wait_node(node, prev); >> - arch_mcs_spin_lock_contended(&node->locked); >> + /* Wait for mcs node lock to be released */ >> + smp_cond_load_acquire(&node->locked, VAL); >> >> /* >> * While waiting for the MCS lock, the next pointer may have >> @@ -554,7 +556,7 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val) >> if (!next) >> next = smp_cond_load_relaxed(&node->next, (VAL)); >> >> - arch_mcs_spin_unlock_contended(&next->locked); >> + smp_store_release(&next->locked, 1); /* unlock the mcs node lock */ > > These are not equivalent. Now it so happens that ARM doesn't use > qspinlock and the other mcs lock users are gone by now, but something > like this should at least have been called out in the Changelog I think. >
Yeah I could mention it in the changelog, I just didn't because it is practically equivalent.
Thanks, Nick
| |