Re: [PATCH 2/5] ARC: Emulate two-byte cmpxchg

[David Laight <david.laight.linux@gmail.com>]

On Tue, 22 Sep 2026 17:33:51 +0000
Bradley Morgan <brads@mainlining.org> wrote:

> ARC has no two-byte atomic compare and swap, so the arch_cmpxchg_relaxed()
> macro switch lets case 2 fall through to BUILD_BUG() via default. Route case
> 2 through the new cmpxchg_emu_u16(), which takes the old and new
> values as unsigned long and narrows them itself, so the (uintptr_t)
> casts on _o_ and _n_ are no longer needed and the case 2 call passes
> them straight.
> 
> The switch also now tests sizeof(*(_p_)) instead of sizeof((_p_)),
> which switched on the pointer and made the size 1 and size 2 cases
> dead code, routing every sub-word cmpxchg() through the 32-bit
> llock/scond pair and comparing whole words against sub-word values,
> so the compare almost never succeeded.

Hmmm... that looks like it warrants a 'Fixes' tag.
Clearly no one uses 'arc'.

David

> The old and new values are
> now declared as unsigned long through (unsigned long)(0 ? *(_p_) :
> (old)), the idiom David Laight suggested, which type checks the
> arguments against the pointee, so cmpxchg(&p, 4, 5) no longer
> compiles silently.
> 
> Signed-off-by: Bradley Morgan <brads@mainlining.org>
> ---
>  arch/arc/include/asm/cmpxchg.h | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
> index 76f43db0890f..4596edfefa95 100644
> --- a/arch/arc/include/asm/cmpxchg.h
> +++ b/arch/arc/include/asm/cmpxchg.h
> @@ -42,16 +42,19 @@
>  #define arch_cmpxchg_relaxed(ptr, old, new)				\
>  ({									\
>  	__typeof__(ptr) _p_ = (ptr);					\
> -	__typeof__(*(ptr)) _o_ = (old);					\
> -	__typeof__(*(ptr)) _n_ = (new);					\
> +	unsigned long _old_ = (unsigned long)(0 ? *(_p_) : (old));		\
> +	unsigned long _new_ = (unsigned long)(0 ? *(_p_) : (new));		\
>  	__typeof__(*(ptr)) _prev_;					\
>  									\
> -	switch(sizeof((_p_))) {						\
> +	switch (sizeof(*(_p_))) {						\
>  	case 1:								\
> -		_prev_ = (__typeof__(*(ptr)))cmpxchg_emu_u8((volatile u8 *__force)_p_, (uintptr_t)_o_, (uintptr_t)_n_);	\
> +		_prev_ = (__typeof__(*(ptr)))cmpxchg_emu_u8((volatile u8 *__force)_p_, _old_, _new_);	\
> +		break;							\
> +	case 2:								\
> +		_prev_ = (__typeof__(*(ptr)))cmpxchg_emu_u16((volatile u16 *__force)_p_, _old_, _new_);	\
>  		break;							\
>  	case 4:								\
> -		_prev_ = __cmpxchg(_p_, _o_, _n_);			\
> +		_prev_ = (__typeof__(*(ptr)))__cmpxchg(_p_, _old_, _new_);			\
>  		break;							\
>  	default:							\
>  		BUILD_BUG();						\
> @@ -64,8 +67,8 @@
>  #define arch_cmpxchg(ptr, old, new)				        \
>  ({									\
>  	volatile __typeof__(ptr) _p_ = (ptr);				\
> -	__typeof__(*(ptr)) _o_ = (old);					\
> -	__typeof__(*(ptr)) _n_ = (new);					\
> +	unsigned long _old_ = (unsigned long)(0 ? *(_p_) : (old));		\
> +	unsigned long _new_ = (unsigned long)(0 ? *(_p_) : (new));		\
>  	__typeof__(*(ptr)) _prev_;					\
>  	unsigned long __flags;						\
>  									\
> @@ -102,7 +105,7 @@
>  	__typeof__(ptr) _p_ = (ptr);					\
>  	__typeof__(*(ptr)) _val_ = (val);				\
>  									\
> -	switch(sizeof(*(_p_))) {					\
> +	switch (sizeof(*(_p_))) {					\
>  	case 4:								\
>  		_val_ = __arch_xchg(_p_, _val_);			\
>  		break;							\