Re: [PATCH v10 00/12] barrier: Add smp_cond_load_{relaxed,acquire}_timeout()
From: David Laight
Date: Wed Mar 25 2026 - 15:36:33 EST
On Wed, 25 Mar 2026 15:55:21 +0000
Catalin Marinas <catalin.marinas@xxxxxxx> wrote:
> On Mon, Mar 16, 2026 at 11:37:12PM +0000, David Laight wrote:
...
> > For osq_lock(), while an IPI will wake it up, there is also a small timing
> > window where the IPI can happen before the ldx and so not actually wake up it.
> > This is true whenever 'expr' is non-trivial.
>
> Hmm, I thought this is fine because of the implicit SEVL on exception
> return but the arm64 __cmpwait_relaxed() does a SEVL+WFE which clears
> any prior event, it can wait in theory forever when the event stream is
> disabled.
Not forever, there will be a timer interrupt in the end.
> Expanding smp_cond_load_relaxed() into asm, we have something like:
>
> LDR X0, [PTR]
> condition check for VAL || need_resched() with branch out
> SEVL
> WFE
> LDXR X1, [PTR]
> EOR X1, X1, X0
> CBNZ out
> WFE
> out:
>
> If the condition is updated to become true (need_resched()) after the
> condition check but before the first WFE while *PTR remains unchanged,
> the IPI won't do anything. Maybe we should revert 1cfc63b5ae60 ("arm64:
> cmpwait: Clear event register before arming exclusive monitor"). Not
> great but probably better than reverting f5bfdc8e3947 ("locking/osq: Use
> optimized spinning loop for arm64")).
Could you change the order to:
LDR X0, [PTR]
SEVL
WFE
condition check for VAL || need_resched() with branch out
LDXR X1, [PTR]
EOR X1, X1, X0
CBNZ out
WFE
out:
that closes the timing window for the interrupt provided the condition
check doesn't change the event register.
I must get back to the osq_lock code again.
I'm happy with the code - the per-cpu data is down to two cpu numbers.
(Apart from the acquire/release semantics in a few places.)
But the comments have got out of hand.
Writing succinct and accurate comments is hard - too verbose and they
hide too much code.
David