When the value comparison fails in atomic_cmpxchg, the code branches
out without executing stxr, leaving the exclusive monitor in the
exclusive state set by ldxr.

Add `clrex` to the failure path to explicitly clear the exclusive
monitor.

Fixes: d2654a556835 ("xen: arm64: atomics")
Signed-off-by: Ryoji Okamoto <okamoto@valinux.co.jp>
---
v2: Use tabs for indentation instead of spaces

 xen/arch/arm/include/asm/arm64/atomic.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/arm64/atomic.h b/xen/arch/arm/include/asm/arm64/atomic.h
index 4460165295..3aa0ffb4d9 100644
--- a/xen/arch/arm/include/asm/arm64/atomic.h
+++ b/xen/arch/arm/include/asm/arm64/atomic.h
@@ -118,7 +118,9 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, int new)
 "	b.ne	2f\n"
 "	stxr	%w0, %w4, %2\n"
 "	cbnz	%w0, 1b\n"
-"2:"
+"	b	3f\n"
+"2: clrex\n"
+"3:"
 	: "=&r" (tmp), "=&r" (oldval), "+Q" (v->counter)
 	: "Ir" (old), "r" (new)
 	: "cc");
-- 
2.43.0

Hi Ryoji,

On 09/09/2026 08:12, Ryoji Okamoto wrote:
> When the value comparison fails in atomic_cmpxchg, the code branches
> out without executing stxr, leaving the exclusive monitor in the
> exclusive state set by ldxr.
> 
> Add `clrex` to the failure path to explicitly clear the exclusive
> monitor.

The Xen atomics operations were originally taken from Linux. Looking at 
the implementation there, I don't see a clrex on the failure path. Do 
you have more details why we would want it?

Also, if this is necessary on arm64, then we most likely we want the 
same for the arm32 implementation (including __atomic_add_unless()).
	

Cheers,

-- 
Julien Grall

Hi Julien,

  *
The Xen atomics operations were originally taken from Linux. Looking at the implementation there, I don't see a clrex on the failure path. Do you have more details why we would want it?

Thanks for pointing this out.
I revisited the rationale for adding CLREX here, and I don't think there is a sufficient reason for Xen to do so.
I found a rejected RFC^[1] in Linux in 2015 to add CLREX in atomic operations. In that RFC, Linux arm maintainers said there is no need to add CLREX.
I also found a patch^[2] that makes LLVM emit CLREX on this path, with the rationale that keeping the monitor set might have a negative performance impact on some microarchitectures. However, I have not been able to find concrete microarchitecture-specific evidence or measurements demonstrating such an impact.
Please consider this patch withdrawn.
Thanks for the review.
Regards,
Ryoji
[1] https://lore.kernel.org/linux-arm-kernel/1425016026-19766-1-git-send-email-bobby.prani@gmail.com/
[2] https://reviews.llvm.org/D13033

________________________________
From: Julien Grall <julien@xen.org>
Sent: Wednesday, September 9, 2026 16:10
To: 岡本 涼二 <okamoto@valinux.co.jp>; xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
Cc: Hirokazu Takahashi <taka@valinux.co.jp>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Michal Orzel <michal.orzel@amd.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v2] xen/arm64/atomic: Clear exclusive monitor on cmpxchg failure

Hi Ryoji,

On 09/09/2026 08:12, Ryoji Okamoto wrote:
> When the value comparison fails in atomic_cmpxchg, the code branches
> out without executing stxr, leaving the exclusive monitor in the
> exclusive state set by ldxr.
>
> Add `clrex` to the failure path to explicitly clear the exclusive
> monitor.

The Xen atomics operations were originally taken from Linux. Looking at
the implementation there, I don't see a clrex on the failure path. Do
you have more details why we would want it?

Also, if this is necessary on arm64, then we most likely we want the
same for the arm32 implementation (including __atomic_add_unless()).


Cheers,

--
Julien Grall

On 09.09.2026 08:12, Ryoji Okamoto wrote:
> When the value comparison fails in atomic_cmpxchg, the code branches
> out without executing stxr, leaving the exclusive monitor in the
> exclusive state set by ldxr.
> 
> Add `clrex` to the failure path to explicitly clear the exclusive
> monitor.
> 
> Fixes: d2654a556835 ("xen: arm64: atomics")
> Signed-off-by: Ryoji Okamoto <okamoto@valinux.co.jp>
> ---
> v2: Use tabs for indentation instead of spaces

Well, ...

> --- a/xen/arch/arm/include/asm/arm64/atomic.h
> +++ b/xen/arch/arm/include/asm/arm64/atomic.h
> @@ -118,7 +118,9 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, int new)
>  "	b.ne	2f\n"
>  "	stxr	%w0, %w4, %2\n"
>  "	cbnz	%w0, 1b\n"
> -"2:"
> +"	b	3f\n"

... you now do here, but ...

> +"2: clrex\n"

... still not here.

Jan

On 9/9/26 15:31, Jan Beulich wrote:
> On 09.09.2026 08:12, Ryoji Okamoto wrote:
>> When the value comparison fails in atomic_cmpxchg, the code branches
>> out without executing stxr, leaving the exclusive monitor in the
>> exclusive state set by ldxr.
>>
>> Add `clrex` to the failure path to explicitly clear the exclusive
>> monitor.
>>
>> Fixes: d2654a556835 ("xen: arm64: atomics")
>> Signed-off-by: Ryoji Okamoto <okamoto@valinux.co.jp>
>> ---
>> v2: Use tabs for indentation instead of spaces
> 
> Well, ...
> 
>> --- a/xen/arch/arm/include/asm/arm64/atomic.h
>> +++ b/xen/arch/arm/include/asm/arm64/atomic.h
>> @@ -118,7 +118,9 @@ static inline int atomic_cmpxchg(atomic_t *v, int old, int new)
>>   "	b.ne	2f\n"
>>   "	stxr	%w0, %w4, %2\n"
>>   "	cbnz	%w0, 1b\n"
>> -"2:"
>> +"	b	3f\n"
> 
> ... you now do here, but ...
> 
>> +"2: clrex\n"
> 
> ... still not here.
> 
> Jan

Hi Jan,

Thanks for the feedback on the indentation.
I'm withdrawing this patch, but I'll definitely keep this in mind for 
the next time.

Regards,

-- 
Ryoji