Re: [PATCH] locking/osq_lock: Ensure proper locking semantics for osq_lock/osq_unlock()

[Haakon Bugge]

> On 15 Sep 2026, at 10:26, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> 
> On Mon, Sep 14, 2026 at 10:30:12PM -0400, Waiman Long wrote:
> > On 9/14/26 7:22 AM, Peter Zijlstra wrote:
> 
> > > > The "node->locked" read in osq_lock() was relaxed by commit 036cc30c6ba
> > > > ("locking/osq: No need for load/acquire when acquire-polling") a while
> > > > ago as the smp_load_acquire() loop was causing a performance hit due to
> > > > the repeated acquire barriers in the loop and it argued that an earlier
> > > > atomic_xchg() call could provide the needed barrier. That may not be
> > > > enough especially if we have to loop for a while before the lock is
> > > > released. Now with the new smp_cond_load_acquire() helper, only one
> > > > acquire barrier is added at the end of the loop. So it shouldn't have
> > > > the performance hit noted in that commit.
> 
> > > You need to substantiate this *should*.
> 
> > I don't have a good benchmark that can show any noticeable performance
> > difference by adding an acquire barrier. My suspicion is that a repeating
> > acquire barrier can be expensive depending on the actual processor used.
> > Anyway the other osq_lock/osq_unlock() entry and exit points all have
> > suitable barrier. I doubt adding one more acquire barrier will have a
> > noticeable impact given that it is one-off at the end instead of a repeating
> > one like the original osq_lock() code.
> 
> The obvious benchmark is the one that got used for 036cc30c6ba, no?

The code in 036cc30c6b was quite different. The
"while (!ACCESS_ONCE(node->locked))" loop doesn't exist
today. I did a quick test, by adding a debug print of the accumulated
number of iterations for my mx_test testing mutex's. So, bigger is better.
Average of 100 runs of 10 seconds on a 160 CPU Arm BM system:

    _relaxed	   __acquire
3978996981,9  	3986106562,2

So, Waiman's _acquire variant is a modest 0,18% faster on this system.


Thxs, Håkon