Messages in this thread | | | Date | Wed, 27 Mar 2024 21:52:55 -0400 | Subject | Re: [PATCH 1/2] locking/pvqspinlock: Use try_cmpxchg_acquire() in trylock_clear_pending() | From | Waiman Long <> |
| |
On 3/25/24 10:09, Uros Bizjak wrote: > Use try_cmpxchg_acquire(*ptr, &old, new) instead of > cmpxchg_relaxed(*ptr, old, new) == old in trylock_clear_pending(). > x86 CMPXCHG instruction returns success in ZF flag, so this change > saves a compare after cmpxchg. > > Also change the return type of the function to bool. > > No functional change intended. > > Signed-off-by: Uros Bizjak <ubizjak@gmail.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Will Deacon <will@kernel.org> > Cc: Waiman Long <longman@redhat.com> > Cc: Boqun Feng <boqun.feng@gmail.com> > --- > kernel/locking/qspinlock_paravirt.h | 31 ++++++++++++----------------- > 1 file changed, 13 insertions(+), 18 deletions(-) > > diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h > index 169950fe1aad..77ba80bd95f9 100644 > --- a/kernel/locking/qspinlock_paravirt.h > +++ b/kernel/locking/qspinlock_paravirt.h > @@ -116,11 +116,12 @@ static __always_inline void set_pending(struct qspinlock *lock) > * barrier. Therefore, an atomic cmpxchg_acquire() is used to acquire the > * lock just to be sure that it will get it. > */ > -static __always_inline int trylock_clear_pending(struct qspinlock *lock) > +static __always_inline bool trylock_clear_pending(struct qspinlock *lock) > { > + u16 old = _Q_PENDING_VAL; > + > return !READ_ONCE(lock->locked) && > - (cmpxchg_acquire(&lock->locked_pending, _Q_PENDING_VAL, > - _Q_LOCKED_VAL) == _Q_PENDING_VAL); > + try_cmpxchg_acquire(&lock->locked_pending, &old, _Q_LOCKED_VAL); > } > #else /* _Q_PENDING_BITS == 8 */ > static __always_inline void set_pending(struct qspinlock *lock) > @@ -128,27 +129,21 @@ static __always_inline void set_pending(struct qspinlock *lock) > atomic_or(_Q_PENDING_VAL, &lock->val); > } > > -static __always_inline int trylock_clear_pending(struct qspinlock *lock) > +static __always_inline bool trylock_clear_pending(struct qspinlock *lock) > { > - int val = atomic_read(&lock->val); > - > - for (;;) { > - int old, new; > - > - if (val & _Q_LOCKED_MASK) > - break; > + int old, new; > > + old = atomic_read(&lock->val); > + do { > + if (old & _Q_LOCKED_MASK) > + return false; > /* > * Try to clear pending bit & set locked bit > */ > - old = val; > - new = (val & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > - val = atomic_cmpxchg_acquire(&lock->val, old, new); > + new = (old & ~_Q_PENDING_MASK) | _Q_LOCKED_VAL; > + } while (!atomic_try_cmpxchg_acquire (&lock->val, &old, new)); > > - if (val == old) > - return 1; > - } > - return 0; > + return true; > } > #endif /* _Q_PENDING_BITS == 8 */ > Reviewed-by: Waiman Long <longman@redhat.com>
| |