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

[Haakon Bugge]

> 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.

I then applied this series, but the OSQ test still failed in the same
way on Arm. I observed no new mutex or rwsem test failures on Arm, and
the test continued to pass on the x86_64 system.


Thxs, Håkon

[1] https://lore.kernel.org/lkml/20260817130239.343594-1-haakon.bugge@xxxxxxxxxx/


> 
>> 
>> It's a bit odd that the first initial xchg uses a different memory
>> ordering than the later one. Maybe there's some reason for it.
> 
> I think the 'entry' ones want to be acquire and the 'exit' ones release.
> osq_unlock() used release, but the equivalent code in osq_wait_next()
> used acquire.
> They can't both have been correct!
> 
>> 
>> But even more importantly, that code right now  explicitly *states*
>> that it needs a full barrier ("We need both ACQUIRE [..] and
>> RELEASE"), so that *comment* would also have to be fixed with a why
>> the ordering isn't as important as it states.
> 
> I left that comment alone - matching the xchg().
> Even though there are now no fields to publish.
> 
>> And finally: none of that will ever be noticeable on x86, since there
>> are no memory orderings on atomics there: lock is all-or-nothing.
> 
> Indeed.
> I don't have a little arm test system, never mind a big one where this
> would all show up.
> 
>> End result: I'd love to see actual performance numbers if they exist.
>> And any memory ordering change would require explaining why it's ok
>> and some other architecture to test it.
> 
> This could even be one of the strange places where making the code
> slower actually speeds things up overall.
> osq_lock() is only used for contended sleep locks, and then not even for
> the first thread to be waiting.
> If you get a lot of threads queued you really need to fix the locking!
> 
>> 
>> Or am I missing something?
> 
> Probably the same thing as I am....
> 
> David
> 
>> 
>>            Linus