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

[Waiman Long <longman@redhat.com>]

On 9/9/26 10:15 AM, Haakon Bugge wrote:
>
>> On 7 Sep 2026, at 19:27, David Laight <david.laight.linux@gmail.com> wrote:
>>
>> On Mon, 7 Sep 2026 09:08:28 -0700
>> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>
>>> On Mon, 7 Sept 2026 at 01:41, David Laight <david.laight.linux@gmail.com> 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