* [PATCH 0/3] powerpc: convert to use ARCH_ATOMIC
@ 2020-11-11 11:07 Nicholas Piggin
2020-11-11 11:07 ` [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC Nicholas Piggin
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Nicholas Piggin @ 2020-11-11 11:07 UTC (permalink / raw)
To: linuxppc-dev
Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Peter Zijlstra,
Boqun Feng, linux-kernel, Nicholas Piggin, Alexey Kardashevskiy,
Will Deacon
This conversion seems to require generic atomic64 changes, looks
like nothing else uses ARCH_ATOMIC and GENERIC_ATOMIC64 yet.
Thanks,
Nick
Nicholas Piggin (3):
asm-generic/atomic64: Add support for ARCH_ATOMIC
powerpc/64s/iommu: don't use atomic_ function on atomic64_t type
powerpc: rewrite atomics to use ARCH_ATOMIC
arch/powerpc/include/asm/atomic.h | 681 ++++++++++-----------------
arch/powerpc/include/asm/cmpxchg.h | 62 +--
arch/powerpc/mm/book3s64/iommu_api.c | 2 +-
include/asm-generic/atomic64.h | 70 ++-
lib/atomic64.c | 36 +-
5 files changed, 324 insertions(+), 527 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC 2020-11-11 11:07 [PATCH 0/3] powerpc: convert to use ARCH_ATOMIC Nicholas Piggin @ 2020-11-11 11:07 ` Nicholas Piggin 2020-11-11 13:39 ` Christophe Leroy 2020-11-11 11:07 ` [PATCH 2/3] powerpc/64s/iommu: don't use atomic_ function on atomic64_t type Nicholas Piggin ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Nicholas Piggin @ 2020-11-11 11:07 UTC (permalink / raw) To: linuxppc-dev Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Peter Zijlstra, Boqun Feng, linux-kernel, Nicholas Piggin, Alexey Kardashevskiy, Will Deacon This passes atomic64 selftest on ppc32 on qemu (uniprocessor only) both before and after powerpc is converted to use ARCH_ATOMIC. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- include/asm-generic/atomic64.h | 70 +++++++++++++++++++++++++++------- lib/atomic64.c | 36 ++++++++--------- 2 files changed, 75 insertions(+), 31 deletions(-) diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h index 370f01d4450f..2b1ecb591bb9 100644 --- a/include/asm-generic/atomic64.h +++ b/include/asm-generic/atomic64.h @@ -15,19 +15,17 @@ typedef struct { #define ATOMIC64_INIT(i) { (i) } -extern s64 atomic64_read(const atomic64_t *v); -extern void atomic64_set(atomic64_t *v, s64 i); - -#define atomic64_set_release(v, i) atomic64_set((v), (i)) +extern s64 __atomic64_read(const atomic64_t *v); +extern void __atomic64_set(atomic64_t *v, s64 i); #define ATOMIC64_OP(op) \ -extern void atomic64_##op(s64 a, atomic64_t *v); +extern void __atomic64_##op(s64 a, atomic64_t *v); #define ATOMIC64_OP_RETURN(op) \ -extern s64 atomic64_##op##_return(s64 a, atomic64_t *v); +extern s64 __atomic64_##op##_return(s64 a, atomic64_t *v); #define ATOMIC64_FETCH_OP(op) \ -extern s64 atomic64_fetch_##op(s64 a, atomic64_t *v); +extern s64 __atomic64_fetch_##op(s64 a, atomic64_t *v); #define ATOMIC64_OPS(op) ATOMIC64_OP(op) ATOMIC64_OP_RETURN(op) ATOMIC64_FETCH_OP(op) @@ -46,11 +44,57 @@ ATOMIC64_OPS(xor) #undef ATOMIC64_OP_RETURN #undef ATOMIC64_OP -extern s64 atomic64_dec_if_positive(atomic64_t *v); -#define atomic64_dec_if_positive atomic64_dec_if_positive -extern s64 atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n); -extern s64 atomic64_xchg(atomic64_t *v, s64 new); -extern s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u); -#define atomic64_fetch_add_unless atomic64_fetch_add_unless +extern s64 __atomic64_dec_if_positive(atomic64_t *v); +extern s64 __atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n); +extern s64 __atomic64_xchg(atomic64_t *v, s64 new); +extern s64 __atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u); + +#ifdef ARCH_ATOMIC +#define arch_atomic64_read __atomic64_read +#define arch_atomic64_set __atomic64_set +#define arch_atomic64_add __atomic64_add +#define arch_atomic64_add_return __atomic64_add_return +#define arch_atomic64_fetch_add __atomic64_fetch_add +#define arch_atomic64_sub __atomic64_sub +#define arch_atomic64_sub_return __atomic64_sub_return +#define arch_atomic64_fetch_sub __atomic64_fetch_sub +#define arch_atomic64_and __atomic64_and +#define arch_atomic64_and_return __atomic64_and_return +#define arch_atomic64_fetch_and __atomic64_fetch_and +#define arch_atomic64_or __atomic64_or +#define arch_atomic64_or_return __atomic64_or_return +#define arch_atomic64_fetch_or __atomic64_fetch_or +#define arch_atomic64_xor __atomic64_xor +#define arch_atomic64_xor_return __atomic64_xor_return +#define arch_atomic64_fetch_xor __atomic64_fetch_xor +#define arch_atomic64_xchg __atomic64_xchg +#define arch_atomic64_cmpxchg __atomic64_cmpxchg +#define arch_atomic64_set_release(v, i) __atomic64_set((v), (i)) +#define arch_atomic64_dec_if_positive __atomic64_dec_if_positive +#define arch_atomic64_fetch_add_unless __atomic64_fetch_add_unless +#else +#define atomic64_read __atomic64_read +#define atomic64_set __atomic64_set +#define atomic64_add __atomic64_add +#define atomic64_add_return __atomic64_add_return +#define atomic64_fetch_add __atomic64_fetch_add +#define atomic64_sub __atomic64_sub +#define atomic64_sub_return __atomic64_sub_return +#define atomic64_fetch_sub __atomic64_fetch_sub +#define atomic64_and __atomic64_and +#define atomic64_and_return __atomic64_and_return +#define atomic64_fetch_and __atomic64_fetch_and +#define atomic64_or __atomic64_or +#define atomic64_or_return __atomic64_or_return +#define atomic64_fetch_or __atomic64_fetch_or +#define atomic64_xor __atomic64_xor +#define atomic64_xor_return __atomic64_xor_return +#define atomic64_fetch_xor __atomic64_fetch_xor +#define atomic64_xchg __atomic64_xchg +#define atomic64_cmpxchg __atomic64_cmpxchg +#define atomic64_set_release(v, i) __atomic64_set((v), (i)) +#define atomic64_dec_if_positive __atomic64_dec_if_positive +#define atomic64_fetch_add_unless __atomic64_fetch_add_unless +#endif #endif /* _ASM_GENERIC_ATOMIC64_H */ diff --git a/lib/atomic64.c b/lib/atomic64.c index e98c85a99787..05aba5e3268f 100644 --- a/lib/atomic64.c +++ b/lib/atomic64.c @@ -42,7 +42,7 @@ static inline raw_spinlock_t *lock_addr(const atomic64_t *v) return &atomic64_lock[addr & (NR_LOCKS - 1)].lock; } -s64 atomic64_read(const atomic64_t *v) +s64 __atomic64_read(const atomic64_t *v) { unsigned long flags; raw_spinlock_t *lock = lock_addr(v); @@ -53,9 +53,9 @@ s64 atomic64_read(const atomic64_t *v) raw_spin_unlock_irqrestore(lock, flags); return val; } -EXPORT_SYMBOL(atomic64_read); +EXPORT_SYMBOL(__atomic64_read); -void atomic64_set(atomic64_t *v, s64 i) +void __atomic64_set(atomic64_t *v, s64 i) { unsigned long flags; raw_spinlock_t *lock = lock_addr(v); @@ -64,10 +64,10 @@ void atomic64_set(atomic64_t *v, s64 i) v->counter = i; raw_spin_unlock_irqrestore(lock, flags); } -EXPORT_SYMBOL(atomic64_set); +EXPORT_SYMBOL(__atomic64_set); #define ATOMIC64_OP(op, c_op) \ -void atomic64_##op(s64 a, atomic64_t *v) \ +void __atomic64_##op(s64 a, atomic64_t *v) \ { \ unsigned long flags; \ raw_spinlock_t *lock = lock_addr(v); \ @@ -76,10 +76,10 @@ void atomic64_##op(s64 a, atomic64_t *v) \ v->counter c_op a; \ raw_spin_unlock_irqrestore(lock, flags); \ } \ -EXPORT_SYMBOL(atomic64_##op); +EXPORT_SYMBOL(__atomic64_##op); #define ATOMIC64_OP_RETURN(op, c_op) \ -s64 atomic64_##op##_return(s64 a, atomic64_t *v) \ +s64 __atomic64_##op##_return(s64 a, atomic64_t *v) \ { \ unsigned long flags; \ raw_spinlock_t *lock = lock_addr(v); \ @@ -90,10 +90,10 @@ s64 atomic64_##op##_return(s64 a, atomic64_t *v) \ raw_spin_unlock_irqrestore(lock, flags); \ return val; \ } \ -EXPORT_SYMBOL(atomic64_##op##_return); +EXPORT_SYMBOL(__atomic64_##op##_return); #define ATOMIC64_FETCH_OP(op, c_op) \ -s64 atomic64_fetch_##op(s64 a, atomic64_t *v) \ +s64 __atomic64_fetch_##op(s64 a, atomic64_t *v) \ { \ unsigned long flags; \ raw_spinlock_t *lock = lock_addr(v); \ @@ -105,7 +105,7 @@ s64 atomic64_fetch_##op(s64 a, atomic64_t *v) \ raw_spin_unlock_irqrestore(lock, flags); \ return val; \ } \ -EXPORT_SYMBOL(atomic64_fetch_##op); +EXPORT_SYMBOL(__atomic64_fetch_##op); #define ATOMIC64_OPS(op, c_op) \ ATOMIC64_OP(op, c_op) \ @@ -130,7 +130,7 @@ ATOMIC64_OPS(xor, ^=) #undef ATOMIC64_OP_RETURN #undef ATOMIC64_OP -s64 atomic64_dec_if_positive(atomic64_t *v) +s64 __atomic64_dec_if_positive(atomic64_t *v) { unsigned long flags; raw_spinlock_t *lock = lock_addr(v); @@ -143,9 +143,9 @@ s64 atomic64_dec_if_positive(atomic64_t *v) raw_spin_unlock_irqrestore(lock, flags); return val; } -EXPORT_SYMBOL(atomic64_dec_if_positive); +EXPORT_SYMBOL(__atomic64_dec_if_positive); -s64 atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n) +s64 __atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n) { unsigned long flags; raw_spinlock_t *lock = lock_addr(v); @@ -158,9 +158,9 @@ s64 atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n) raw_spin_unlock_irqrestore(lock, flags); return val; } -EXPORT_SYMBOL(atomic64_cmpxchg); +EXPORT_SYMBOL(__atomic64_cmpxchg); -s64 atomic64_xchg(atomic64_t *v, s64 new) +s64 __atomic64_xchg(atomic64_t *v, s64 new) { unsigned long flags; raw_spinlock_t *lock = lock_addr(v); @@ -172,9 +172,9 @@ s64 atomic64_xchg(atomic64_t *v, s64 new) raw_spin_unlock_irqrestore(lock, flags); return val; } -EXPORT_SYMBOL(atomic64_xchg); +EXPORT_SYMBOL(__atomic64_xchg); -s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u) +s64 __atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u) { unsigned long flags; raw_spinlock_t *lock = lock_addr(v); @@ -188,4 +188,4 @@ s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u) return val; } -EXPORT_SYMBOL(atomic64_fetch_add_unless); +EXPORT_SYMBOL(__atomic64_fetch_add_unless); -- 2.23.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC 2020-11-11 11:07 ` [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC Nicholas Piggin @ 2020-11-11 13:39 ` Christophe Leroy 2020-11-11 13:44 ` Peter Zijlstra 0 siblings, 1 reply; 13+ messages in thread From: Christophe Leroy @ 2020-11-11 13:39 UTC (permalink / raw) To: Nicholas Piggin, linuxppc-dev Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Peter Zijlstra, Boqun Feng, linux-kernel, Alexey Kardashevskiy, Will Deacon Hello, Le 11/11/2020 à 12:07, Nicholas Piggin a écrit : > This passes atomic64 selftest on ppc32 on qemu (uniprocessor only) > both before and after powerpc is converted to use ARCH_ATOMIC. Can you explain what this change does and why it is needed ? Christophe > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > include/asm-generic/atomic64.h | 70 +++++++++++++++++++++++++++------- > lib/atomic64.c | 36 ++++++++--------- > 2 files changed, 75 insertions(+), 31 deletions(-) > > diff --git a/include/asm-generic/atomic64.h b/include/asm-generic/atomic64.h > index 370f01d4450f..2b1ecb591bb9 100644 > --- a/include/asm-generic/atomic64.h > +++ b/include/asm-generic/atomic64.h > @@ -15,19 +15,17 @@ typedef struct { > > #define ATOMIC64_INIT(i) { (i) } > > -extern s64 atomic64_read(const atomic64_t *v); > -extern void atomic64_set(atomic64_t *v, s64 i); > - > -#define atomic64_set_release(v, i) atomic64_set((v), (i)) > +extern s64 __atomic64_read(const atomic64_t *v); > +extern void __atomic64_set(atomic64_t *v, s64 i); > > #define ATOMIC64_OP(op) \ > -extern void atomic64_##op(s64 a, atomic64_t *v); > +extern void __atomic64_##op(s64 a, atomic64_t *v); > > #define ATOMIC64_OP_RETURN(op) \ > -extern s64 atomic64_##op##_return(s64 a, atomic64_t *v); > +extern s64 __atomic64_##op##_return(s64 a, atomic64_t *v); > > #define ATOMIC64_FETCH_OP(op) \ > -extern s64 atomic64_fetch_##op(s64 a, atomic64_t *v); > +extern s64 __atomic64_fetch_##op(s64 a, atomic64_t *v); > > #define ATOMIC64_OPS(op) ATOMIC64_OP(op) ATOMIC64_OP_RETURN(op) ATOMIC64_FETCH_OP(op) > > @@ -46,11 +44,57 @@ ATOMIC64_OPS(xor) > #undef ATOMIC64_OP_RETURN > #undef ATOMIC64_OP > > -extern s64 atomic64_dec_if_positive(atomic64_t *v); > -#define atomic64_dec_if_positive atomic64_dec_if_positive > -extern s64 atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n); > -extern s64 atomic64_xchg(atomic64_t *v, s64 new); > -extern s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u); > -#define atomic64_fetch_add_unless atomic64_fetch_add_unless > +extern s64 __atomic64_dec_if_positive(atomic64_t *v); > +extern s64 __atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n); > +extern s64 __atomic64_xchg(atomic64_t *v, s64 new); > +extern s64 __atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u); > + > +#ifdef ARCH_ATOMIC > +#define arch_atomic64_read __atomic64_read > +#define arch_atomic64_set __atomic64_set > +#define arch_atomic64_add __atomic64_add > +#define arch_atomic64_add_return __atomic64_add_return > +#define arch_atomic64_fetch_add __atomic64_fetch_add > +#define arch_atomic64_sub __atomic64_sub > +#define arch_atomic64_sub_return __atomic64_sub_return > +#define arch_atomic64_fetch_sub __atomic64_fetch_sub > +#define arch_atomic64_and __atomic64_and > +#define arch_atomic64_and_return __atomic64_and_return > +#define arch_atomic64_fetch_and __atomic64_fetch_and > +#define arch_atomic64_or __atomic64_or > +#define arch_atomic64_or_return __atomic64_or_return > +#define arch_atomic64_fetch_or __atomic64_fetch_or > +#define arch_atomic64_xor __atomic64_xor > +#define arch_atomic64_xor_return __atomic64_xor_return > +#define arch_atomic64_fetch_xor __atomic64_fetch_xor > +#define arch_atomic64_xchg __atomic64_xchg > +#define arch_atomic64_cmpxchg __atomic64_cmpxchg > +#define arch_atomic64_set_release(v, i) __atomic64_set((v), (i)) > +#define arch_atomic64_dec_if_positive __atomic64_dec_if_positive > +#define arch_atomic64_fetch_add_unless __atomic64_fetch_add_unless > +#else > +#define atomic64_read __atomic64_read > +#define atomic64_set __atomic64_set > +#define atomic64_add __atomic64_add > +#define atomic64_add_return __atomic64_add_return > +#define atomic64_fetch_add __atomic64_fetch_add > +#define atomic64_sub __atomic64_sub > +#define atomic64_sub_return __atomic64_sub_return > +#define atomic64_fetch_sub __atomic64_fetch_sub > +#define atomic64_and __atomic64_and > +#define atomic64_and_return __atomic64_and_return > +#define atomic64_fetch_and __atomic64_fetch_and > +#define atomic64_or __atomic64_or > +#define atomic64_or_return __atomic64_or_return > +#define atomic64_fetch_or __atomic64_fetch_or > +#define atomic64_xor __atomic64_xor > +#define atomic64_xor_return __atomic64_xor_return > +#define atomic64_fetch_xor __atomic64_fetch_xor > +#define atomic64_xchg __atomic64_xchg > +#define atomic64_cmpxchg __atomic64_cmpxchg > +#define atomic64_set_release(v, i) __atomic64_set((v), (i)) > +#define atomic64_dec_if_positive __atomic64_dec_if_positive > +#define atomic64_fetch_add_unless __atomic64_fetch_add_unless > +#endif > > #endif /* _ASM_GENERIC_ATOMIC64_H */ > diff --git a/lib/atomic64.c b/lib/atomic64.c > index e98c85a99787..05aba5e3268f 100644 > --- a/lib/atomic64.c > +++ b/lib/atomic64.c > @@ -42,7 +42,7 @@ static inline raw_spinlock_t *lock_addr(const atomic64_t *v) > return &atomic64_lock[addr & (NR_LOCKS - 1)].lock; > } > > -s64 atomic64_read(const atomic64_t *v) > +s64 __atomic64_read(const atomic64_t *v) > { > unsigned long flags; > raw_spinlock_t *lock = lock_addr(v); > @@ -53,9 +53,9 @@ s64 atomic64_read(const atomic64_t *v) > raw_spin_unlock_irqrestore(lock, flags); > return val; > } > -EXPORT_SYMBOL(atomic64_read); > +EXPORT_SYMBOL(__atomic64_read); > > -void atomic64_set(atomic64_t *v, s64 i) > +void __atomic64_set(atomic64_t *v, s64 i) > { > unsigned long flags; > raw_spinlock_t *lock = lock_addr(v); > @@ -64,10 +64,10 @@ void atomic64_set(atomic64_t *v, s64 i) > v->counter = i; > raw_spin_unlock_irqrestore(lock, flags); > } > -EXPORT_SYMBOL(atomic64_set); > +EXPORT_SYMBOL(__atomic64_set); > > #define ATOMIC64_OP(op, c_op) \ > -void atomic64_##op(s64 a, atomic64_t *v) \ > +void __atomic64_##op(s64 a, atomic64_t *v) \ > { \ > unsigned long flags; \ > raw_spinlock_t *lock = lock_addr(v); \ > @@ -76,10 +76,10 @@ void atomic64_##op(s64 a, atomic64_t *v) \ > v->counter c_op a; \ > raw_spin_unlock_irqrestore(lock, flags); \ > } \ > -EXPORT_SYMBOL(atomic64_##op); > +EXPORT_SYMBOL(__atomic64_##op); > > #define ATOMIC64_OP_RETURN(op, c_op) \ > -s64 atomic64_##op##_return(s64 a, atomic64_t *v) \ > +s64 __atomic64_##op##_return(s64 a, atomic64_t *v) \ > { \ > unsigned long flags; \ > raw_spinlock_t *lock = lock_addr(v); \ > @@ -90,10 +90,10 @@ s64 atomic64_##op##_return(s64 a, atomic64_t *v) \ > raw_spin_unlock_irqrestore(lock, flags); \ > return val; \ > } \ > -EXPORT_SYMBOL(atomic64_##op##_return); > +EXPORT_SYMBOL(__atomic64_##op##_return); > > #define ATOMIC64_FETCH_OP(op, c_op) \ > -s64 atomic64_fetch_##op(s64 a, atomic64_t *v) \ > +s64 __atomic64_fetch_##op(s64 a, atomic64_t *v) \ > { \ > unsigned long flags; \ > raw_spinlock_t *lock = lock_addr(v); \ > @@ -105,7 +105,7 @@ s64 atomic64_fetch_##op(s64 a, atomic64_t *v) \ > raw_spin_unlock_irqrestore(lock, flags); \ > return val; \ > } \ > -EXPORT_SYMBOL(atomic64_fetch_##op); > +EXPORT_SYMBOL(__atomic64_fetch_##op); > > #define ATOMIC64_OPS(op, c_op) \ > ATOMIC64_OP(op, c_op) \ > @@ -130,7 +130,7 @@ ATOMIC64_OPS(xor, ^=) > #undef ATOMIC64_OP_RETURN > #undef ATOMIC64_OP > > -s64 atomic64_dec_if_positive(atomic64_t *v) > +s64 __atomic64_dec_if_positive(atomic64_t *v) > { > unsigned long flags; > raw_spinlock_t *lock = lock_addr(v); > @@ -143,9 +143,9 @@ s64 atomic64_dec_if_positive(atomic64_t *v) > raw_spin_unlock_irqrestore(lock, flags); > return val; > } > -EXPORT_SYMBOL(atomic64_dec_if_positive); > +EXPORT_SYMBOL(__atomic64_dec_if_positive); > > -s64 atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n) > +s64 __atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n) > { > unsigned long flags; > raw_spinlock_t *lock = lock_addr(v); > @@ -158,9 +158,9 @@ s64 atomic64_cmpxchg(atomic64_t *v, s64 o, s64 n) > raw_spin_unlock_irqrestore(lock, flags); > return val; > } > -EXPORT_SYMBOL(atomic64_cmpxchg); > +EXPORT_SYMBOL(__atomic64_cmpxchg); > > -s64 atomic64_xchg(atomic64_t *v, s64 new) > +s64 __atomic64_xchg(atomic64_t *v, s64 new) > { > unsigned long flags; > raw_spinlock_t *lock = lock_addr(v); > @@ -172,9 +172,9 @@ s64 atomic64_xchg(atomic64_t *v, s64 new) > raw_spin_unlock_irqrestore(lock, flags); > return val; > } > -EXPORT_SYMBOL(atomic64_xchg); > +EXPORT_SYMBOL(__atomic64_xchg); > > -s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u) > +s64 __atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u) > { > unsigned long flags; > raw_spinlock_t *lock = lock_addr(v); > @@ -188,4 +188,4 @@ s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u) > > return val; > } > -EXPORT_SYMBOL(atomic64_fetch_add_unless); > +EXPORT_SYMBOL(__atomic64_fetch_add_unless); > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC 2020-11-11 13:39 ` Christophe Leroy @ 2020-11-11 13:44 ` Peter Zijlstra 2020-11-16 1:48 ` Nicholas Piggin 0 siblings, 1 reply; 13+ messages in thread From: Peter Zijlstra @ 2020-11-11 13:44 UTC (permalink / raw) To: Christophe Leroy Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Alexey Kardashevskiy, linuxppc-dev, Boqun Feng, linux-kernel, Nicholas Piggin, Will Deacon On Wed, Nov 11, 2020 at 02:39:01PM +0100, Christophe Leroy wrote: > Hello, > > Le 11/11/2020 à 12:07, Nicholas Piggin a écrit : > > This passes atomic64 selftest on ppc32 on qemu (uniprocessor only) > > both before and after powerpc is converted to use ARCH_ATOMIC. > > Can you explain what this change does and why it is needed ? That certainly should've been in the Changelog. This enables atomic instrumentation, see asm-generic/atomic-instrumented.h. IOW, it makes atomic ops visible to K*SAN. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC 2020-11-11 13:44 ` Peter Zijlstra @ 2020-11-16 1:48 ` Nicholas Piggin 0 siblings, 0 replies; 13+ messages in thread From: Nicholas Piggin @ 2020-11-16 1:48 UTC (permalink / raw) To: Christophe Leroy, Peter Zijlstra Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Alexey Kardashevskiy, Boqun Feng, linux-kernel, Will Deacon, linuxppc-dev Excerpts from Peter Zijlstra's message of November 11, 2020 11:44 pm: > On Wed, Nov 11, 2020 at 02:39:01PM +0100, Christophe Leroy wrote: >> Hello, >> >> Le 11/11/2020 à 12:07, Nicholas Piggin a écrit : >> > This passes atomic64 selftest on ppc32 on qemu (uniprocessor only) >> > both before and after powerpc is converted to use ARCH_ATOMIC. >> >> Can you explain what this change does and why it is needed ? > > That certainly should've been in the Changelog. This enables atomic > instrumentation, see asm-generic/atomic-instrumented.h. IOW, it makes > atomic ops visible to K*SAN. > Right. This specific patch doesn't actually "do" anything except allow generic atomic64 to be used with ARCH_ATOMIC. It does that by re-naming some things to avoid name collisions and also providing the arch_ prefix that ARCH_ATOMIC expects. I don't know what should be in the changelog. I suppose the latter, the former is discoverable by looking at ARCH_ATOMIC code and patches but I guess it's polite and doesn't hurt to include the former as well. I'll send an update before long. Thanks, Nick ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] powerpc/64s/iommu: don't use atomic_ function on atomic64_t type 2020-11-11 11:07 [PATCH 0/3] powerpc: convert to use ARCH_ATOMIC Nicholas Piggin 2020-11-11 11:07 ` [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC Nicholas Piggin @ 2020-11-11 11:07 ` Nicholas Piggin 2020-11-11 11:07 ` [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC Nicholas Piggin 2020-12-15 11:01 ` [PATCH 0/3] powerpc: convert " Michael Ellerman 3 siblings, 0 replies; 13+ messages in thread From: Nicholas Piggin @ 2020-11-11 11:07 UTC (permalink / raw) To: linuxppc-dev Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Peter Zijlstra, Boqun Feng, linux-kernel, Nicholas Piggin, Alexey Kardashevskiy, Will Deacon Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/mm/book3s64/iommu_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/mm/book3s64/iommu_api.c b/arch/powerpc/mm/book3s64/iommu_api.c index 563faa10bb66..685d7bb3d26f 100644 --- a/arch/powerpc/mm/book3s64/iommu_api.c +++ b/arch/powerpc/mm/book3s64/iommu_api.c @@ -263,7 +263,7 @@ long mm_iommu_put(struct mm_struct *mm, struct mm_iommu_table_group_mem_t *mem) goto unlock_exit; /* Are there still mappings? */ - if (atomic_cmpxchg(&mem->mapped, 1, 0) != 1) { + if (atomic64_cmpxchg(&mem->mapped, 1, 0) != 1) { ++mem->used; ret = -EBUSY; goto unlock_exit; -- 2.23.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC 2020-11-11 11:07 [PATCH 0/3] powerpc: convert to use ARCH_ATOMIC Nicholas Piggin 2020-11-11 11:07 ` [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC Nicholas Piggin 2020-11-11 11:07 ` [PATCH 2/3] powerpc/64s/iommu: don't use atomic_ function on atomic64_t type Nicholas Piggin @ 2020-11-11 11:07 ` Nicholas Piggin 2020-11-11 19:07 ` kernel test robot ` (2 more replies) 2020-12-15 11:01 ` [PATCH 0/3] powerpc: convert " Michael Ellerman 3 siblings, 3 replies; 13+ messages in thread From: Nicholas Piggin @ 2020-11-11 11:07 UTC (permalink / raw) To: linuxppc-dev Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Peter Zijlstra, Boqun Feng, linux-kernel, Nicholas Piggin, Alexey Kardashevskiy, Will Deacon All the cool kids are doing it. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/include/asm/atomic.h | 681 ++++++++++------------------- arch/powerpc/include/asm/cmpxchg.h | 62 +-- 2 files changed, 248 insertions(+), 495 deletions(-) diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h index 8a55eb8cc97b..899aa2403ba7 100644 --- a/arch/powerpc/include/asm/atomic.h +++ b/arch/powerpc/include/asm/atomic.h @@ -11,185 +11,285 @@ #include <asm/cmpxchg.h> #include <asm/barrier.h> +#define ARCH_ATOMIC + +#ifndef CONFIG_64BIT +#include <asm-generic/atomic64.h> +#endif + /* * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with * a "bne-" instruction at the end, so an isync is enough as a acquire barrier * on the platform without lwsync. */ #define __atomic_acquire_fence() \ - __asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory") + asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory") #define __atomic_release_fence() \ - __asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory") + asm volatile(PPC_RELEASE_BARRIER "" : : : "memory") -static __inline__ int atomic_read(const atomic_t *v) -{ - int t; +#define __atomic_pre_full_fence smp_mb - __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter)); +#define __atomic_post_full_fence smp_mb - return t; +#define arch_atomic_read(v) __READ_ONCE((v)->counter) +#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), (i)) +#ifdef CONFIG_64BIT +#define ATOMIC64_INIT(i) { (i) } +#define arch_atomic64_read(v) __READ_ONCE((v)->counter) +#define arch_atomic64_set(v, i) __WRITE_ONCE(((v)->counter), (i)) +#endif + +#define ATOMIC_OP(name, type, dtype, width, asm_op) \ +static inline void arch_##name(dtype a, type *v) \ +{ \ + dtype t; \ + \ + asm volatile( \ +"1: l" #width "arx %0,0,%3 # " #name "\n" \ +"\t" #asm_op " %0,%2,%0 \n" \ +" st" #width "cx. %0,0,%3 \n" \ +" bne- 1b \n" \ + : "=&r" (t), "+m" (v->counter) \ + : "r" (a), "r" (&v->counter) \ + : "cr0", "xer"); \ } -static __inline__ void atomic_set(atomic_t *v, int i) -{ - __asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i)); +#define ATOMIC_OP_IMM(name, type, dtype, width, asm_op, imm) \ +static inline void arch_##name(type *v) \ +{ \ + dtype t; \ + \ + asm volatile( \ +"1: l" #width "arx %0,0,%3 # " #name "\n" \ +"\t" #asm_op " %0,%0,%2 \n" \ +" st" #width "cx. %0,0,%3 \n" \ +" bne- 1b \n" \ + : "=&r" (t), "+m" (v->counter) \ + : "i" (imm), "r" (&v->counter) \ + : "cr0", "xer"); \ } -#define ATOMIC_OP(op, asm_op) \ -static __inline__ void atomic_##op(int a, atomic_t *v) \ +#define ATOMIC_OP_RETURN_RELAXED(name, type, dtype, width, asm_op) \ +static inline dtype arch_##name##_relaxed(dtype a, type *v) \ { \ - int t; \ + dtype t; \ \ - __asm__ __volatile__( \ -"1: lwarx %0,0,%3 # atomic_" #op "\n" \ - #asm_op " %0,%2,%0\n" \ -" stwcx. %0,0,%3 \n" \ -" bne- 1b\n" \ + asm volatile( \ +"1: l" #width "arx %0,0,%3 # " #name "\n" \ +"\t" #asm_op " %0,%2,%0 \n" \ +" st" #width "cx. %0,0,%3 \n" \ +" bne- 1b \n" \ : "=&r" (t), "+m" (v->counter) \ : "r" (a), "r" (&v->counter) \ - : "cc"); \ -} \ + : "cr0", "xer"); \ + \ + return t; \ +} -#define ATOMIC_OP_RETURN_RELAXED(op, asm_op) \ -static inline int atomic_##op##_return_relaxed(int a, atomic_t *v) \ +#define ATOMIC_OP_IMM_RETURN_RELAXED(name, type, dtype, width, asm_op, imm) \ +static inline dtype arch_##name##_relaxed(type *v) \ { \ - int t; \ + dtype t; \ \ - __asm__ __volatile__( \ -"1: lwarx %0,0,%3 # atomic_" #op "_return_relaxed\n" \ - #asm_op " %0,%2,%0\n" \ -" stwcx. %0,0,%3\n" \ -" bne- 1b\n" \ + asm volatile( \ +"1: l" #width "arx %0,0,%3 # " #name "\n" \ +"\t" #asm_op " %0,%0,%2 \n" \ +" st" #width "cx. %0,0,%3 \n" \ +" bne- 1b \n" \ : "=&r" (t), "+m" (v->counter) \ - : "r" (a), "r" (&v->counter) \ - : "cc"); \ + : "i" (imm), "r" (&v->counter) \ + : "cr0", "xer"); \ \ return t; \ } -#define ATOMIC_FETCH_OP_RELAXED(op, asm_op) \ -static inline int atomic_fetch_##op##_relaxed(int a, atomic_t *v) \ +#define ATOMIC_FETCH_OP_RELAXED(name, type, dtype, width, asm_op) \ +static inline dtype arch_##name##_relaxed(dtype a, type *v) \ { \ - int res, t; \ + dtype res, t; \ \ - __asm__ __volatile__( \ -"1: lwarx %0,0,%4 # atomic_fetch_" #op "_relaxed\n" \ - #asm_op " %1,%3,%0\n" \ -" stwcx. %1,0,%4\n" \ -" bne- 1b\n" \ + asm volatile( \ +"1: l" #width "arx %0,0,%4 # " #name "\n" \ +"\t" #asm_op " %1,%3,%0 \n" \ +" st" #width "cx. %1,0,%4 \n" \ +" bne- 1b \n" \ : "=&r" (res), "=&r" (t), "+m" (v->counter) \ : "r" (a), "r" (&v->counter) \ - : "cc"); \ + : "cr0", "xer"); \ \ return res; \ } +#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \ +static inline int arch_##name##_relaxed(type *v, dtype a, dtype u) \ +{ \ + dtype res, t; \ + \ + asm volatile( \ +"1: l" #width "arx %0,0,%5 # " #name "\n" \ +" cmp" #width " 0,%0,%3 \n" \ +" beq- 2f \n" \ +"\t" #asm_op " %1,%2,%0 \n" \ +" st" #width "cx. %1,0,%5 \n" \ +" bne- 1b \n" \ +"2: \n" \ + : "=&r" (res), "=&r" (t), "+m" (v->counter) \ + : "r" (a), "r" (u), "r" (&v->counter) \ + : "cr0", "xer"); \ + \ + return res; \ +} + +#define ATOMIC_INC_NOT_ZERO_RELAXED(name, type, dtype, width) \ +static inline dtype arch_##name##_relaxed(type *v) \ +{ \ + dtype t1, t2; \ + \ + asm volatile( \ +"1: l" #width "arx %0,0,%3 # " #name "\n" \ +" cmp" #width "i 0,%0,0 \n" \ +" beq- 2f \n" \ +" addic %1,%2,1 \n" \ +" st" #width "cx. %1,0,%3 \n" \ +" bne- 1b \n" \ +"2: \n" \ + : "=&r" (t1), "=&r" (t2), "+m" (v->counter) \ + : "r" (&v->counter) \ + : "cr0", "xer"); \ + \ + return t1; \ +} + +#undef ATOMIC_OPS #define ATOMIC_OPS(op, asm_op) \ - ATOMIC_OP(op, asm_op) \ - ATOMIC_OP_RETURN_RELAXED(op, asm_op) \ - ATOMIC_FETCH_OP_RELAXED(op, asm_op) +ATOMIC_OP(atomic_##op, atomic_t, int, w, asm_op) \ +ATOMIC_OP_RETURN_RELAXED(atomic_##op##_return, atomic_t, int, w, asm_op) \ +ATOMIC_FETCH_OP_RELAXED(atomic_fetch_##op, atomic_t, int, w, asm_op) \ +ATOMIC_FETCH_OP_UNLESS_RELAXED(atomic_fetch_##op##_unless, atomic_t, int, w, asm_op) + +#undef ATOMIC64_OPS +#define ATOMIC64_OPS(op, asm_op) \ +ATOMIC_OP(atomic64_##op, atomic64_t, u64, d, asm_op) \ +ATOMIC_OP_RETURN_RELAXED(atomic64_##op##_return, atomic64_t, u64, d, asm_op) \ +ATOMIC_FETCH_OP_RELAXED(atomic64_fetch_##op, atomic64_t, u64, d, asm_op) \ +ATOMIC_FETCH_OP_UNLESS_RELAXED(atomic64_fetch_##op##_unless, atomic64_t, u64, d, asm_op) ATOMIC_OPS(add, add) +#define arch_atomic_add arch_atomic_add +#define arch_atomic_add_return_relaxed arch_atomic_add_return_relaxed +#define arch_atomic_fetch_add_relaxed arch_atomic_fetch_add_relaxed +#define arch_atomic_fetch_add_unless_relaxed arch_atomic_fetch_add_unless_relaxed + ATOMIC_OPS(sub, subf) +#define arch_atomic_sub arch_atomic_sub +#define arch_atomic_sub_return_relaxed arch_atomic_sub_return_relaxed +#define arch_atomic_fetch_sub_relaxed arch_atomic_fetch_sub_relaxed +/* skip atomic_fetch_sub_unless_relaxed */ -#define atomic_add_return_relaxed atomic_add_return_relaxed -#define atomic_sub_return_relaxed atomic_sub_return_relaxed +#ifdef CONFIG_64BIT +ATOMIC64_OPS(add, add) +#define arch_atomic64_add arch_atomic64_add +#define arch_atomic64_add_return_relaxed arch_atomic64_add_return_relaxed +#define arch_atomic64_fetch_add_relaxed arch_atomic64_fetch_add_relaxed +#define arch_atomic64_fetch_add_unless_relaxed arch_atomic64_fetch_add_unless_relaxed -#define atomic_fetch_add_relaxed atomic_fetch_add_relaxed -#define atomic_fetch_sub_relaxed atomic_fetch_sub_relaxed +ATOMIC64_OPS(sub, subf) +#define arch_atomic64_sub arch_atomic64_sub +#define arch_atomic64_sub_return_relaxed arch_atomic64_sub_return_relaxed +#define arch_atomic64_fetch_sub_relaxed arch_atomic64_fetch_sub_relaxed +/* skip atomic64_fetch_sub_unless_relaxed */ +#endif #undef ATOMIC_OPS #define ATOMIC_OPS(op, asm_op) \ - ATOMIC_OP(op, asm_op) \ - ATOMIC_FETCH_OP_RELAXED(op, asm_op) +ATOMIC_OP(atomic_##op, atomic_t, int, w, asm_op) \ +ATOMIC_FETCH_OP_RELAXED(atomic_fetch_##op, atomic_t, int, w, asm_op) + +#undef ATOMIC64_OPS +#define ATOMIC64_OPS(op, asm_op) \ +ATOMIC_OP(atomic64_##op, atomic64_t, u64, d, asm_op) \ +ATOMIC_FETCH_OP_RELAXED(atomic64_fetch_##op, atomic64_t, u64, d, asm_op) ATOMIC_OPS(and, and) +#define arch_atomic_and arch_atomic_and +#define arch_atomic_fetch_and_relaxed arch_atomic_fetch_and_relaxed + ATOMIC_OPS(or, or) +#define arch_atomic_or arch_atomic_or +#define arch_atomic_fetch_or_relaxed arch_atomic_fetch_or_relaxed + ATOMIC_OPS(xor, xor) +#define arch_atomic_xor arch_atomic_xor +#define arch_atomic_fetch_xor_relaxed arch_atomic_fetch_xor_relaxed + +#ifdef CONFIG_64BIT +ATOMIC64_OPS(and, and) +#define arch_atomic64_and arch_atomic64_and +#define arch_atomic64_fetch_and_relaxed arch_atomic64_fetch_and_relaxed -#define atomic_fetch_and_relaxed atomic_fetch_and_relaxed -#define atomic_fetch_or_relaxed atomic_fetch_or_relaxed -#define atomic_fetch_xor_relaxed atomic_fetch_xor_relaxed +ATOMIC64_OPS(or, or) +#define arch_atomic64_or arch_atomic64_or +#define arch_atomic64_fetch_or_relaxed arch_atomic64_fetch_or_relaxed + +ATOMIC64_OPS(xor, xor) +#define arch_atomic64_xor arch_atomic64_xor +#define arch_atomic64_fetch_xor_relaxed arch_atomic64_fetch_xor_relaxed +#endif #undef ATOMIC_OPS +#define ATOMIC_OPS(op, asm_op, imm) \ +ATOMIC_OP_IMM(atomic_##op, atomic_t, int, w, asm_op, imm) \ +ATOMIC_OP_IMM_RETURN_RELAXED(atomic_##op##_return, atomic_t, int, w, asm_op, imm) + +#undef ATOMIC64_OPS +#define ATOMIC64_OPS(op, asm_op, imm) \ +ATOMIC_OP_IMM(atomic64_##op, atomic64_t, u64, d, asm_op, imm) \ +ATOMIC_OP_IMM_RETURN_RELAXED(atomic64_##op##_return, atomic64_t, u64, d, asm_op, imm) + +ATOMIC_OPS(inc, addic, 1) +#define arch_atomic_inc arch_atomic_inc +#define arch_atomic_inc_return_relaxed arch_atomic_inc_return_relaxed + +ATOMIC_OPS(dec, addic, -1) +#define arch_atomic_dec arch_atomic_dec +#define arch_atomic_dec_return_relaxed arch_atomic_dec_return_relaxed + +#ifdef CONFIG_64BIT +ATOMIC64_OPS(inc, addic, 1) +#define arch_atomic64_inc arch_atomic64_inc +#define arch_atomic64_inc_return_relaxed arch_atomic64_inc_return_relaxed + +ATOMIC64_OPS(dec, addic, -1) +#define arch_atomic64_dec arch_atomic64_dec +#define arch_atomic64_dec_return_relaxed arch_atomic64_dec_return_relaxed +#endif + +ATOMIC_INC_NOT_ZERO_RELAXED(atomic_inc_not_zero, atomic_t, int, w) +#define arch_atomic_inc_not_zero_relaxed(v) arch_atomic_inc_not_zero_relaxed(v) + +#ifdef CONFIG_64BIT +ATOMIC_INC_NOT_ZERO_RELAXED(atomic64_inc_not_zero, atomic64_t, u64, d) +#define arch_atomic64_inc_not_zero_relaxed(v) arch_atomic64_inc_not_zero_relaxed(v) +#endif + +#undef ATOMIC_INC_NOT_ZERO_RELAXED +#undef ATOMIC_FETCH_OP_UNLESS_RELAXED #undef ATOMIC_FETCH_OP_RELAXED +#undef ATOMIC_OP_IMM_RETURN_RELAXED #undef ATOMIC_OP_RETURN_RELAXED +#undef ATOMIC_OP_IMM #undef ATOMIC_OP +#undef ATOMIC_OPS +#undef ATOMIC64_OPS -static __inline__ void atomic_inc(atomic_t *v) -{ - int t; - - __asm__ __volatile__( -"1: lwarx %0,0,%2 # atomic_inc\n\ - addic %0,%0,1\n" -" stwcx. %0,0,%2 \n\ - bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); -} -#define atomic_inc atomic_inc - -static __inline__ int atomic_inc_return_relaxed(atomic_t *v) -{ - int t; - - __asm__ __volatile__( -"1: lwarx %0,0,%2 # atomic_inc_return_relaxed\n" -" addic %0,%0,1\n" -" stwcx. %0,0,%2\n" -" bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); - - return t; -} - -static __inline__ void atomic_dec(atomic_t *v) -{ - int t; - - __asm__ __volatile__( -"1: lwarx %0,0,%2 # atomic_dec\n\ - addic %0,%0,-1\n" -" stwcx. %0,0,%2\n\ - bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); -} -#define atomic_dec atomic_dec - -static __inline__ int atomic_dec_return_relaxed(atomic_t *v) -{ - int t; - - __asm__ __volatile__( -"1: lwarx %0,0,%2 # atomic_dec_return_relaxed\n" -" addic %0,%0,-1\n" -" stwcx. %0,0,%2\n" -" bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); - - return t; -} - -#define atomic_inc_return_relaxed atomic_inc_return_relaxed -#define atomic_dec_return_relaxed atomic_dec_return_relaxed - -#define atomic_cmpxchg(v, o, n) (cmpxchg(&((v)->counter), (o), (n))) -#define atomic_cmpxchg_relaxed(v, o, n) \ - cmpxchg_relaxed(&((v)->counter), (o), (n)) -#define atomic_cmpxchg_acquire(v, o, n) \ - cmpxchg_acquire(&((v)->counter), (o), (n)) +#define arch_atomic_cmpxchg_relaxed(v, o, n) arch_cmpxchg_relaxed(&((v)->counter), (o), (n)) +#define arch_atomic_xchg_relaxed(v, new) arch_xchg_relaxed(&((v)->counter), (new)) -#define atomic_xchg(v, new) (xchg(&((v)->counter), new)) -#define atomic_xchg_relaxed(v, new) xchg_relaxed(&((v)->counter), (new)) +#ifdef CONFIG_64BIT +#define arch_atomic64_cmpxchg_relaxed(v, o, n) arch_cmpxchg_relaxed(&((v)->counter), (o), (n)) +#define arch_atomic64_xchg_relaxed(v, new) arch_xchg_relaxed(&((v)->counter), (new)) +#endif /* * Don't want to override the generic atomic_try_cmpxchg_acquire, because @@ -203,7 +303,7 @@ atomic_try_cmpxchg_lock(atomic_t *v, int *old, int new) int r, o = *old; __asm__ __volatile__ ( -"1:\t" PPC_LWARX(%0,0,%2,1) " # atomic_try_cmpxchg_acquire \n" +"1:\t" PPC_LWARX(%0,0,%2,1) " # atomic_try_cmpxchg_lock \n" " cmpw 0,%0,%3 \n" " bne- 2f \n" " stwcx. %4,0,%2 \n" @@ -219,270 +319,41 @@ atomic_try_cmpxchg_lock(atomic_t *v, int *old, int new) return likely(r == o); } -/** - * atomic_fetch_add_unless - add unless the number is a given value - * @v: pointer of type atomic_t - * @a: the amount to add to v... - * @u: ...unless v is equal to u. - * - * Atomically adds @a to @v, so long as it was not @u. - * Returns the old value of @v. - */ -static __inline__ int atomic_fetch_add_unless(atomic_t *v, int a, int u) -{ - int t; - - __asm__ __volatile__ ( - PPC_ATOMIC_ENTRY_BARRIER -"1: lwarx %0,0,%1 # atomic_fetch_add_unless\n\ - cmpw 0,%0,%3 \n\ - beq 2f \n\ - add %0,%2,%0 \n" -" stwcx. %0,0,%1 \n\ - bne- 1b \n" - PPC_ATOMIC_EXIT_BARRIER -" subf %0,%2,%0 \n\ -2:" - : "=&r" (t) - : "r" (&v->counter), "r" (a), "r" (u) - : "cc", "memory"); - - return t; -} -#define atomic_fetch_add_unless atomic_fetch_add_unless - -/** - * atomic_inc_not_zero - increment unless the number is zero - * @v: pointer of type atomic_t - * - * Atomically increments @v by 1, so long as @v is non-zero. - * Returns non-zero if @v was non-zero, and zero otherwise. - */ -static __inline__ int atomic_inc_not_zero(atomic_t *v) -{ - int t1, t2; - - __asm__ __volatile__ ( - PPC_ATOMIC_ENTRY_BARRIER -"1: lwarx %0,0,%2 # atomic_inc_not_zero\n\ - cmpwi 0,%0,0\n\ - beq- 2f\n\ - addic %1,%0,1\n" -" stwcx. %1,0,%2\n\ - bne- 1b\n" - PPC_ATOMIC_EXIT_BARRIER - "\n\ -2:" - : "=&r" (t1), "=&r" (t2) - : "r" (&v->counter) - : "cc", "xer", "memory"); - - return t1; -} -#define atomic_inc_not_zero(v) atomic_inc_not_zero((v)) - /* * Atomically test *v and decrement if it is greater than 0. * The function returns the old value of *v minus 1, even if * the atomic variable, v, was not decremented. */ -static __inline__ int atomic_dec_if_positive(atomic_t *v) +static inline int atomic_dec_if_positive_relaxed(atomic_t *v) { int t; - __asm__ __volatile__( - PPC_ATOMIC_ENTRY_BARRIER -"1: lwarx %0,0,%1 # atomic_dec_if_positive\n\ - cmpwi %0,1\n\ - addi %0,%0,-1\n\ - blt- 2f\n" -" stwcx. %0,0,%1\n\ - bne- 1b" - PPC_ATOMIC_EXIT_BARRIER - "\n\ -2:" : "=&b" (t) + asm volatile( +"1: lwarx %0,0,%1 # atomic_dec_if_positive \n" +" cmpwi %0,1 \n" +" addi %0,%0,-1 \n" +" blt- 2f \n" +" stwcx. %0,0,%1 \n" +" bne- 1b \n" +"2: \n" + : "=&b" (t) : "r" (&v->counter) : "cc", "memory"); return t; } -#define atomic_dec_if_positive atomic_dec_if_positive - -#ifdef __powerpc64__ - -#define ATOMIC64_INIT(i) { (i) } - -static __inline__ s64 atomic64_read(const atomic64_t *v) -{ - s64 t; - - __asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter)); - - return t; -} - -static __inline__ void atomic64_set(atomic64_t *v, s64 i) -{ - __asm__ __volatile__("std%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i)); -} - -#define ATOMIC64_OP(op, asm_op) \ -static __inline__ void atomic64_##op(s64 a, atomic64_t *v) \ -{ \ - s64 t; \ - \ - __asm__ __volatile__( \ -"1: ldarx %0,0,%3 # atomic64_" #op "\n" \ - #asm_op " %0,%2,%0\n" \ -" stdcx. %0,0,%3 \n" \ -" bne- 1b\n" \ - : "=&r" (t), "+m" (v->counter) \ - : "r" (a), "r" (&v->counter) \ - : "cc"); \ -} - -#define ATOMIC64_OP_RETURN_RELAXED(op, asm_op) \ -static inline s64 \ -atomic64_##op##_return_relaxed(s64 a, atomic64_t *v) \ -{ \ - s64 t; \ - \ - __asm__ __volatile__( \ -"1: ldarx %0,0,%3 # atomic64_" #op "_return_relaxed\n" \ - #asm_op " %0,%2,%0\n" \ -" stdcx. %0,0,%3\n" \ -" bne- 1b\n" \ - : "=&r" (t), "+m" (v->counter) \ - : "r" (a), "r" (&v->counter) \ - : "cc"); \ - \ - return t; \ -} - -#define ATOMIC64_FETCH_OP_RELAXED(op, asm_op) \ -static inline s64 \ -atomic64_fetch_##op##_relaxed(s64 a, atomic64_t *v) \ -{ \ - s64 res, t; \ - \ - __asm__ __volatile__( \ -"1: ldarx %0,0,%4 # atomic64_fetch_" #op "_relaxed\n" \ - #asm_op " %1,%3,%0\n" \ -" stdcx. %1,0,%4\n" \ -" bne- 1b\n" \ - : "=&r" (res), "=&r" (t), "+m" (v->counter) \ - : "r" (a), "r" (&v->counter) \ - : "cc"); \ - \ - return res; \ -} - -#define ATOMIC64_OPS(op, asm_op) \ - ATOMIC64_OP(op, asm_op) \ - ATOMIC64_OP_RETURN_RELAXED(op, asm_op) \ - ATOMIC64_FETCH_OP_RELAXED(op, asm_op) - -ATOMIC64_OPS(add, add) -ATOMIC64_OPS(sub, subf) - -#define atomic64_add_return_relaxed atomic64_add_return_relaxed -#define atomic64_sub_return_relaxed atomic64_sub_return_relaxed - -#define atomic64_fetch_add_relaxed atomic64_fetch_add_relaxed -#define atomic64_fetch_sub_relaxed atomic64_fetch_sub_relaxed - -#undef ATOMIC64_OPS -#define ATOMIC64_OPS(op, asm_op) \ - ATOMIC64_OP(op, asm_op) \ - ATOMIC64_FETCH_OP_RELAXED(op, asm_op) - -ATOMIC64_OPS(and, and) -ATOMIC64_OPS(or, or) -ATOMIC64_OPS(xor, xor) - -#define atomic64_fetch_and_relaxed atomic64_fetch_and_relaxed -#define atomic64_fetch_or_relaxed atomic64_fetch_or_relaxed -#define atomic64_fetch_xor_relaxed atomic64_fetch_xor_relaxed - -#undef ATOPIC64_OPS -#undef ATOMIC64_FETCH_OP_RELAXED -#undef ATOMIC64_OP_RETURN_RELAXED -#undef ATOMIC64_OP - -static __inline__ void atomic64_inc(atomic64_t *v) -{ - s64 t; - - __asm__ __volatile__( -"1: ldarx %0,0,%2 # atomic64_inc\n\ - addic %0,%0,1\n\ - stdcx. %0,0,%2 \n\ - bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); -} -#define atomic64_inc atomic64_inc - -static __inline__ s64 atomic64_inc_return_relaxed(atomic64_t *v) -{ - s64 t; - - __asm__ __volatile__( -"1: ldarx %0,0,%2 # atomic64_inc_return_relaxed\n" -" addic %0,%0,1\n" -" stdcx. %0,0,%2\n" -" bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); - - return t; -} - -static __inline__ void atomic64_dec(atomic64_t *v) -{ - s64 t; - - __asm__ __volatile__( -"1: ldarx %0,0,%2 # atomic64_dec\n\ - addic %0,%0,-1\n\ - stdcx. %0,0,%2\n\ - bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); -} -#define atomic64_dec atomic64_dec - -static __inline__ s64 atomic64_dec_return_relaxed(atomic64_t *v) -{ - s64 t; - - __asm__ __volatile__( -"1: ldarx %0,0,%2 # atomic64_dec_return_relaxed\n" -" addic %0,%0,-1\n" -" stdcx. %0,0,%2\n" -" bne- 1b" - : "=&r" (t), "+m" (v->counter) - : "r" (&v->counter) - : "cc", "xer"); - - return t; -} - -#define atomic64_inc_return_relaxed atomic64_inc_return_relaxed -#define atomic64_dec_return_relaxed atomic64_dec_return_relaxed +#define atomic_dec_if_positive_relaxed atomic_dec_if_positive_relaxed +#ifdef CONFIG_64BIT /* * Atomically test *v and decrement if it is greater than 0. * The function returns the old value of *v minus 1. */ -static __inline__ s64 atomic64_dec_if_positive(atomic64_t *v) +static inline s64 atomic64_dec_if_positive_relaxed(atomic64_t *v) { s64 t; - __asm__ __volatile__( + asm volatile( PPC_ATOMIC_ENTRY_BARRIER "1: ldarx %0,0,%1 # atomic64_dec_if_positive\n\ addic. %0,%0,-1\n\ @@ -497,80 +368,8 @@ static __inline__ s64 atomic64_dec_if_positive(atomic64_t *v) return t; } -#define atomic64_dec_if_positive atomic64_dec_if_positive - -#define atomic64_cmpxchg(v, o, n) (cmpxchg(&((v)->counter), (o), (n))) -#define atomic64_cmpxchg_relaxed(v, o, n) \ - cmpxchg_relaxed(&((v)->counter), (o), (n)) -#define atomic64_cmpxchg_acquire(v, o, n) \ - cmpxchg_acquire(&((v)->counter), (o), (n)) - -#define atomic64_xchg(v, new) (xchg(&((v)->counter), new)) -#define atomic64_xchg_relaxed(v, new) xchg_relaxed(&((v)->counter), (new)) - -/** - * atomic64_fetch_add_unless - add unless the number is a given value - * @v: pointer of type atomic64_t - * @a: the amount to add to v... - * @u: ...unless v is equal to u. - * - * Atomically adds @a to @v, so long as it was not @u. - * Returns the old value of @v. - */ -static __inline__ s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u) -{ - s64 t; - - __asm__ __volatile__ ( - PPC_ATOMIC_ENTRY_BARRIER -"1: ldarx %0,0,%1 # atomic64_fetch_add_unless\n\ - cmpd 0,%0,%3 \n\ - beq 2f \n\ - add %0,%2,%0 \n" -" stdcx. %0,0,%1 \n\ - bne- 1b \n" - PPC_ATOMIC_EXIT_BARRIER -" subf %0,%2,%0 \n\ -2:" - : "=&r" (t) - : "r" (&v->counter), "r" (a), "r" (u) - : "cc", "memory"); - - return t; -} -#define atomic64_fetch_add_unless atomic64_fetch_add_unless - -/** - * atomic_inc64_not_zero - increment unless the number is zero - * @v: pointer of type atomic64_t - * - * Atomically increments @v by 1, so long as @v is non-zero. - * Returns non-zero if @v was non-zero, and zero otherwise. - */ -static __inline__ int atomic64_inc_not_zero(atomic64_t *v) -{ - s64 t1, t2; - - __asm__ __volatile__ ( - PPC_ATOMIC_ENTRY_BARRIER -"1: ldarx %0,0,%2 # atomic64_inc_not_zero\n\ - cmpdi 0,%0,0\n\ - beq- 2f\n\ - addic %1,%0,1\n\ - stdcx. %1,0,%2\n\ - bne- 1b\n" - PPC_ATOMIC_EXIT_BARRIER - "\n\ -2:" - : "=&r" (t1), "=&r" (t2) - : "r" (&v->counter) - : "cc", "xer", "memory"); - - return t1 != 0; -} -#define atomic64_inc_not_zero(v) atomic64_inc_not_zero((v)) - -#endif /* __powerpc64__ */ +#define atomic64_dec_if_positive_relaxed atomic64_dec_if_positive_relaxed +#endif #endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_ATOMIC_H_ */ diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h index cf091c4c22e5..181f7e8b3281 100644 --- a/arch/powerpc/include/asm/cmpxchg.h +++ b/arch/powerpc/include/asm/cmpxchg.h @@ -192,7 +192,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size) (unsigned long)_x_, sizeof(*(ptr))); \ }) -#define xchg_relaxed(ptr, x) \ +#define arch_xchg_relaxed(ptr, x) \ ({ \ __typeof__(*(ptr)) _x_ = (x); \ (__typeof__(*(ptr))) __xchg_relaxed((ptr), \ @@ -448,35 +448,7 @@ __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, return old; } -static __always_inline unsigned long -__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, - unsigned int size) -{ - switch (size) { - case 1: - return __cmpxchg_u8_acquire(ptr, old, new); - case 2: - return __cmpxchg_u16_acquire(ptr, old, new); - case 4: - return __cmpxchg_u32_acquire(ptr, old, new); -#ifdef CONFIG_PPC64 - case 8: - return __cmpxchg_u64_acquire(ptr, old, new); -#endif - } - BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire"); - return old; -} -#define cmpxchg(ptr, o, n) \ - ({ \ - __typeof__(*(ptr)) _o_ = (o); \ - __typeof__(*(ptr)) _n_ = (n); \ - (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_, \ - (unsigned long)_n_, sizeof(*(ptr))); \ - }) - - -#define cmpxchg_local(ptr, o, n) \ +#define arch_cmpxchg_local(ptr, o, n) \ ({ \ __typeof__(*(ptr)) _o_ = (o); \ __typeof__(*(ptr)) _n_ = (n); \ @@ -484,7 +456,7 @@ __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, (unsigned long)_n_, sizeof(*(ptr))); \ }) -#define cmpxchg_relaxed(ptr, o, n) \ +#define arch_cmpxchg_relaxed(ptr, o, n) \ ({ \ __typeof__(*(ptr)) _o_ = (o); \ __typeof__(*(ptr)) _n_ = (n); \ @@ -493,38 +465,20 @@ __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, sizeof(*(ptr))); \ }) -#define cmpxchg_acquire(ptr, o, n) \ -({ \ - __typeof__(*(ptr)) _o_ = (o); \ - __typeof__(*(ptr)) _n_ = (n); \ - (__typeof__(*(ptr))) __cmpxchg_acquire((ptr), \ - (unsigned long)_o_, (unsigned long)_n_, \ - sizeof(*(ptr))); \ -}) #ifdef CONFIG_PPC64 -#define cmpxchg64(ptr, o, n) \ - ({ \ - BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ - cmpxchg((ptr), (o), (n)); \ - }) -#define cmpxchg64_local(ptr, o, n) \ +#define arch_cmpxchg64_local(ptr, o, n) \ ({ \ BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ - cmpxchg_local((ptr), (o), (n)); \ + arch_cmpxchg_local((ptr), (o), (n)); \ }) -#define cmpxchg64_relaxed(ptr, o, n) \ -({ \ - BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ - cmpxchg_relaxed((ptr), (o), (n)); \ -}) -#define cmpxchg64_acquire(ptr, o, n) \ +#define arch_cmpxchg64_relaxed(ptr, o, n) \ ({ \ BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ - cmpxchg_acquire((ptr), (o), (n)); \ + arch_cmpxchg_relaxed((ptr), (o), (n)); \ }) #else #include <asm-generic/cmpxchg-local.h> -#define cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n)) +#define arch_cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n)) #endif #endif /* __KERNEL__ */ -- 2.23.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC 2020-11-11 11:07 ` [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC Nicholas Piggin @ 2020-11-11 19:07 ` kernel test robot 2020-11-13 5:05 ` kernel test robot 2020-11-13 15:30 ` Boqun Feng 2 siblings, 0 replies; 13+ messages in thread From: kernel test robot @ 2020-11-11 19:07 UTC (permalink / raw) To: Nicholas Piggin, linuxppc-dev Cc: Christophe Leroy, kbuild-all, Arnd Bergmann, Peter Zijlstra, Boqun Feng, linux-kernel, Nicholas Piggin, Alexey Kardashevskiy, Will Deacon [-- Attachment #1: Type: text/plain, Size: 13242 bytes --] Hi Nicholas, I love your patch! Perhaps something to improve: [auto build test WARNING on powerpc/next] [also build test WARNING on asm-generic/master linus/master v5.10-rc3 next-20201111] [cannot apply to scottwood/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc-allyesconfig (attached as .config) compiler: powerpc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/9e1bec8fe216b0745c647e52c40d1f0033fb4efd git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941 git checkout 9e1bec8fe216b0745c647e52c40d1f0033fb4efd # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): In file included from arch/powerpc/include/asm/atomic.h:11, from include/linux/atomic.h:7, from include/linux/rcupdate.h:25, from include/linux/rculist.h:11, from include/linux/sched/signal.h:5, from drivers/gpu/drm/drm_lock.c:37: drivers/gpu/drm/drm_lock.c: In function 'drm_lock_take': >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed' 73 | typeof(op##_relaxed(args)) __ret; \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:75:10: note: in expansion of macro 'cmpxchg' 75 | prev = cmpxchg(lock, old, new); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed' 75 | __ret = op##_relaxed(args); \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:75:10: note: in expansion of macro 'cmpxchg' 75 | prev = cmpxchg(lock, old, new); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ drivers/gpu/drm/drm_lock.c: In function 'drm_lock_transfer': >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed' 73 | typeof(op##_relaxed(args)) __ret; \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:118:10: note: in expansion of macro 'cmpxchg' 118 | prev = cmpxchg(lock, old, new); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed' 75 | __ret = op##_relaxed(args); \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:118:10: note: in expansion of macro 'cmpxchg' 118 | prev = cmpxchg(lock, old, new); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ drivers/gpu/drm/drm_lock.c: In function 'drm_legacy_lock_free': >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed' 73 | typeof(op##_relaxed(args)) __ret; \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:141:10: note: in expansion of macro 'cmpxchg' 141 | prev = cmpxchg(lock, old, new); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed' 75 | __ret = op##_relaxed(args); \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:141:10: note: in expansion of macro 'cmpxchg' 141 | prev = cmpxchg(lock, old, new); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ drivers/gpu/drm/drm_lock.c: In function 'drm_legacy_idlelock_release': >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:73:9: note: in expansion of macro 'arch_cmpxchg_relaxed' 73 | typeof(op##_relaxed(args)) __ret; \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:319:12: note: in expansion of macro 'cmpxchg' 319 | prev = cmpxchg(lock, old, DRM_KERNEL_CONTEXT); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ >> arch/powerpc/include/asm/cmpxchg.h:463:41: warning: passing argument 1 of '__cmpxchg_relaxed' discards 'volatile' qualifier from pointer target type [-Wdiscarded-qualifiers] 463 | (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ | ^~~~~ include/linux/atomic.h:75:10: note: in expansion of macro 'arch_cmpxchg_relaxed' 75 | __ret = op##_relaxed(args); \ | ^~ include/linux/atomic-arch-fallback.h:52:2: note: in expansion of macro '__atomic_op_fence' 52 | __atomic_op_fence(arch_cmpxchg, __VA_ARGS__) | ^~~~~~~~~~~~~~~~~ include/asm-generic/atomic-instrumented.h:1685:2: note: in expansion of macro 'arch_cmpxchg' 1685 | arch_cmpxchg(__ai_ptr, __VA_ARGS__); \ | ^~~~~~~~~~~~ drivers/gpu/drm/drm_lock.c:319:12: note: in expansion of macro 'cmpxchg' 319 | prev = cmpxchg(lock, old, DRM_KERNEL_CONTEXT); | ^~~~~~~ arch/powerpc/include/asm/cmpxchg.h:432:25: note: expected 'void *' but argument is of type 'volatile unsigned int *' 432 | __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, | ~~~~~~^~~ vim +463 arch/powerpc/include/asm/cmpxchg.h 56c08e6d226c860 Boqun Feng 2015-12-15 450 9e1bec8fe216b07 Nicholas Piggin 2020-11-11 451 #define arch_cmpxchg_local(ptr, o, n) \ ae3a197e3d0bfe3 David Howells 2012-03-28 452 ({ \ ae3a197e3d0bfe3 David Howells 2012-03-28 453 __typeof__(*(ptr)) _o_ = (o); \ ae3a197e3d0bfe3 David Howells 2012-03-28 454 __typeof__(*(ptr)) _n_ = (n); \ ae3a197e3d0bfe3 David Howells 2012-03-28 455 (__typeof__(*(ptr))) __cmpxchg_local((ptr), (unsigned long)_o_, \ ae3a197e3d0bfe3 David Howells 2012-03-28 456 (unsigned long)_n_, sizeof(*(ptr))); \ ae3a197e3d0bfe3 David Howells 2012-03-28 457 }) ae3a197e3d0bfe3 David Howells 2012-03-28 458 9e1bec8fe216b07 Nicholas Piggin 2020-11-11 459 #define arch_cmpxchg_relaxed(ptr, o, n) \ 56c08e6d226c860 Boqun Feng 2015-12-15 460 ({ \ 56c08e6d226c860 Boqun Feng 2015-12-15 461 __typeof__(*(ptr)) _o_ = (o); \ 56c08e6d226c860 Boqun Feng 2015-12-15 462 __typeof__(*(ptr)) _n_ = (n); \ 56c08e6d226c860 Boqun Feng 2015-12-15 @463 (__typeof__(*(ptr))) __cmpxchg_relaxed((ptr), \ 56c08e6d226c860 Boqun Feng 2015-12-15 464 (unsigned long)_o_, (unsigned long)_n_, \ 56c08e6d226c860 Boqun Feng 2015-12-15 465 sizeof(*(ptr))); \ 56c08e6d226c860 Boqun Feng 2015-12-15 466 }) 56c08e6d226c860 Boqun Feng 2015-12-15 467 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 71460 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC 2020-11-11 11:07 ` [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC Nicholas Piggin 2020-11-11 19:07 ` kernel test robot @ 2020-11-13 5:05 ` kernel test robot 2020-11-13 15:30 ` Boqun Feng 2 siblings, 0 replies; 13+ messages in thread From: kernel test robot @ 2020-11-13 5:05 UTC (permalink / raw) To: Nicholas Piggin, linuxppc-dev Cc: Christophe Leroy, kbuild-all, Arnd Bergmann, Peter Zijlstra, Boqun Feng, linux-kernel, Nicholas Piggin, Alexey Kardashevskiy, Will Deacon [-- Attachment #1: Type: text/plain, Size: 7933 bytes --] Hi Nicholas, I love your patch! Perhaps something to improve: [auto build test WARNING on powerpc/next] [also build test WARNING on asm-generic/master linus/master v5.10-rc3 next-20201112] [cannot apply to scottwood/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941 base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next config: powerpc64-randconfig-s031-20201111 (attached as .config) compiler: powerpc-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-107-gaf3512a6-dirty # https://github.com/0day-ci/linux/commit/9e1bec8fe216b0745c647e52c40d1f0033fb4efd git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Nicholas-Piggin/powerpc-convert-to-use-ARCH_ATOMIC/20201111-190941 git checkout 9e1bec8fe216b0745c647e52c40d1f0033fb4efd # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=powerpc64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> "sparse warnings: (new ones prefixed by >>)" >> drivers/gpu/drm/drm_lock.c:75:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:75:24: sparse: expected void *ptr >> drivers/gpu/drm/drm_lock.c:75:24: sparse: got unsigned int volatile *__ai_ptr >> drivers/gpu/drm/drm_lock.c:75:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:75:24: sparse: expected void *ptr >> drivers/gpu/drm/drm_lock.c:75:24: sparse: got unsigned int volatile *__ai_ptr drivers/gpu/drm/drm_lock.c:118:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:118:24: sparse: expected void *ptr drivers/gpu/drm/drm_lock.c:118:24: sparse: got unsigned int volatile *__ai_ptr drivers/gpu/drm/drm_lock.c:118:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:118:24: sparse: expected void *ptr drivers/gpu/drm/drm_lock.c:118:24: sparse: got unsigned int volatile *__ai_ptr drivers/gpu/drm/drm_lock.c:141:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:141:24: sparse: expected void *ptr drivers/gpu/drm/drm_lock.c:141:24: sparse: got unsigned int volatile *__ai_ptr drivers/gpu/drm/drm_lock.c:141:24: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:141:24: sparse: expected void *ptr drivers/gpu/drm/drm_lock.c:141:24: sparse: got unsigned int volatile *__ai_ptr drivers/gpu/drm/drm_lock.c:319:40: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:319:40: sparse: expected void *ptr drivers/gpu/drm/drm_lock.c:319:40: sparse: got unsigned int volatile *__ai_ptr drivers/gpu/drm/drm_lock.c:319:40: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void *ptr @@ got unsigned int volatile *__ai_ptr @@ drivers/gpu/drm/drm_lock.c:319:40: sparse: expected void *ptr drivers/gpu/drm/drm_lock.c:319:40: sparse: got unsigned int volatile *__ai_ptr vim +75 drivers/gpu/drm/drm_lock.c 4ac5ec40ec70022 Daniel Vetter 2010-08-23 48 bd50d4a2168370b Benjamin Gaignard 2020-03-06 49 /* 1a75a222f5ca106 Daniel Vetter 2016-06-14 50 * Take the heavyweight lock. 1a75a222f5ca106 Daniel Vetter 2016-06-14 51 * 1a75a222f5ca106 Daniel Vetter 2016-06-14 52 * \param lock lock pointer. 1a75a222f5ca106 Daniel Vetter 2016-06-14 53 * \param context locking context. 1a75a222f5ca106 Daniel Vetter 2016-06-14 54 * \return one if the lock is held, or zero otherwise. 1a75a222f5ca106 Daniel Vetter 2016-06-14 55 * 1a75a222f5ca106 Daniel Vetter 2016-06-14 56 * Attempt to mark the lock as held by the given context, via the \p cmpxchg instruction. 1a75a222f5ca106 Daniel Vetter 2016-06-14 57 */ 1a75a222f5ca106 Daniel Vetter 2016-06-14 58 static 1a75a222f5ca106 Daniel Vetter 2016-06-14 59 int drm_lock_take(struct drm_lock_data *lock_data, 1a75a222f5ca106 Daniel Vetter 2016-06-14 60 unsigned int context) 1a75a222f5ca106 Daniel Vetter 2016-06-14 61 { 1a75a222f5ca106 Daniel Vetter 2016-06-14 62 unsigned int old, new, prev; 1a75a222f5ca106 Daniel Vetter 2016-06-14 63 volatile unsigned int *lock = &lock_data->hw_lock->lock; 1a75a222f5ca106 Daniel Vetter 2016-06-14 64 1a75a222f5ca106 Daniel Vetter 2016-06-14 65 spin_lock_bh(&lock_data->spinlock); 1a75a222f5ca106 Daniel Vetter 2016-06-14 66 do { 1a75a222f5ca106 Daniel Vetter 2016-06-14 67 old = *lock; 1a75a222f5ca106 Daniel Vetter 2016-06-14 68 if (old & _DRM_LOCK_HELD) 1a75a222f5ca106 Daniel Vetter 2016-06-14 69 new = old | _DRM_LOCK_CONT; 1a75a222f5ca106 Daniel Vetter 2016-06-14 70 else { 1a75a222f5ca106 Daniel Vetter 2016-06-14 71 new = context | _DRM_LOCK_HELD | 1a75a222f5ca106 Daniel Vetter 2016-06-14 72 ((lock_data->user_waiters + lock_data->kernel_waiters > 1) ? 1a75a222f5ca106 Daniel Vetter 2016-06-14 73 _DRM_LOCK_CONT : 0); 1a75a222f5ca106 Daniel Vetter 2016-06-14 74 } 1a75a222f5ca106 Daniel Vetter 2016-06-14 @75 prev = cmpxchg(lock, old, new); 1a75a222f5ca106 Daniel Vetter 2016-06-14 76 } while (prev != old); 1a75a222f5ca106 Daniel Vetter 2016-06-14 77 spin_unlock_bh(&lock_data->spinlock); 1a75a222f5ca106 Daniel Vetter 2016-06-14 78 1a75a222f5ca106 Daniel Vetter 2016-06-14 79 if (_DRM_LOCKING_CONTEXT(old) == context) { 1a75a222f5ca106 Daniel Vetter 2016-06-14 80 if (old & _DRM_LOCK_HELD) { 1a75a222f5ca106 Daniel Vetter 2016-06-14 81 if (context != DRM_KERNEL_CONTEXT) { 1a75a222f5ca106 Daniel Vetter 2016-06-14 82 DRM_ERROR("%d holds heavyweight lock\n", 1a75a222f5ca106 Daniel Vetter 2016-06-14 83 context); 1a75a222f5ca106 Daniel Vetter 2016-06-14 84 } 1a75a222f5ca106 Daniel Vetter 2016-06-14 85 return 0; 1a75a222f5ca106 Daniel Vetter 2016-06-14 86 } 1a75a222f5ca106 Daniel Vetter 2016-06-14 87 } 1a75a222f5ca106 Daniel Vetter 2016-06-14 88 1a75a222f5ca106 Daniel Vetter 2016-06-14 89 if ((_DRM_LOCKING_CONTEXT(new)) == context && (new & _DRM_LOCK_HELD)) { 1a75a222f5ca106 Daniel Vetter 2016-06-14 90 /* Have lock */ 1a75a222f5ca106 Daniel Vetter 2016-06-14 91 return 1; 1a75a222f5ca106 Daniel Vetter 2016-06-14 92 } 1a75a222f5ca106 Daniel Vetter 2016-06-14 93 return 0; 1a75a222f5ca106 Daniel Vetter 2016-06-14 94 } 1a75a222f5ca106 Daniel Vetter 2016-06-14 95 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 37481 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC 2020-11-11 11:07 ` [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC Nicholas Piggin 2020-11-11 19:07 ` kernel test robot 2020-11-13 5:05 ` kernel test robot @ 2020-11-13 15:30 ` Boqun Feng 2020-12-22 3:52 ` Nicholas Piggin 2 siblings, 1 reply; 13+ messages in thread From: Boqun Feng @ 2020-11-13 15:30 UTC (permalink / raw) To: Nicholas Piggin Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Peter Zijlstra, linuxppc-dev, linux-kernel, Alexey Kardashevskiy, Will Deacon Hi Nicholas, On Wed, Nov 11, 2020 at 09:07:23PM +1000, Nicholas Piggin wrote: > All the cool kids are doing it. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/include/asm/atomic.h | 681 ++++++++++------------------- > arch/powerpc/include/asm/cmpxchg.h | 62 +-- > 2 files changed, 248 insertions(+), 495 deletions(-) > > diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h > index 8a55eb8cc97b..899aa2403ba7 100644 > --- a/arch/powerpc/include/asm/atomic.h > +++ b/arch/powerpc/include/asm/atomic.h > @@ -11,185 +11,285 @@ > #include <asm/cmpxchg.h> > #include <asm/barrier.h> > > +#define ARCH_ATOMIC > + > +#ifndef CONFIG_64BIT > +#include <asm-generic/atomic64.h> > +#endif > + > /* > * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with > * a "bne-" instruction at the end, so an isync is enough as a acquire barrier > * on the platform without lwsync. > */ > #define __atomic_acquire_fence() \ > - __asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory") > + asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory") > > #define __atomic_release_fence() \ > - __asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory") > + asm volatile(PPC_RELEASE_BARRIER "" : : : "memory") > > -static __inline__ int atomic_read(const atomic_t *v) > -{ > - int t; > +#define __atomic_pre_full_fence smp_mb > > - __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter)); > +#define __atomic_post_full_fence smp_mb > Do you need to define __atomic_{pre,post}_full_fence for PPC? IIRC, they are default smp_mb__{before,atomic}_atomic(), so are smp_mb() defautly on PPC. > - return t; > +#define arch_atomic_read(v) __READ_ONCE((v)->counter) > +#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), (i)) > +#ifdef CONFIG_64BIT > +#define ATOMIC64_INIT(i) { (i) } > +#define arch_atomic64_read(v) __READ_ONCE((v)->counter) > +#define arch_atomic64_set(v, i) __WRITE_ONCE(((v)->counter), (i)) > +#endif > + [...] > > +#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \ > +static inline int arch_##name##_relaxed(type *v, dtype a, dtype u) \ I don't think we have atomic_fetch_*_unless_relaxed() at atomic APIs, ditto for: atomic_fetch_add_unless_relaxed() atomic_inc_not_zero_relaxed() atomic_dec_if_positive_relaxed() , and we don't have the _acquire() and _release() variants for them either, and if you don't define their fully-ordered version (e.g. atomic_inc_not_zero()), atomic-arch-fallback.h will use read and cmpxchg to implement them, and I think not what we want. [...] > > #endif /* __KERNEL__ */ > #endif /* _ASM_POWERPC_ATOMIC_H_ */ > diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h > index cf091c4c22e5..181f7e8b3281 100644 > --- a/arch/powerpc/include/asm/cmpxchg.h > +++ b/arch/powerpc/include/asm/cmpxchg.h > @@ -192,7 +192,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size) > (unsigned long)_x_, sizeof(*(ptr))); \ > }) > > -#define xchg_relaxed(ptr, x) \ > +#define arch_xchg_relaxed(ptr, x) \ > ({ \ > __typeof__(*(ptr)) _x_ = (x); \ > (__typeof__(*(ptr))) __xchg_relaxed((ptr), \ > @@ -448,35 +448,7 @@ __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, > return old; > } > > -static __always_inline unsigned long > -__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, > - unsigned int size) > -{ > - switch (size) { > - case 1: > - return __cmpxchg_u8_acquire(ptr, old, new); > - case 2: > - return __cmpxchg_u16_acquire(ptr, old, new); > - case 4: > - return __cmpxchg_u32_acquire(ptr, old, new); > -#ifdef CONFIG_PPC64 > - case 8: > - return __cmpxchg_u64_acquire(ptr, old, new); > -#endif > - } > - BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire"); > - return old; > -} > -#define cmpxchg(ptr, o, n) \ > - ({ \ > - __typeof__(*(ptr)) _o_ = (o); \ > - __typeof__(*(ptr)) _n_ = (n); \ > - (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_, \ > - (unsigned long)_n_, sizeof(*(ptr))); \ > - }) > - > - If you remove {atomic_}_cmpxchg_{,_acquire}() and use the version provided by atomic-arch-fallback.h, then a fail cmpxchg or cmpxchg_acquire() will still result into a full barrier or a acquire barrier after the RMW operation, the barrier is not necessary and probably this is not what we want? Regards, Boqun > -#define cmpxchg_local(ptr, o, n) \ > +#define arch_cmpxchg_local(ptr, o, n) \ > ({ \ > __typeof__(*(ptr)) _o_ = (o); \ > __typeof__(*(ptr)) _n_ = (n); \ > @@ -484,7 +456,7 @@ __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, > (unsigned long)_n_, sizeof(*(ptr))); \ > }) > > -#define cmpxchg_relaxed(ptr, o, n) \ > +#define arch_cmpxchg_relaxed(ptr, o, n) \ > ({ \ > __typeof__(*(ptr)) _o_ = (o); \ > __typeof__(*(ptr)) _n_ = (n); \ > @@ -493,38 +465,20 @@ __cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, > sizeof(*(ptr))); \ > }) > > -#define cmpxchg_acquire(ptr, o, n) \ > -({ \ > - __typeof__(*(ptr)) _o_ = (o); \ > - __typeof__(*(ptr)) _n_ = (n); \ > - (__typeof__(*(ptr))) __cmpxchg_acquire((ptr), \ > - (unsigned long)_o_, (unsigned long)_n_, \ > - sizeof(*(ptr))); \ > -}) > #ifdef CONFIG_PPC64 > -#define cmpxchg64(ptr, o, n) \ > - ({ \ > - BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ > - cmpxchg((ptr), (o), (n)); \ > - }) > -#define cmpxchg64_local(ptr, o, n) \ > +#define arch_cmpxchg64_local(ptr, o, n) \ > ({ \ > BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ > - cmpxchg_local((ptr), (o), (n)); \ > + arch_cmpxchg_local((ptr), (o), (n)); \ > }) > -#define cmpxchg64_relaxed(ptr, o, n) \ > -({ \ > - BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ > - cmpxchg_relaxed((ptr), (o), (n)); \ > -}) > -#define cmpxchg64_acquire(ptr, o, n) \ > +#define arch_cmpxchg64_relaxed(ptr, o, n) \ > ({ \ > BUILD_BUG_ON(sizeof(*(ptr)) != 8); \ > - cmpxchg_acquire((ptr), (o), (n)); \ > + arch_cmpxchg_relaxed((ptr), (o), (n)); \ > }) > #else > #include <asm-generic/cmpxchg-local.h> > -#define cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n)) > +#define arch_cmpxchg64_local(ptr, o, n) __cmpxchg64_local_generic((ptr), (o), (n)) > #endif > > #endif /* __KERNEL__ */ > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC 2020-11-13 15:30 ` Boqun Feng @ 2020-12-22 3:52 ` Nicholas Piggin 2020-12-23 2:45 ` Boqun Feng 0 siblings, 1 reply; 13+ messages in thread From: Nicholas Piggin @ 2020-12-22 3:52 UTC (permalink / raw) To: Boqun Feng Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Alexey Kardashevskiy, linux-kernel, Peter Zijlstra, Will Deacon, linuxppc-dev Excerpts from Boqun Feng's message of November 14, 2020 1:30 am: > Hi Nicholas, > > On Wed, Nov 11, 2020 at 09:07:23PM +1000, Nicholas Piggin wrote: >> All the cool kids are doing it. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> arch/powerpc/include/asm/atomic.h | 681 ++++++++++------------------- >> arch/powerpc/include/asm/cmpxchg.h | 62 +-- >> 2 files changed, 248 insertions(+), 495 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h >> index 8a55eb8cc97b..899aa2403ba7 100644 >> --- a/arch/powerpc/include/asm/atomic.h >> +++ b/arch/powerpc/include/asm/atomic.h >> @@ -11,185 +11,285 @@ >> #include <asm/cmpxchg.h> >> #include <asm/barrier.h> >> >> +#define ARCH_ATOMIC >> + >> +#ifndef CONFIG_64BIT >> +#include <asm-generic/atomic64.h> >> +#endif >> + >> /* >> * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with >> * a "bne-" instruction at the end, so an isync is enough as a acquire barrier >> * on the platform without lwsync. >> */ >> #define __atomic_acquire_fence() \ >> - __asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory") >> + asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory") >> >> #define __atomic_release_fence() \ >> - __asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory") >> + asm volatile(PPC_RELEASE_BARRIER "" : : : "memory") >> >> -static __inline__ int atomic_read(const atomic_t *v) >> -{ >> - int t; >> +#define __atomic_pre_full_fence smp_mb >> >> - __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter)); >> +#define __atomic_post_full_fence smp_mb >> Thanks for the review. > Do you need to define __atomic_{pre,post}_full_fence for PPC? IIRC, they > are default smp_mb__{before,atomic}_atomic(), so are smp_mb() defautly > on PPC. Okay I didn't realise that's not required. >> - return t; >> +#define arch_atomic_read(v) __READ_ONCE((v)->counter) >> +#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), (i)) >> +#ifdef CONFIG_64BIT >> +#define ATOMIC64_INIT(i) { (i) } >> +#define arch_atomic64_read(v) __READ_ONCE((v)->counter) >> +#define arch_atomic64_set(v, i) __WRITE_ONCE(((v)->counter), (i)) >> +#endif >> + > [...] >> >> +#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \ >> +static inline int arch_##name##_relaxed(type *v, dtype a, dtype u) \ > > I don't think we have atomic_fetch_*_unless_relaxed() at atomic APIs, > ditto for: > > atomic_fetch_add_unless_relaxed() > atomic_inc_not_zero_relaxed() > atomic_dec_if_positive_relaxed() > > , and we don't have the _acquire() and _release() variants for them > either, and if you don't define their fully-ordered version (e.g. > atomic_inc_not_zero()), atomic-arch-fallback.h will use read and cmpxchg > to implement them, and I think not what we want. Okay. How can those be added? The atoimc generation is pretty complicated. > [...] >> >> #endif /* __KERNEL__ */ >> #endif /* _ASM_POWERPC_ATOMIC_H_ */ >> diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h >> index cf091c4c22e5..181f7e8b3281 100644 >> --- a/arch/powerpc/include/asm/cmpxchg.h >> +++ b/arch/powerpc/include/asm/cmpxchg.h >> @@ -192,7 +192,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size) >> (unsigned long)_x_, sizeof(*(ptr))); \ >> }) >> >> -#define xchg_relaxed(ptr, x) \ >> +#define arch_xchg_relaxed(ptr, x) \ >> ({ \ >> __typeof__(*(ptr)) _x_ = (x); \ >> (__typeof__(*(ptr))) __xchg_relaxed((ptr), \ >> @@ -448,35 +448,7 @@ __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, >> return old; >> } >> >> -static __always_inline unsigned long >> -__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, >> - unsigned int size) >> -{ >> - switch (size) { >> - case 1: >> - return __cmpxchg_u8_acquire(ptr, old, new); >> - case 2: >> - return __cmpxchg_u16_acquire(ptr, old, new); >> - case 4: >> - return __cmpxchg_u32_acquire(ptr, old, new); >> -#ifdef CONFIG_PPC64 >> - case 8: >> - return __cmpxchg_u64_acquire(ptr, old, new); >> -#endif >> - } >> - BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire"); >> - return old; >> -} >> -#define cmpxchg(ptr, o, n) \ >> - ({ \ >> - __typeof__(*(ptr)) _o_ = (o); \ >> - __typeof__(*(ptr)) _n_ = (n); \ >> - (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_, \ >> - (unsigned long)_n_, sizeof(*(ptr))); \ >> - }) >> - >> - > > If you remove {atomic_}_cmpxchg_{,_acquire}() and use the version > provided by atomic-arch-fallback.h, then a fail cmpxchg or > cmpxchg_acquire() will still result into a full barrier or a acquire > barrier after the RMW operation, the barrier is not necessary and > probably this is not what we want? Why is that done? That seems like a very subtle difference. Shouldn't the fallback version skip the barrier? Thanks, Nick ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC 2020-12-22 3:52 ` Nicholas Piggin @ 2020-12-23 2:45 ` Boqun Feng 0 siblings, 0 replies; 13+ messages in thread From: Boqun Feng @ 2020-12-23 2:45 UTC (permalink / raw) To: Nicholas Piggin Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Alexey Kardashevskiy, linux-kernel, Peter Zijlstra, Will Deacon, linuxppc-dev On Tue, Dec 22, 2020 at 01:52:50PM +1000, Nicholas Piggin wrote: > Excerpts from Boqun Feng's message of November 14, 2020 1:30 am: > > Hi Nicholas, > > > > On Wed, Nov 11, 2020 at 09:07:23PM +1000, Nicholas Piggin wrote: > >> All the cool kids are doing it. > >> > >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > >> --- > >> arch/powerpc/include/asm/atomic.h | 681 ++++++++++------------------- > >> arch/powerpc/include/asm/cmpxchg.h | 62 +-- > >> 2 files changed, 248 insertions(+), 495 deletions(-) > >> > >> diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h > >> index 8a55eb8cc97b..899aa2403ba7 100644 > >> --- a/arch/powerpc/include/asm/atomic.h > >> +++ b/arch/powerpc/include/asm/atomic.h > >> @@ -11,185 +11,285 @@ > >> #include <asm/cmpxchg.h> > >> #include <asm/barrier.h> > >> > >> +#define ARCH_ATOMIC > >> + > >> +#ifndef CONFIG_64BIT > >> +#include <asm-generic/atomic64.h> > >> +#endif > >> + > >> /* > >> * Since *_return_relaxed and {cmp}xchg_relaxed are implemented with > >> * a "bne-" instruction at the end, so an isync is enough as a acquire barrier > >> * on the platform without lwsync. > >> */ > >> #define __atomic_acquire_fence() \ > >> - __asm__ __volatile__(PPC_ACQUIRE_BARRIER "" : : : "memory") > >> + asm volatile(PPC_ACQUIRE_BARRIER "" : : : "memory") > >> > >> #define __atomic_release_fence() \ > >> - __asm__ __volatile__(PPC_RELEASE_BARRIER "" : : : "memory") > >> + asm volatile(PPC_RELEASE_BARRIER "" : : : "memory") > >> > >> -static __inline__ int atomic_read(const atomic_t *v) > >> -{ > >> - int t; > >> +#define __atomic_pre_full_fence smp_mb > >> > >> - __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter)); > >> +#define __atomic_post_full_fence smp_mb > >> > > Thanks for the review. > > > Do you need to define __atomic_{pre,post}_full_fence for PPC? IIRC, they > > are default smp_mb__{before,atomic}_atomic(), so are smp_mb() defautly > > on PPC. > > Okay I didn't realise that's not required. > > >> - return t; > >> +#define arch_atomic_read(v) __READ_ONCE((v)->counter) > >> +#define arch_atomic_set(v, i) __WRITE_ONCE(((v)->counter), (i)) > >> +#ifdef CONFIG_64BIT > >> +#define ATOMIC64_INIT(i) { (i) } > >> +#define arch_atomic64_read(v) __READ_ONCE((v)->counter) > >> +#define arch_atomic64_set(v, i) __WRITE_ONCE(((v)->counter), (i)) > >> +#endif > >> + > > [...] > >> > >> +#define ATOMIC_FETCH_OP_UNLESS_RELAXED(name, type, dtype, width, asm_op) \ > >> +static inline int arch_##name##_relaxed(type *v, dtype a, dtype u) \ > > > > I don't think we have atomic_fetch_*_unless_relaxed() at atomic APIs, > > ditto for: > > > > atomic_fetch_add_unless_relaxed() > > atomic_inc_not_zero_relaxed() > > atomic_dec_if_positive_relaxed() > > > > , and we don't have the _acquire() and _release() variants for them > > either, and if you don't define their fully-ordered version (e.g. > > atomic_inc_not_zero()), atomic-arch-fallback.h will use read and cmpxchg > > to implement them, and I think not what we want. > > Okay. How can those be added? The atoimc generation is pretty > complicated. > Yeah, I know ;-) I think you can just implement and define fully-ordered verions: arch_atomic_fetch_*_unless() arch_atomic_inc_not_zero() arch_atomic_dec_if_postive() , that should work. Rules of atomic generation, IIRC: 1. If you define _relaxed, _acquire, _release or fully-ordered version, atomic generation will use that version 2. If you define _relaxed, atomic generation will use that and barriers to generate _acquire, _release and fully-ordered versions, unless they are already defined (as Rule #1 says) 3. If you don't define _relaxed, but define the fully-ordered version, atomic generation will use the fully-ordered version and use it as _relaxed variants and generate the rest using Rule #2. > > [...] > >> > >> #endif /* __KERNEL__ */ > >> #endif /* _ASM_POWERPC_ATOMIC_H_ */ > >> diff --git a/arch/powerpc/include/asm/cmpxchg.h b/arch/powerpc/include/asm/cmpxchg.h > >> index cf091c4c22e5..181f7e8b3281 100644 > >> --- a/arch/powerpc/include/asm/cmpxchg.h > >> +++ b/arch/powerpc/include/asm/cmpxchg.h > >> @@ -192,7 +192,7 @@ __xchg_relaxed(void *ptr, unsigned long x, unsigned int size) > >> (unsigned long)_x_, sizeof(*(ptr))); \ > >> }) > >> > >> -#define xchg_relaxed(ptr, x) \ > >> +#define arch_xchg_relaxed(ptr, x) \ > >> ({ \ > >> __typeof__(*(ptr)) _x_ = (x); \ > >> (__typeof__(*(ptr))) __xchg_relaxed((ptr), \ > >> @@ -448,35 +448,7 @@ __cmpxchg_relaxed(void *ptr, unsigned long old, unsigned long new, > >> return old; > >> } > >> > >> -static __always_inline unsigned long > >> -__cmpxchg_acquire(void *ptr, unsigned long old, unsigned long new, > >> - unsigned int size) > >> -{ > >> - switch (size) { > >> - case 1: > >> - return __cmpxchg_u8_acquire(ptr, old, new); > >> - case 2: > >> - return __cmpxchg_u16_acquire(ptr, old, new); > >> - case 4: > >> - return __cmpxchg_u32_acquire(ptr, old, new); > >> -#ifdef CONFIG_PPC64 > >> - case 8: > >> - return __cmpxchg_u64_acquire(ptr, old, new); > >> -#endif > >> - } > >> - BUILD_BUG_ON_MSG(1, "Unsupported size for __cmpxchg_acquire"); > >> - return old; > >> -} > >> -#define cmpxchg(ptr, o, n) \ > >> - ({ \ > >> - __typeof__(*(ptr)) _o_ = (o); \ > >> - __typeof__(*(ptr)) _n_ = (n); \ > >> - (__typeof__(*(ptr))) __cmpxchg((ptr), (unsigned long)_o_, \ > >> - (unsigned long)_n_, sizeof(*(ptr))); \ > >> - }) > >> - > >> - > > > > If you remove {atomic_}_cmpxchg_{,_acquire}() and use the version > > provided by atomic-arch-fallback.h, then a fail cmpxchg or > > cmpxchg_acquire() will still result into a full barrier or a acquire > > barrier after the RMW operation, the barrier is not necessary and > > probably this is not what we want? > > Why is that done? That seems like a very subtle difference. Shouldn't > the fallback version skip the barrier? > The fallback version is something like: smp_mb__before_atomic(); cmpxchg_relaxed(); smp_mb__after_atomic(); , so there will be a full barrier on PPC after the cmpxchg no matter whether the cmpxchg succeed or not. And the fallback version cannot skip the barrier, because there is no way the fallback version tells whether the cmpxchg_relaxed() succeed or not. So in my previous version of PPC atomic variants support, I defined cmpxchg_acquire() in asm header instead of using atomic generation. That said, now I think about this, maybe we can implement the fallback version as: smp_mb__before_atomic(); ret = cmpxchg_relaxed(ptr, old, new); if (old == ret) smp_mb__after_atomic(); , in this way, the fallback version can handle the barrier skipping better. Regards, Boqun > Thanks, > Nick ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] powerpc: convert to use ARCH_ATOMIC 2020-11-11 11:07 [PATCH 0/3] powerpc: convert to use ARCH_ATOMIC Nicholas Piggin ` (2 preceding siblings ...) 2020-11-11 11:07 ` [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC Nicholas Piggin @ 2020-12-15 11:01 ` Michael Ellerman 3 siblings, 0 replies; 13+ messages in thread From: Michael Ellerman @ 2020-12-15 11:01 UTC (permalink / raw) To: Nicholas Piggin, linuxppc-dev Cc: Christophe Leroy, linux-arch, Arnd Bergmann, Alexey Kardashevskiy, Boqun Feng, linux-kernel, Peter Zijlstra, Will Deacon On Wed, 11 Nov 2020 21:07:20 +1000, Nicholas Piggin wrote: > This conversion seems to require generic atomic64 changes, looks > like nothing else uses ARCH_ATOMIC and GENERIC_ATOMIC64 yet. > > Thanks, > Nick > > Nicholas Piggin (3): > asm-generic/atomic64: Add support for ARCH_ATOMIC > powerpc/64s/iommu: don't use atomic_ function on atomic64_t type > powerpc: rewrite atomics to use ARCH_ATOMIC > > [...] Patch 2 applied to powerpc/next. [2/3] powerpc/64s/iommu: Don't use atomic_ function on atomic64_t type https://git.kernel.org/powerpc/c/c33cd1ed60013ec2ae50f91fed260def5f1d9851 cheers ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-12-23 2:49 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-11 11:07 [PATCH 0/3] powerpc: convert to use ARCH_ATOMIC Nicholas Piggin 2020-11-11 11:07 ` [PATCH 1/3] asm-generic/atomic64: Add support for ARCH_ATOMIC Nicholas Piggin 2020-11-11 13:39 ` Christophe Leroy 2020-11-11 13:44 ` Peter Zijlstra 2020-11-16 1:48 ` Nicholas Piggin 2020-11-11 11:07 ` [PATCH 2/3] powerpc/64s/iommu: don't use atomic_ function on atomic64_t type Nicholas Piggin 2020-11-11 11:07 ` [PATCH 3/3] powerpc: rewrite atomics to use ARCH_ATOMIC Nicholas Piggin 2020-11-11 19:07 ` kernel test robot 2020-11-13 5:05 ` kernel test robot 2020-11-13 15:30 ` Boqun Feng 2020-12-22 3:52 ` Nicholas Piggin 2020-12-23 2:45 ` Boqun Feng 2020-12-15 11:01 ` [PATCH 0/3] powerpc: convert " Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).