Hi Boqun,
On Wed, Apr 13, 2022 at 11:46 PM Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
>
> [Cc Andrea]
>
> On Tue, Apr 12, 2022 at 11:49:54AM +0800, guoren@xxxxxxxxxx wrote:
> > From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> >
> > 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/
