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

[Peter Zijlstra]

On Mon, Sep 14, 2026 at 04:21:02PM -0400, Waiman Long wrote:
> The osq_lock is special in the sense that lock transfer from one CPU to
> the next can happen either over the common optimistic_spin_queue.tail
> value with uncontended lock or over a lock waiter's own percpu
> optimistic_spin_node.locked flag when the lock is contended.
> 
> To ensure proper lock synchronization, we need to provide
> the acquire/release semantics for the osq_lock/osq_unlock()
> functions in both cases. This is currently the case for the
> common optimistic_spin_queue.tail value, but not for the percpu
> optimistic_spin_node.locked flag as the proper barriers can be missing.
> 
> The "node->locked" read in osq_lock() was relaxed by commit 036cc30c6b6a
> ("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 and reordering
> wasn't a problem in the way osq_lock is being used by mutex and rwsem
> for queuing purpose only. That atomic_xchg() barrier does not work
> as a proper acquire barrier for osq_lock() if the lock hasn't been
> acquired or isn't ready to be acquired when the barrier ends. So an
> acquire barrier is still needed in order to have proper locking semantics.
> 
> The performance impact stated in that patch is due to repeated issuance
> of acquire barrier which can be expensive depending on the architectures
> and the actual processor used. It was not clear what machine and what
> benchmark was being used to produce the performance data. Anyway, with
> the new smp_cond_load_acquire() helper, only one acquire barrier is
> issued at the end of the loop. So even if there is a performance impact,
> it should be less than a repeating one.
> 
> As for the two percpu optimistic_spin_node.locked setting in osq_unlock(),
> they are currently preceded by a full barrier xchg() call which can
> provide the needed release barrier. Add comments saying that a release
> barrier is needed for the proper functioning of the unlock operation
> to alert people from accidentally remove the barrier when the code is
> updated.
> 
> Fixes: 036cc30c6b6a ("locking/osq: No need for load/acquire when acquire-polling")
> Tested-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>
> Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
> ---
>  kernel/locking/osq_lock.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> [v2] Reword the commit log and keep the WRITE_ONCE() in osq_unlock()
>      with comments.
> [v3] Fix the comment above smp_cond_load_acquire().

I still see no reason why this should be applied. Or even have this
Fixes tag.