On Thu Nov 10, 2022 at 10:40 AM AEST, Jordan Niethe wrote:
> On Thu, 2022-07-28 at 16:31 +1000, Nicholas Piggin wrote:
> [resend as utf-8, not utf-7]
> > Store the owner CPU number in the lock word so it may be yielded to,
> > as powerpc's paravirtualised simple spinlocks do.
> > ---
> > arch/powerpc/include/asm/qspinlock.h | 8 +++++++-
> > arch/powerpc/include/asm/qspinlock_types.h | 10 ++++++++++
> > arch/powerpc/lib/qspinlock.c | 6 +++---
> > 3 files changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/qspinlock.h
> > b/arch/powerpc/include/asm/qspinlock.h
> > index 3ab354159e5e..44601b261e08 100644
> > --- a/arch/powerpc/include/asm/qspinlock.h
> > +++ b/arch/powerpc/include/asm/qspinlock.h
> > @@ -20,9 +20,15 @@ static __always_inline int
> > queued_spin_is_contended(struct qspinlock *lock)
> > return !!(READ_ONCE(lock->val) & _Q_TAIL_CPU_MASK);
> > }
> >
> > +static __always_inline u32 queued_spin_get_locked_val(void)
>
> Maybe this function should have "encode" in the name to match with
> encode_tail_cpu().
Yep.
> > +{
> > + /* XXX: make this use lock value in paca like simple spinlocks? */
>
> Is that the paca's lock_token which is 0x8000?
Yes, which AFAIKS is actually unused now with queued spinlocks.
> > + return _Q_LOCKED_VAL | (smp_processor_id() << _Q_OWNER_CPU_OFFSET);
> > +}
> > +
> > static __always_inline int queued_spin_trylock(struct qspinlock *lock)
> > {
> > - u32 new = _Q_LOCKED_VAL;
> > + u32 new = queued_spin_get_locked_val();
> > u32 prev;
> >
> > asm volatile(
> > diff --git a/arch/powerpc/include/asm/qspinlock_types.h
> > b/arch/powerpc/include/asm/qspinlock_types.h
> > index 8b20f5e22bba..35f9525381e6 100644
> > --- a/arch/powerpc/include/asm/qspinlock_types.h
> > +++ b/arch/powerpc/include/asm/qspinlock_types.h
> > @@ -29,6 +29,8 @@ typedef struct qspinlock {
> > * Bitfields in the lock word:
> > *
> > * 0: locked bit
> > + * 1-14: lock holder cpu
> > + * 15: unused bit
> > * 16: must queue bit
> > * 17-31: tail cpu (+1)
>
> So there is one more bit to store the tail cpu vs the lock holder cpu?
Yeah but the tail has to encode it as CPU+1.
Thanks,
Nick
