Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220412034957.1481088-1-guoren@kernel.org> <YlbwOG46mCR8Q5tJ@tardis>
In-Reply-To: <YlbwOG46mCR8Q5tJ@tardis>
From:   Guo Ren <guoren@kernel.org>
Date:   Sun, 17 Apr 2022 00:49:44 +0800
Message-ID: <CAJF2gTRws6RqKmJHBdKsycWSkFgYna_MocJ+qp3Z9r1v7mQzsg@mail.gmail.com>
Subject: Re: [PATCH V2 0/3] riscv: atomic: Optimize AMO instructions usage
To:     Boqun Feng <boqun.feng@gmail.com>,
        Andrea Parri <parri.andrea@gmail.com>,
        Daniel Lustig <dlustig@nvidia.com>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc:     Arnd Bergmann <arnd@arndb.de>, Palmer Dabbelt <palmer@dabbelt.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        linux-arch <linux-arch@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        Guo Ren <guoren@linux.alibaba.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Boqun,

On Wed, Apr 13, 2022 at 11:46 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> [Cc Andrea]
>
> On Tue, Apr 12, 2022 at 11:49:54AM +0800, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.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 =E2=80=9CA=E2=80=9D Standard Extension for Atomic Instructions sp=
ec, 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 pro=
per.

>
> 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/