Re: [PATCH v2 3/5] csky: Emulate two-byte cmpxchg

From: David Laight <david.laight.linux@gmail.com>
To: Bradley Morgan <brads@mainlining.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vineet Gupta <vgupta@kernel.org>, Guo Ren <guoren@kernel.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Rich Felker <dalias@libc.org>, Chris Zankel <chris@zankel.net>,
	Max Filippov <jcmvbkbc@gmail.com>, Arnd Bergmann <arnd@arndb.de>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
	linux-snps-arc@lists.infradead.org, linux-csky@vger.kernel.org,
	linux-sh@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 3/5] csky: Emulate two-byte cmpxchg
Date: Thu, 17 Sep 2026 10:04:35 +0100	[thread overview]
Message-ID: <20260917100435.664cf00d@pumpkin> (raw)
In-Reply-To: <20260916200215.21774-4-brads@mainlining.org>

On Wed, 16 Sep 2026 20:02:13 +0000
Bradley Morgan <brads@mainlining.org> wrote:

> CSKY has no two-byte atomic compare and swap, so the __cmpxchg()
> switches in cmpxchg.h let case 2 fall through to the undefined
> __cmpxchg_called_with_bad_pointer(), failing at link time. Route case
> 2 through the new cmpxchg_emu_u16(), which now takes natural u16
> arguments, so the (uintptr_t) casts on __old and __new become (u16)
> casts in all three switch instances.
> 
> Signed-off-by: Bradley Morgan <brads@mainlining.org>
> ---
>  arch/csky/include/asm/cmpxchg.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/csky/include/asm/cmpxchg.h b/arch/csky/include/asm/cmpxchg.h
> index db6dda47184e..e53628268e8c 100644
> --- a/arch/csky/include/asm/cmpxchg.h
> +++ b/arch/csky/include/asm/cmpxchg.h
> @@ -65,6 +65,9 @@
>  	case 1:							\
>  		__ret = (__typeof__(*(ptr)))cmpxchg_emu_u8((volatile u8 *)__ptr, (uintptr_t)__old, (uintptr_t)__new); \
>  		break;						\
> +	case 2:							\
> +		__ret = (__typeof__(*(ptr)))cmpxchg_emu_u16((volatile u16 *)__ptr, (u16)__old, (u16)__new); \
> +		break;						\

Did you check that compiles?
It needs to compile without a warning when exchanging a pointer type.
I think it would be better to make both __old and __new 'unsigned long'
adding a cast in the definition/assignment to convert pointers.
aka:
	unsigned long __old = (unsigned long)(old);

(The cast on the call is separate from the type of the function parameters.)

Not a new bug, but there also doesn't seem to be a pointer-integer type
check here (there is probably one in another architecture).

I think that means these compile:
	int i;
	int *p;
	...
	cmpxchg(&p, 4, 5);
	cmpxchg(&i, p, 5);

Simplest fix is probably:
	unsigned long __old = (unsigned long)(0 ? *__ptr : (old));

David

>  	case 4:							\
>  		asm volatile (					\
>  		"1:	ldex.w		%0, (%3) \n"		\
> @@ -98,6 +101,9 @@
>  	case 1:							\
>  		__ret = (__typeof__(*(ptr)))cmpxchg_emu_u8((volatile u8 *)__ptr, (uintptr_t)__old, (uintptr_t)__new); \
>  		break;						\
> +	case 2:							\
> +		__ret = (__typeof__(*(ptr)))cmpxchg_emu_u16((volatile u16 *)__ptr, (u16)__old, (u16)__new); \
> +		break;						\
>  	case 4:							\
>  		asm volatile (					\
>  		"1:	ldex.w		%0, (%3) \n"		\
> @@ -132,6 +138,9 @@
>  	case 1:							\
>  		__ret = (__typeof__(*(ptr)))cmpxchg_emu_u8((volatile u8 *)__ptr, (uintptr_t)__old, (uintptr_t)__new); \
>  		break;						\
> +	case 2:							\
> +		__ret = (__typeof__(*(ptr)))cmpxchg_emu_u16((volatile u16 *)__ptr, (u16)__old, (u16)__new); \
> +		break;						\
>  	case 4:							\
>  		asm volatile (					\
>  		RELEASE_FENCE					\