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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

On Thu, Apr 21, 2022 at 1:03 AM Dan Lustig <dlustig@xxxxxxxxxx> wrote:
>
> On 4/20/2022 1:33 AM, Guo Ren wrote:
> > Thx Dan,
> >
> > On Wed, Apr 20, 2022 at 1:12 AM Dan Lustig <dlustig@xxxxxxxxxx> wrote:
> >>
> >> On 4/17/2022 12:51 AM, Guo Ren wrote:
> >>> Hi Boqun & Andrea,
> >>>
> >>> On Sun, Apr 17, 2022 at 10:26 AM Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
> >>>>
> >>>> On Sun, Apr 17, 2022 at 12:49:44AM +0800, Guo Ren wrote:
> >>>> [...]
> >>>>>
> >>>>> 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.
> >>>>>
> >>>>
> >>>> Can .aqrl order memory accesses before and after it (not against itself,
> >>>> against each other), i.e. act as a full memory barrier? For example, can
> >>> From the RVWMO spec description, the .aqrl annotation appends the same
> >>> effect with "fence rw, rw" to the AMO instruction, so it's RCsc.
> >>>
> >>> Not only .aqrl, and I think the below also could be an RCsc when
> >>> sc.w.aq is executed:
> >>> A: Pre-Access
> >>> B: lr.w.rl ADDR-0
> >>> ...
> >>> C: sc.w.aq ADDR-0
> >>> D: Post-Acess
> >>> Because sc.w.aq has overlap address & data dependency on lr.w.rl, the
> >>> global memory order should be A->B->C->D when sc.w.aq is executed. For
> >>> the amoswap
> >>
> >> These opcodes aren't actually meaningful, unfortunately.
> >>
> >> Quoting the ISA manual chapter 10.2: "Software should not set the rl bit
> >> on an LR instruction unless the aq bit is also set, nor should software
> >> set the aq bit on an SC instruction unless the rl bit is also set."
> > 1. Oh, I've missed the behind half of the ISA manual. But why can't we
> > utilize lr.rl & sc.aq in software programming to guarantee the
> > sequence?
>
> lr.aq and sc.rl map more naturally to hardware than lr.rl and sc.aq.
> Plus, they just aren't common operations to begin with, e.g., there
> is no smp_store_acquire() or smp_load_release(), nor are there
> equivalents in C/C++ atomics.
First, thx for pointing out that my patch violates the rules defined
in the ISA manual. I've abandoned these parts in v3.

It's easy to let hw support lr.rl & sc.aq (eg: our hardware supports
them). I agree there are no equivalents in C/C++ atomics. But they are
useful for LR/SC pairs to implement atomic_acqurie/release semantics.
Compare below:
A): fence rw, r; lr
B): lr.rl
The A has another "fence ,r" effect in semantics, it's over commit
from a software design view.

ps: Current definition has problems:
#define RISCV_ACQUIRE_BARRIER           "\tfence r , rw\n"
#define RISCV_RELEASE_BARRIER           "\tfence rw,  w\n"

#define __cmpxchg_release(ptr, old, new, size)                          \
...
                __asm__ __volatile__ (                                  \
                        RISCV_RELEASE_BARRIER                           \
                        "0:     lr.w %0, %2\n"                          \

That means "fence rw, w" can't prevent lr.w beyond the fence, we need
a "fence.rw. r" here. Here is the Fixup patch which I'm preparing:


[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux