On 11/3/25 17:48, Stephen Hemminger wrote:
On Thu, 23 Oct 2025 18:47:24 +0000
Wathsala Vithanage <[email protected]> wrote:
Fix incorrect memory ordering in the MCS lock implementation by
adding proper synchronizing edges to establish clear happens-before
relationships between threads invoking lock() and unlock(). These
synchronizing edges prevent potential deadlocks caused by improper
ordering and are documented in detail through in-code comments.
The previously relaxed load of the successor’s lock object pointer
in unlock() has been upgraded to a load-acquire operation. This
change ensures that the successor’s initialization does not
overwrite the current lock holder’s update to the locked field,
which could otherwise lead to deadlocks.
Remove two unnecessary fences:
The acquire fence in unlock() had no matching release fence, making
it ineffective for enforcing memory order. The associated comment
suggested it prevented speculative reordering, but such fences (data
memory barriers) only establish memory ordering and do not control
instruction speculation.
The release-acquire fence pair in lock() was previously justified as
preventing reordering between the load-acquire loop of me->locked
and the store-release of prev->next. This is no longer needed, as the
new synchronizing edges ensure a chain of happens-before
relationships between memory operations of threads calling lock() and
unlock().
Signed-off-by: Wathsala Vithanage <[email protected]>
Reviewed-by: Ola Liljedahl <[email protected]>
Thanks for the good explanatory comments.
Could you please add a Fixes: tag and Cc: [email protected]
so it can go to the right stable releases as well.
I noticed that Progress64 has same effective code.
Yes, the P64 MCS implementation is aligned with the DPDK version
after these changes.
Ola’s verification tool confirms that the P64 implementation is correct,
so this should be correct as well.
