| Message ID | 20260916200215.21774-4-brads@mainlining.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add two-byte cmpxchg emulation and wire it into the architectures | expand |
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 \
On 17 September 2026 10:04:35 BST, David Laight <david.laight.linux@gmail.com> wrote: >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 > Makes sense. I'll just do a V3 adding tags and your fix, cause csky ? And the main patch is the one having issues in sashiko. > >> 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 \ > > --- Thanks! https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/
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; \ 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 \
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(+)