Use the new cmpxchg_emu_u8() and cmpxchg_emu_u16() to emulate one-byte
and two-byte cmpxchg() on 32-bit sparc.
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andreas Larsson <andreas@gaisler.com>
Cc: Palmer Dabbelt <palmer@rivosinc.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Marco Elver <elver@google.com>
---
arch/sparc/Kconfig | 1 +
arch/sparc/include/asm/cmpxchg_32.h | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 11bf9d312318c..e6d1bbc4fedd0 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -55,6 +55,7 @@ config SPARC32
select ARCH_32BIT_OFF_T
select ARCH_HAS_CPU_FINALIZE_INIT if !SMP
select ARCH_HAS_SYNC_DMA_FOR_CPU
+ select ARCH_NEED_CMPXCHG_1_2_EMU
select CLZ_TAB
select DMA_DIRECT_REMAP
select GENERIC_ATOMIC64
diff --git a/arch/sparc/include/asm/cmpxchg_32.h b/arch/sparc/include/asm/cmpxchg_32.h
index d0af82c240b73..8eb483b5887a1 100644
--- a/arch/sparc/include/asm/cmpxchg_32.h
+++ b/arch/sparc/include/asm/cmpxchg_32.h
@@ -41,11 +41,17 @@ void __cmpxchg_called_with_bad_pointer(void);
/* we only need to support cmpxchg of a u32 on sparc */
unsigned long __cmpxchg_u32(volatile u32 *m, u32 old, u32 new_);
+#include <linux/cmpxchg-emu.h>
+
/* don't worry...optimizer will get rid of most of this */
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_);
+ case 2:
+ return cmpxchg_emu_u16((volatile u16 *)ptr, old, new_);
case 4:
return __cmpxchg_u32((u32 *)ptr, (u32)old, (u32)new_);
default:
--
2.40.1On Mon, Apr 01, 2024 at 02:39:44PM -0700, Paul E. McKenney wrote:
> Use the new cmpxchg_emu_u8() and cmpxchg_emu_u16() to emulate one-byte
> and two-byte cmpxchg() on 32-bit sparc.
> __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_);
> + case 2:
> + return cmpxchg_emu_u16((volatile u16 *)ptr, old, new_);
> case 4:
> return __cmpxchg_u32((u32 *)ptr, (u32)old, (u32)new_);
> default:
Considering how awful sparc32 32bit cmpxchg is, it might be better to
implement those directly rather than trying to do them on top of
that. Something like
#define CMPXCHG(T) \
T __cmpxchg_##T(volatile ##T *ptr, ##T old, ##T new) \
{ \
unsigned long flags; \
##T prev; \
\
spin_lock_irqsave(ATOMIC_HASH(ptr), flags); \
if ((prev = *ptr) == old) \
*ptr = new; \
spin_unlock_irqrestore(ATOMIC_HASH(ptr), flags);\
return prev; \
}
CMPXCHG(u8)
CMPXCHG(u16)
CMPXCHG(u32)
CMPXCHG(u64)
in arch/sparc/lib/atomic32.c, replacing equivalent __cmpxchg_u{32,64}()
definitions already there and use of those in that switch in __cmpxchg()
definition...
On Mon, Apr 01, 2024 at 11:38:03PM +0100, Al Viro wrote:
> On Mon, Apr 01, 2024 at 02:39:44PM -0700, Paul E. McKenney wrote:
> > Use the new cmpxchg_emu_u8() and cmpxchg_emu_u16() to emulate one-byte
> > and two-byte cmpxchg() on 32-bit sparc.
>
> > __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_);
> > + case 2:
> > + return cmpxchg_emu_u16((volatile u16 *)ptr, old, new_);
> > case 4:
> > return __cmpxchg_u32((u32 *)ptr, (u32)old, (u32)new_);
> > default:
>
> Considering how awful sparc32 32bit cmpxchg is, it might be better to
> implement those directly rather than trying to do them on top of
> that. Something like
>
> #define CMPXCHG(T) \
> T __cmpxchg_##T(volatile ##T *ptr, ##T old, ##T new) \
> { \
> unsigned long flags; \
> ##T prev; \
> \
> spin_lock_irqsave(ATOMIC_HASH(ptr), flags); \
> if ((prev = *ptr) == old) \
> *ptr = new; \
> spin_unlock_irqrestore(ATOMIC_HASH(ptr), flags);\
> return prev; \
> }
>
> CMPXCHG(u8)
> CMPXCHG(u16)
> CMPXCHG(u32)
> CMPXCHG(u64)
>
> in arch/sparc/lib/atomic32.c, replacing equivalent __cmpxchg_u{32,64}()
> definitions already there and use of those in that switch in __cmpxchg()
> definition...
Fair enough, and ATOMIC_HASH() is set up to permit mixed-size atomic
accesses courtesy of ignoring the bottom bits, though ignoring more
of them than absolutely necessary. Maybe 32-bit sparc has 32-byte
cache lines?
Would you like to do that patch? If so, I would be happy to drop mine
in favor of yours. If not, could I please have your Signed-off-by so
I can do the Co-developed-by dance?
Thanx, Paul
On Mon, Apr 01, 2024 at 04:58:03PM -0700, Paul E. McKenney wrote:
> > #define CMPXCHG(T) \
> > T __cmpxchg_##T(volatile ##T *ptr, ##T old, ##T new) \
^^^
*blink*
I understand what search-and-replace has produced that, but not
how I hadn't noticed the results... Sorry ;-/
> > { \
> > unsigned long flags; \
> > ##T prev; \
> > \
> > spin_lock_irqsave(ATOMIC_HASH(ptr), flags); \
> > if ((prev = *ptr) == old) \
> > *ptr = new; \
> > spin_unlock_irqrestore(ATOMIC_HASH(ptr), flags);\
> > return prev; \
> > }
> >
> > CMPXCHG(u8)
> > CMPXCHG(u16)
> > CMPXCHG(u32)
> > CMPXCHG(u64)
> >
> > in arch/sparc/lib/atomic32.c, replacing equivalent __cmpxchg_u{32,64}()
> > definitions already there and use of those in that switch in __cmpxchg()
> > definition...
>
> Fair enough, and ATOMIC_HASH() is set up to permit mixed-size atomic
> accesses courtesy of ignoring the bottom bits, though ignoring more
> of them than absolutely necessary. Maybe 32-bit sparc has 32-byte
> cache lines?
It does, IIRC.
> Would you like to do that patch? If so, I would be happy to drop mine
> in favor of yours. If not, could I please have your Signed-off-by so
> I can do the Co-developed-by dance?
Will do once I dig my way from under the pile of mail (sick for a week
and subscribed to l-k, among other lists)...
On Tue, Apr 02, 2024 at 01:07:58AM +0100, Al Viro wrote: > It does, IIRC. > > > Would you like to do that patch? If so, I would be happy to drop mine > > in favor of yours. If not, could I please have your Signed-off-by so > > I can do the Co-developed-by dance? > > Will do once I dig my way from under the pile of mail (sick for a week > and subscribed to l-k, among other lists)... FWIW, parisc is in the same situation - atomics-by-cached-spinlocks. 've a candidate branch, will post if it survives build... Re parisc: why does it bother with arch_cmpxchg_local()? Default is * save and disable local interrupts * read the current value, compare to old * if equal, store new there * restore local interrupts For 32bit case parisc goes for __cmpxchg_u32(), which is * if (SMP) choose the spinlock (indexed by hash of address) * save and disable local interrupes * if (SMP) arch_spin_lock(spinlock) * read the current value, compare to old * if equal, store new there * if (SMP) arch_spin_unlock(spinlock) * restore local interrupts In UP case it's identical to generic; on SMP it's strictly more work. Unless I'm very confused about cmpxchg_local() semantics, the callers do not expect atomicity wrt other CPUs, so why do we bother?
On Tue, Apr 02, 2024 at 04:37:53AM +0100, Al Viro wrote: > On Tue, Apr 02, 2024 at 01:07:58AM +0100, Al Viro wrote: > > > It does, IIRC. > > > > > Would you like to do that patch? If so, I would be happy to drop mine > > > in favor of yours. If not, could I please have your Signed-off-by so > > > I can do the Co-developed-by dance? > > > > Will do once I dig my way from under the pile of mail (sick for a week > > and subscribed to l-k, among other lists)... > > FWIW, parisc is in the same situation - atomics-by-cached-spinlocks. > 've a candidate branch, will post if it survives build... I am sure that it seemed like a good idea at the time. ;-) > Re parisc: why does it bother with arch_cmpxchg_local()? Default is > * save and disable local interrupts > * read the current value, compare to old > * if equal, store new there > * restore local interrupts > For 32bit case parisc goes for __cmpxchg_u32(), which is > * if (SMP) choose the spinlock (indexed by hash of address) > * save and disable local interrupes > * if (SMP) arch_spin_lock(spinlock) > * read the current value, compare to old > * if equal, store new there > * if (SMP) arch_spin_unlock(spinlock) > * restore local interrupts > In UP case it's identical to generic; on SMP it's strictly more work. > Unless I'm very confused about cmpxchg_local() semantics, the > callers do not expect atomicity wrt other CPUs, so why do we bother? ;-) ;-) ;-) In any case, happy to replace my patches with yours whenever you have them ready. Thanx, Paul
On Tue, Apr 02, 2024 at 04:37:53AM +0100, Al Viro wrote:
> On Tue, Apr 02, 2024 at 01:07:58AM +0100, Al Viro wrote:
>
> > It does, IIRC.
> >
> > > Would you like to do that patch? If so, I would be happy to drop mine
> > > in favor of yours. If not, could I please have your Signed-off-by so
> > > I can do the Co-developed-by dance?
> >
> > Will do once I dig my way from under the pile of mail (sick for a week
> > and subscribed to l-k, among other lists)...
>
> FWIW, parisc is in the same situation - atomics-by-cached-spinlocks.
> 've a candidate branch, will post if it survives build...
Seems to survive. See
git://git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git misc.cmpxchg
Completely untested; builds on several configs, but that's it.
Al Viro (8):
sparc32: make __cmpxchg_u32() return u32
sparc32: make the first argument of __cmpxchg_u64() volatile u64 *
sparc32: unify __cmpxchg_u{32,64}
sparc32: add __cmpxchg_u{8,16}() and teach __cmpxchg() to handle those sizes
parisc: __cmpxchg_u32(): lift conversion into the callers
parisc: unify implementations of __cmpxchg_u{8,32,64}
parisc: add missing export of __cmpxchg_u8()
parisc: add u16 support to cmpxchg()
arch/parisc/include/asm/cmpxchg.h | 16 ++++++------
arch/parisc/kernel/parisc_ksyms.c | 2 ++
arch/parisc/lib/bitops.c | 52 ++++++++++++-------------------------
arch/sparc/include/asm/cmpxchg_32.h | 11 +++++---
arch/sparc/lib/atomic32.c | 45 ++++++++++++++------------------
5 files changed, 55 insertions(+), 71 deletions(-)
Individual patches in followups.
On Tue, Apr 02, 2024 at 05:11:38AM +0100, Al Viro wrote:
> On Tue, Apr 02, 2024 at 04:37:53AM +0100, Al Viro wrote:
> > On Tue, Apr 02, 2024 at 01:07:58AM +0100, Al Viro wrote:
> >
> > > It does, IIRC.
> > >
> > > > Would you like to do that patch? If so, I would be happy to drop mine
> > > > in favor of yours. If not, could I please have your Signed-off-by so
> > > > I can do the Co-developed-by dance?
> > >
> > > Will do once I dig my way from under the pile of mail (sick for a week
> > > and subscribed to l-k, among other lists)...
> >
> > FWIW, parisc is in the same situation - atomics-by-cached-spinlocks.
> > 've a candidate branch, will post if it survives build...
>
> Seems to survive. See
> git://git.kernel.org:/pub/scm/linux/kernel/git/viro/vfs.git misc.cmpxchg
>
> Completely untested; builds on several configs, but that's it.
> Al Viro (8):
> sparc32: make __cmpxchg_u32() return u32
> sparc32: make the first argument of __cmpxchg_u64() volatile u64 *
> sparc32: unify __cmpxchg_u{32,64}
> sparc32: add __cmpxchg_u{8,16}() and teach __cmpxchg() to handle those sizes
> parisc: __cmpxchg_u32(): lift conversion into the callers
> parisc: unify implementations of __cmpxchg_u{8,32,64}
> parisc: add missing export of __cmpxchg_u8()
> parisc: add u16 support to cmpxchg()
>
> arch/parisc/include/asm/cmpxchg.h | 16 ++++++------
> arch/parisc/kernel/parisc_ksyms.c | 2 ++
> arch/parisc/lib/bitops.c | 52 ++++++++++++-------------------------
> arch/sparc/include/asm/cmpxchg_32.h | 11 +++++---
> arch/sparc/lib/atomic32.c | 45 ++++++++++++++------------------
> 5 files changed, 55 insertions(+), 71 deletions(-)
>
> Individual patches in followups.
Very good, thank you! I will take yours in place of mine on my next
rebase.
Thanx, Paul
© 2016 - 2026 Red Hat, Inc.