linux-kernel - Re: [PATCH V2 0/3] riscv: atomic: Optimize AMO instructions usage

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]

Date:   Sun, 17 Apr 2022 00:49:44 +0800
From:   Guo Ren <guoren@...nel.org>
To:     Boqun Feng <boqun.feng@...il.com>,
        Andrea Parri <parri.andrea@...il.com>,
        Daniel Lustig <dlustig@...dia.com>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:     Arnd Bergmann <arnd@...db.de>, Palmer Dabbelt <palmer@...belt.com>,
        Mark Rutland <mark.rutland@....com>,
        Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        Guo Ren <guoren@...ux.alibaba.com>
Subject: Re: [PATCH V2 0/3] riscv: atomic: Optimize AMO instructions usage

Hi Boqun,

On Wed, Apr 13, 2022 at 11:46 PM Boqun Feng <boqun.feng@...il.com> wrote:
>
> [Cc Andrea]
>
> On Tue, Apr 12, 2022 at 11:49:54AM +0800, guoren@...nel.org wrote:
> > From: Guo Ren <guoren@...ux.alibaba.com>
> >
> > These patch series contain one cleanup and some optimizations for
> > atomic operations.
> >
>
> Seems to me that you are basically reverting 5ce6c1f3535f
> ("riscv/atomic: Strengthen implementations with fences"). That commit
> fixed an memory ordering issue, could you explain why the issue no
> longer needs a fix?

I'm not reverting the prior patch, just optimizing it.

In RISC-V “A” Standard Extension for Atomic Instructions spec, it said:
If only the aq bit is set, the atomic memory operation is treated as
an acquire access, i.e., no following memory operations on this RISC-V
hart can be observed to take place before the acquire memory
operation.
-                       "       amoswap.w %0, %2, %1\n"                 \
-                       RISCV_ACQUIRE_BARRIER                           \
+                       "       amoswap.w.aq %0, %2, %1\n"              \
So RISCV_ACQUIRE_BARRIER is "fence r, rw" and "fence r" is over
constraints to protect amoswap.w. Here using amoswap.w.aq is more
proper.

If only the rl bit is set, the atomic memory operation is treated as a
release access, i.e., the release memory operation cannot be observed
to take place before any earlier memory operations on this RISC-V
hart.
-                       RISCV_RELEASE_BARRIER                           \
-                       "       amoswap.w %0, %2, %1\n"                 \
+                       "       amoswap.w.rl %0, %2, %1\n"              \
So RISCV_RELEASE_BARRIER is "fence rw, w" and "fence ,w" is over
constraints to protect amoswap.w. Here using amoswap.w.rl is more
proper.

If both the aq and rl bits are set, the atomic memory operation is
sequentially consistent and cannot be observed to happen before any
earlier memory operations or after any later memory operations in the
same RISC-V hart and to the same address domain.
                "0:     lr.w     %[p],  %[c]\n"
                "       sub      %[rc], %[p], %[o]\n"
                "       bltz     %[rc], 1f\n".
-               "       sc.w.rl  %[rc], %[rc], %[c]\n"
+               "       sc.w.aqrl %[rc], %[rc], %[c]\n"
                "       bnez     %[rc], 0b\n"
-               "       fence    rw, rw\n"
                "1:\n"
So .rl + fence rw, rw is over constraints, only using sc.w.aqrl is more proper.

>
> Regards,
> Boqun
>
> > Changes in V2:
> >  - Fixup LR/SC memory barrier semantic problems which pointed by
> >    Rutland
> >  - Combine patches into one patchset series
> >  - Separate AMO optimization & LRSC optimization for convenience
> >    patch review
> >
> > Guo Ren (3):
> >   riscv: atomic: Cleanup unnecessary definition
> >   riscv: atomic: Optimize acquire and release for AMO operations
> >   riscv: atomic: Optimize memory barrier semantics of LRSC-pairs
> >
> >  arch/riscv/include/asm/atomic.h  | 70 ++++++++++++++++++++++++++++++--
> >  arch/riscv/include/asm/cmpxchg.h | 42 +++++--------------
> >  2 files changed, 76 insertions(+), 36 deletions(-)
> >
> > --
> > 2.25.1
> >



--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/