Re: [PATCH v4 next 0/9] locking/osq_lock: Optimisations to osq_lock code

From: Waiman Long

Date: Wed Sep 09 2026 - 17:06:33 EST


On 9/9/26 10:15 AM, Haakon Bugge wrote:

On 7 Sep 2026, at 19:27, David Laight <david.laight.linux@xxxxxxxxx> wrote:

On Mon, 7 Sep 2026 09:08:28 -0700
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

On Mon, 7 Sept 2026 at 01:41, David Laight <david.laight.linux@xxxxxxxxx> wrote:
I've fixed some broken/missing memory barriers but left the initial xchg()
when acquiring the lock as a full barrier, I think it could be relaxed.
Well, it should almost certainly be at least an
atomic_cmpxchg_acquire(), since that's what osq_wait_next() uses for
the contention case.
I'm not sure, but am no expert on acquire/release barriers.
The 'fast path' osq_lock() code only has one memory access so there
isn't anything to sequence it with.
The important one is the smp_wmb() a bit lower down that ensures the
list tail (or head) is written before the back link.
When that was missing things went badly wrong.
(I think the WRITE_ONCE() could be a store_release() instead.)

The ACQUIRE semantics were added to ensure the 'node->next = NULL'
assignment happened before the xchg().
That assignment goes away in patch 5.
But I'd want someone who really understands arm64 to comment.
These are preliminary results. I added osq_lock's to my
mutual-exclusion selftest [1], which has not yet been reviewed. The
test is based on v7.3-rc2.

For lock acquisition, I used:

preempt_disable();
while (!osq_lock(&el->mx_osq_lock.lock)) {
preempt_enable();
cond_resched();
preempt_disable();
}

with the corresponding release:

osq_unlock(&el->mx_osq_lock.lock);
preempt_enable();

Assuming that this is a correct use of the OSQ API, the OSQ test fails
on a 160-CPU bare-metal Arm system. The same test passes on a 512-CPU
AMD x86_64 system as expected, showing at least that the test is
capable of passing.

osq_unlock() must provide the release barrier. I think the two "WRITE_ONCE(next->locked, 1)" should have been "smp_store_release(&next->locked, 1)".  There is an xchg() call before the WRITE_ONCE's, but it is on a different cacheline so it may not apply.

Could you make that change to the existing code and rerun the test again on arm64 to see if it can pass?

Thanks,
Longman