Re: [PATCH bpf-next v1 08/22] rqspinlock: Protect pending bit owners from stalls

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Jan 07, 2025 at 10:44:16PM +0530, Kumar Kartikeya Dwivedi wrote:
> On Tue, 7 Jan 2025 at 20:22, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Tue, Jan 07, 2025 at 05:59:50AM -0800, Kumar Kartikeya Dwivedi wrote:
> > > +     if (val & _Q_LOCKED_MASK) {
> > > +             RES_RESET_TIMEOUT(ts);
> > > +             smp_cond_load_acquire(&lock->locked, !VAL || RES_CHECK_TIMEOUT(ts, ret));
> > > +     }
> >
> > Please check how smp_cond_load_acquire() works on ARM64 and then add
> > some words on how RES_CHECK_TIMEOUT() is still okay.
> 
> Thanks Peter,
> 
> The __cmpwait_relaxed bit does indeed look problematic, my
> understanding is that the ldxr + wfe sequence can get stuck because we
> may not have any updates on the &lock->locked address, and we’ll not
> call into RES_CHECK_TIMEOUT since that cond_expr check precedes the
> __cmpwait macro.

IIRC the WFE will wake at least on every interrupt but might have an
inherent timeout itself, so it will make some progress, but not at a
speed comparable to a pure spin.

> Do you have suggestions on resolving this? We want to invoke this
> macro as part of the waiting loop. We can have a
> rqspinlock_smp_cond_load_acquire that maps to no-WFE smp_load_acquire
> loop on arm64 and uses the asm-generic version elsewhere.

That will make arm64 sad -- that wfe thing is how they get away with not
having paravirt spinlocks iirc. Also power consumption.

I've not read well enough to remember what order of timeout you're
looking for, but you could have the tick sample the lock like a watchdog
like, and write a magic 'lock' value when it is deemed stuck.