Re: [PATCH] locking/osq_lock: Ensure proper locking semantics for osq_lock/osq_unlock()
From: Waiman Long
Date: Mon Sep 14 2026 - 22:34:10 EST
On 9/14/26 7:22 AM, Peter Zijlstra wrote:
On Thu, Sep 10, 2026 at 10:19:08AM -0400, Waiman Long wrote: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 osq_lock is special in the sense that lock transfer from one CPU toYou need to substantiate this *should*.
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 are missing in
some places. Fix that by adding the needed barriers in those places.
Note that the two percpu optimistic_spin_node.locked setting in
osq_unlock() are proceeded by a full barrier xchg() call, but the
contended cachelines are different. This should probably work in most
cases except in some exotic architectures where the barrier semantics
may be cacheline specific. Nevertheless a release barrier is still added
for safety reason as we may opt to relax the xchg() calls in the future.
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. 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.
Currently osq_lock is used only by mutex and rw_semaphore code for queuingThis doesn't make sense. You cannot argue that the code is correct as is
purpose. As a result, the imperfect lock synchronization support does
not cause harmful consequence as the new osq_lock owner of a contended
osq_lock will still have to wait for the real mutex and rwsem lock to
be released by the pervious osq_lock owner before it can acquire it and
go into its critical section. For correctness, we still have to fix it
in case it is used elsewhere which doesn't have this inherent protection.
Fixes: 036cc30c6b6a ("locking/osq: No need for load/acquire when acquire-polling")
and still add Fixes.
That is true. I added this paragraph to show that it is not a serious bug, but I still think it is better to fix it or we have to explicitly say that it is not a real lock in term of how it can be used. I have taken it out to avoid confusion.
Cheers,
Longman