Re: [PATCH cmpxchg 2/3] ARC: Emulate one-byte cmpxchg

Paul E. McKenney
2024-08-06T13:57:39Z

On Mon, Aug 05, 2024 at 09:44:39PM -0700, Vineet Gupta wrote:
>
>
> On 8/5/24 21:28, Paul E. McKenney wrote:
> > On Mon, Aug 05, 2024 at 06:27:57PM -0700, Vineet Gupta wrote:
> >> Hi Paul,
> >>
> >> On 8/5/24 12:21, Paul E. McKenney wrote:
> >>> Use the new cmpxchg_emu_u8() to emulate one-byte cmpxchg() on arc.
> >>>
> >>> [ paulmck: Drop two-byte support per Arnd Bergmann feedback. ]
> >>> [ paulmck: Apply feedback from Naresh Kamboju. ]
> >>> [ paulmck: Apply kernel test robot feedback. ]
> >>>
> >>> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> >>> Cc: Vineet Gupta <vgupta@kernel.org>
> >>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
> >>> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> >>> Cc: Arnd Bergmann <arnd@arndb.de>
> >>> Cc: Palmer Dabbelt <palmer@rivosinc.com>
> >>> Cc: <linux-snps-arc@lists.infradead.org>
> >>> ---
> >>> arch/arc/Kconfig | 1 +
> >>> arch/arc/include/asm/cmpxchg.h | 33 ++++++++++++++++++++++++---------
> >>> 2 files changed, 25 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> >>> index fd0b0a0d4686a..163608fd49d18 100644
> >>> --- a/arch/arc/Kconfig
> >>> +++ b/arch/arc/Kconfig
> >>> @@ -13,6 +13,7 @@ config ARC
> >>> select ARCH_HAS_SETUP_DMA_OPS
> >>> select ARCH_HAS_SYNC_DMA_FOR_CPU
> >>> select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> >>> + select ARCH_NEED_CMPXCHG_1_EMU
> >>> select ARCH_SUPPORTS_ATOMIC_RMW if ARC_HAS_LLSC
> >>> select ARCH_32BIT_OFF_T
> >>> select BUILDTIME_TABLE_SORT
> >>> diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
> >>> index e138fde067dea..2102ce076f28b 100644
> >>> --- a/arch/arc/include/asm/cmpxchg.h
> >>> +++ b/arch/arc/include/asm/cmpxchg.h
> >>> @@ -8,6 +8,7 @@
> >>>
> >>> #include <linux/build_bug.h>
> >>> #include <linux/types.h>
> >>> +#include <linux/cmpxchg-emu.h>
> >>>
> >>> #include <asm/barrier.h>
> >>> #include <asm/smp.h>
> >>> @@ -46,6 +47,9 @@
> >>> __typeof__(*(ptr)) _prev_; \
> >>> \
> >>> switch(sizeof((_p_))) { \
> >>> + case 1: \
> >>> + _prev_ = (__typeof__(*(ptr)))cmpxchg_emu_u8((volatile u8 *)_p_, (uintptr_t)_o_, (uintptr_t)_n_); \
> >>> + break; \
> >>> case 4: \
> >>> _prev_ = __cmpxchg(_p_, _o_, _n_); \
> >>> break; \
> >>> @@ -65,16 +69,27 @@
> >>> __typeof__(*(ptr)) _prev_; \
> >>> unsigned long __flags; \
> >>> \
> >>> - BUILD_BUG_ON(sizeof(_p_) != 4); \
> >> Is this alone not sufficient: i.e. for !LLSC let the atomic op happen
> >> under a spin-lock for non 4 byte quantities as well.
> > Now that you mention it, that would be a lot simpler.
> >
> >>> + switch(sizeof((_p_))) { \
> >>> + case 1: \
> >>> + __flags = cmpxchg_emu_u8((volatile u8 *)_p_, (uintptr_t)_o_, (uintptr_t)_n_); \
> >>> + _prev_ = (__typeof__(*(ptr)))__flags; \
> >>> + break; \
> >>> + break; \
> >> FWIW, the 2nd break seems extraneous.
> > And to your earlier point, the first break as well. ;-)
> >
> > How does the updated patch below look? Or did I miss your point?
> >
> > Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > commit 96c1107797ca329fe203818cdfda2fe5f5a9a82e
> > Author: Paul E. McKenney <paulmck@kernel.org>
> > Date: Mon Mar 18 01:27:35 2024 -0700
> >
> > ARC: Emulate one-byte cmpxchg
> >
> > Use the new cmpxchg_emu_u8() to emulate one-byte cmpxchg() on arc.
> >
> > [ paulmck: Drop two-byte support per Arnd Bergmann feedback. ]
> > [ paulmck: Apply feedback from Naresh Kamboju. ]
> > [ paulmck: Apply kernel test robot feedback. ]
> > [ paulmck: Apply feedback from Vineet Gupta. ]
> >
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > Cc: Vineet Gupta <vgupta@kernel.org>
> > Cc: Andi Shyti <andi.shyti@linux.intel.com>
> > Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Palmer Dabbelt <palmer@rivosinc.com>
> > Cc: <linux-snps-arc@lists.infradead.org>
>
> Acked-by: Vineet Gupta <vgupta@kernel.org>

Thank you, and I will apply this on my next rebase.

Thanx, Paul

> Thx,
> -Vineet
>
> >
> > diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> > index fd0b0a0d4686a..163608fd49d18 100644
> > --- a/arch/arc/Kconfig
> > +++ b/arch/arc/Kconfig
> > @@ -13,6 +13,7 @@ config ARC
> > select ARCH_HAS_SETUP_DMA_OPS
> > select ARCH_HAS_SYNC_DMA_FOR_CPU
> > select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> > + select ARCH_NEED_CMPXCHG_1_EMU
> > select ARCH_SUPPORTS_ATOMIC_RMW if ARC_HAS_LLSC
> > select ARCH_32BIT_OFF_T
> > select BUILDTIME_TABLE_SORT
> > diff --git a/arch/arc/include/asm/cmpxchg.h b/arch/arc/include/asm/cmpxchg.h
> > index e138fde067dea..58045c8983404 100644
> > --- a/arch/arc/include/asm/cmpxchg.h
> > +++ b/arch/arc/include/asm/cmpxchg.h
> > @@ -8,6 +8,7 @@
> >
> > #include <linux/build_bug.h>
> > #include <linux/types.h>
> > +#include <linux/cmpxchg-emu.h>
> >
> > #include <asm/barrier.h>
> > #include <asm/smp.h>
> > @@ -46,6 +47,9 @@
> > __typeof__(*(ptr)) _prev_; \
> > \
> > switch(sizeof((_p_))) { \
> > + case 1: \
> > + _prev_ = (__typeof__(*(ptr)))cmpxchg_emu_u8((volatile u8 *)_p_, (uintptr_t)_o_, (uintptr_t)_n_); \
> > + break; \
> > case 4: \
> > _prev_ = __cmpxchg(_p_, _o_, _n_); \
> > break; \
> > @@ -65,8 +69,6 @@
> > __typeof__(*(ptr)) _prev_; \
> > unsigned long __flags; \
> > \
> > - BUILD_BUG_ON(sizeof(_p_) != 4); \
> > - \
> > /* \
> > * spin lock/unlock provide the needed smp_mb() before/after \
> > */ \