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

[David Laight]

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.

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