lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ