[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJF2gTT1_mP-wiK2HsCpTeU61NqZVKZX1A5ye=TwqvGN4TPmrA@mail.gmail.com>
Date: Wed, 25 Nov 2020 22:18:43 +0800
From: Guo Ren <guoren@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Arnd Bergmann <arnd@...db.de>,
Palmer Dabbelt <palmerdabbelt@...gle.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Anup Patel <anup@...infault.org>,
linux-riscv <linux-riscv@...ts.infradead.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-csky@...r.kernel.org, Guo Ren <guoren@...ux.alibaba.com>,
Michael Clark <michaeljclark@....com>
Subject: Re: [PATCH 1/5] riscv: Coding convention for xchg
On Tue, Nov 24, 2020 at 10:29 PM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Tue, Nov 24, 2020 at 01:43:53PM +0000, guoren@...nel.org wrote:
> > From: Guo Ren <guoren@...ux.alibaba.com>
> >
> > This is prepare for QUEUED_SPINLOCKS which need xchg support short
> > type value.
> > - Remove unused codes (xchg32, xchg64, cmpxchg32 ...)
> > - Combine xchg_relaxed, xchg_acquire, xchg_release into one asm
> > - Make atomic.aq/rl with seperated fence acquire & release
>
> Every time you find yourself doing multiple things, make it multiple
> patches.
Ok.
>
> > @@ -242,58 +239,58 @@ static __always_inline s64 atomic64_fetch_add_unless(atomic64_t *v, s64 a, s64 u
> > * atomic_{cmp,}xchg is required to have exactly the same ordering semantics as
> > * {cmp,}xchg and the operations that return, so they need a full barrier.
> > */
> > +#define ATOMIC_OP(c_t, prefix) \
> > static __always_inline \
> > c_t atomic##prefix##_xchg_relaxed(atomic##prefix##_t *v, c_t n) \
> > { \
> > + return xchg_relaxed(&(v->counter), n); \
> > } \
> > static __always_inline \
> > c_t atomic##prefix##_xchg_acquire(atomic##prefix##_t *v, c_t n) \
> > { \
> > + return xchg_acquire(&(v->counter), n); \
> > } \
> > static __always_inline \
> > c_t atomic##prefix##_xchg_release(atomic##prefix##_t *v, c_t n) \
> > { \
> > + return xchg_release(&(v->counter), n); \
> > } \
> > static __always_inline \
> > c_t atomic##prefix##_xchg(atomic##prefix##_t *v, c_t n) \
> > { \
> > + return xchg(&(v->counter), n); \
> > } \
> > static __always_inline \
> > c_t atomic##prefix##_cmpxchg_relaxed(atomic##prefix##_t *v, \
> > c_t o, c_t n) \
> > { \
> > + return cmpxchg_relaxed(&(v->counter), o, n); \
> > } \
> > static __always_inline \
> > c_t atomic##prefix##_cmpxchg_acquire(atomic##prefix##_t *v, \
> > c_t o, c_t n) \
> > { \
> > + return cmpxchg_acquire(&(v->counter), o, n); \
> > } \
> > static __always_inline \
> > c_t atomic##prefix##_cmpxchg_release(atomic##prefix##_t *v, \
> > c_t o, c_t n) \
> > { \
> > + return cmpxchg_release(&(v->counter), o, n); \
> > } \
> > static __always_inline \
> > c_t atomic##prefix##_cmpxchg(atomic##prefix##_t *v, c_t o, c_t n) \
> > { \
> > + return cmpxchg(&(v->counter), o, n); \
> > }
>
> > diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> > index 262e5bb..5609185 100644
> > --- a/arch/riscv/include/asm/cmpxchg.h
> > +++ b/arch/riscv/include/asm/cmpxchg.h
> > @@ -44,118 +44,31 @@
> > _x_, sizeof(*(ptr))); \
> > })
> >
> > #define xchg_acquire(ptr, x) \
> > ({ \
> > + __typeof__(*(ptr)) _x_ = (x); \
> > + __ret = __xchg_relaxed((ptr), _x_, sizeof(*(ptr))); \
> > + __acquire_fence(); \
> > __ret; \
> > })
> >
> > #define xchg_release(ptr, x) \
> > ({ \
> > __typeof__(*(ptr)) _x_ = (x); \
> > + __release_fence(); \
> > + (__typeof__(*(ptr))) __xchg_relaxed((ptr), \
> > _x_, sizeof(*(ptr))); \
> > })
> >
> > #define xchg(ptr, x) \
> > ({ \
> > + __typeof__(*(ptr)) __ret; \
> > __typeof__(*(ptr)) _x_ = (x); \
> > + __smp_mb(); \
> > + __ret = __xchg_relaxed((ptr), _x_, sizeof(*(ptr))); \
> > + __smp_mb(); \
> > + __ret; \
> > })
> >
> > /*
>
> Why are you defining *{,_acquire,_release}() at all, doesn't
> atomic-fallback.h DTRT for you?
Yes, you are right. I could reuse that. Thx
#define cmpxchg_acquire(...) \
__atomic_op_acquire(cmpxchg, __VA_ARGS__)
#endif
#define __atomic_op_acquire(op, args...) \
({ \
typeof(op##_relaxed(args)) __ret = op##_relaxed(args); \
__atomic_acquire_fence(); \
__ret; \
})
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
Powered by blists - more mailing lists