Re: [PATCH cmpxchg 13/14] xtensa: Emulate one-byte cmpxchg

[Yujie Liu]

On Thu, Apr 18, 2024 at 04:21:46PM -0700, Paul E. McKenney wrote:
> On Thu, Apr 18, 2024 at 10:06:21AM +0200, Geert Uytterhoeven wrote:
> > Hi Paul,
> > 
> > On Mon, Apr 8, 2024 at 7:49 PM Paul E. McKenney <paulmck@xxxxxxxxxx> wrote:
> > > Use the new cmpxchg_emu_u8() to emulate one-byte cmpxchg() on xtensa.
> > >
> > > [ paulmck: Apply kernel test robot feedback. ]
> > > [ paulmck: Drop two-byte support per Arnd Bergmann feedback. ]
> > >
> > > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > 
> > Thanks for your patch!
> > 
> > > --- a/arch/xtensa/include/asm/cmpxchg.h
> > > +++ b/arch/xtensa/include/asm/cmpxchg.h
> > > @@ -74,6 +75,7 @@ static __inline__ unsigned long
> > >  __cmpxchg(volatile void *ptr, unsigned long old, unsigned long new, int size)
> > >  {
> > >         switch (size) {
> > > +       case 1:  return cmpxchg_emu_u8((volatile u8 *)ptr, old, new);
> > 
> > The cast is not needed.
> 
> In both cases, kernel test robot yelled at me when it was not present.
> 
> Happy to resubmit without it, though, if that is a yell that I should
> have ignored.

FYI, kernel test robot did yell some reports on various architectures such as:

[1] https://lore.kernel.org/oe-kbuild-all/202403292321.T55etywH-lkp@xxxxxxxxx/
[2] https://lore.kernel.org/oe-kbuild-all/202404040526.GVzaL2io-lkp@xxxxxxxxx/
[3] https://lore.kernel.org/oe-kbuild-all/202404022106.mYwpypit-lkp@xxxxxxxxx/

In brief, there were mainly three types of issues:

* The cmpxchg-emu.h header is missing
* The parameters of cmpxchg_emu_u8 need to be cast to corresponding types
* The return value of cmpxchg_emu_u8 needs to be cast to the "ret" type

As for this specific case of xtensa arch, the compiler doesn't warn
regardless of whether there is an explicit cast for "ptr" or not.
The "ptr" being passed in is "void *", and it seems that a "void *"
pointer can be automatically cast to any other type of pointer, so it
is not necessary to have an explicit cast of "u8 *".

As for the implementations of other architectures that don't pass the
"ptr" as "void *" (such as a macro implementation), the explicit cast to
"u8 *" may still be required.

Thanks,
Yujie

> 
> 							Thanx, Paul
> 
> > >         case 4:  return __cmpxchg_u32(ptr, old, new);
> > >         default: __cmpxchg_called_with_bad_pointer();
> > >                  return old;
> > 
> > Gr{oetje,eeting}s,
> > 
> >                         Geert
> > 
> > -- 
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> > 
> > In personal conversations with technical people, I call myself a hacker. But
> > when I'm talking to journalists I just say "programmer" or something like that.
> >                                 -- Linus Torvalds