From: Stephen Hemminger <stephen@networkplumber.org>
To: Wathsala Vithanage <wathsala.vithanage@arm.com>
Cc: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>,
Tyler Retzlaff <roretzla@linux.microsoft.com>,
dev@dpdk.org, Ola Liljedahl <ola.liljedahl@arm.com>
Subject: Re: [PATCH 1/1] eal: correct memory ordering in MCS lock
Date: Mon, 3 Nov 2025 15:48:46 -0800 [thread overview]
Message-ID: <20251103154846.07be1aa0@phoenix> (raw)
In-Reply-To: <20251023184724.1759497-1-wathsala.vithanage@arm.com>
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.
next prev parent reply other threads:[~2025-11-03 23:48 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-23 18:47 [PATCH 1/1] eal: correct memory ordering in MCS lock Wathsala Vithanage
2025-11-03 15:12 ` Wathsala Vithanage
2025-11-03 17:07 ` Stephen Hemminger
2025-11-03 17:30 ` Wathsala Vithanage
2025-11-03 18:06 ` Konstantin Ananyev
2025-11-03 18:47 ` Wathsala Vithanage
2025-11-03 18:48 ` Stephen Hemminger
2025-11-03 19:13 ` Wathsala Vithanage
2025-11-04 8:18 ` Konstantin Ananyev
2025-11-05 19:39 ` Ola Liljedahl
2025-11-06 7:48 ` Konstantin Ananyev
2025-11-06 14:10 ` Ola Liljedahl
2025-11-03 23:48 ` Stephen Hemminger [this message]
2025-11-04 23:30 ` Wathsala Vithanage
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251103154846.07be1aa0@phoenix \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=honnappa.nagarahalli@arm.com \
--cc=ola.liljedahl@arm.com \
--cc=roretzla@linux.microsoft.com \
--cc=wathsala.vithanage@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.