On Wed, 25 Mar 2026 15:55:21 +0000
Catalin Marinas [off-list ref] wrote:

On Mon, Mar 16, 2026 at 11:37:12PM +0000, David Laight wrote:

...

quoted

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