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

[David Laight]

On Thu, 10 Sep 2026 15:30:03 +0000
Haakon Bugge <haakon.bugge@xxxxxxxxxx> wrote:

...
> > > > Do you know which part matters?   
> > > 
> > > No, but now that I am able to test the OSQ locks as a module, I'll
> > > quickly find out.  
> 
> Only the first hunk is allegedly required:
> 
> @@ -143,7 +143,7 @@ bool osq_lock(struct optimistic_spin_queue *lock)
>          * is implemented with a monitor-wait. vcpu_is_preempted() relies on
>          * polling, be careful.
>          */
> -       if (smp_cond_load_relaxed(&node->locked, VAL || need_resched() ||
> +       if (smp_cond_load_acquire(&node->locked, VAL || need_resched() ||
> 	    vcpu_is_preempted(node_cpu(node->prev))))
>                 return true;
> 
> I say allegedly because a passing test doesn't prove anything, it just
> gives a good indication that it is working.

Indeed.

> > That also means you can quickly check which _acquire/_release are definitely
> > required.  
> 
> I did it in another way. I just added an "atomic_xchg" test to my
> mutual exclusion framework.
> 
> Lock acquire:
>                 while (atomic_xchg_acquire(&el->mx_atomic_xchg.lock, 1) != 0)
>                         cpu_relax();
> 
> Lock release:
>                 atomic_set_release(&el->mx_atomic_xchg.lock, 0);
> 
> This passes. It also (obviously) passes with atomic_xchg() in the lock
> acquire. But the _release _is_ required in the lock release.

Isn't that accessing another branch of the 'big union'?
So will be overlaying some other field - possibly the count itself?
It seems unlikely that adding a barrier between the lock and unlock
(or unlock and lock) will have any effect.
In any case I think it is enough to use smp_rmb(), smp_wmb() or smp_mb().

David