Re: [PATCH 1/1] eal: correct memory ordering in MCS lock

[Wathsala Vithanage <wathsala.vithanage@arm.com>]

On 11/3/25 17:48, Stephen Hemminger wrote:
> On Thu, 23 Oct 2025 18:47:24 +0000
> Wathsala Vithanage <wathsala.vithanage@arm.com> 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 <wathsala.vithanage@arm.com>
>> Reviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>
> Thanks for the good explanatory comments.
>
> Could you please add a Fixes: tag and Cc: stable@dpdk.org
> 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.