On Thu Nov 10, 2022 at 4:37 PM AEST, Christophe Leroy wrote:
>
>
> Le 10/11/2022 à 01:35, Jordan Niethe a écrit :
> > On Thu, 2022-07-28 at 16:31 +1000, Nicholas Piggin wrote:
> > <snip>
> >> -#define queued_spin_lock queued_spin_lock
> >>
> >> -static inline void queued_spin_unlock(struct qspinlock *lock)
> >> +static __always_inline int queued_spin_trylock(struct qspinlock *lock)
> >> {
> >> - if (!IS_ENABLED(CONFIG_PARAVIRT_SPINLOCKS) || !is_shared_processor())
> >> - smp_store_release(&lock->locked, 0);
> >> - else
> >> - __pv_queued_spin_unlock(lock);
> >> + if (atomic_cmpxchg_acquire(&lock->val, 0, 1) == 0)
> >> + return 1;
> >> + return 0;
> >
> > optional style nit: return (atomic_cmpxchg_acquire(&lock->val, 0, 1) == 0);
>
> No parenthesis.
> No == 0
>
> Should be :
>
> return !atomic_cmpxchg_acquire(&lock->val, 0, 1);
In this case I prefer the == 0 because we're testing against the 0 old
parameter being passed in. This is the recognisable cmpxchg pattern.
The other style of cmpxchg returns true if it succeeded, so it's less
clear we're not using that version if using !.
Thanks,
Nick
