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;
Date:   Sun, 17 Apr 2022 14:30:50 +0800
From:   Boqun Feng <boqun.feng@gmail.com>
To:     Guo Ren <guoren@kernel.org>
Cc:     Andrea Parri <parri.andrea@gmail.com>,
        Daniel Lustig <dlustig@nvidia.com>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        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>
Subject: Re: [PATCH V2 0/3] riscv: atomic: Optimize AMO instructions usage
Message-ID: <Ylu0GqNmYmCnpv9Z@tardis>
References: <20220412034957.1481088-1-guoren@kernel.org>
 <YlbwOG46mCR8Q5tJ@tardis>
 <CAJF2gTRws6RqKmJHBdKsycWSkFgYna_MocJ+qp3Z9r1v7mQzsg@mail.gmail.com>
 <Ylt6zqPgimmKpJzg@tardis>
 <CAJF2gTTZnBh_z31VK81cYiBrTt5NRVpSahoPh35Zo4Ns5hCv7A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="eKl0vpmQLAWLx6Dq"
Content-Disposition: inline
In-Reply-To: <CAJF2gTTZnBh_z31VK81cYiBrTt5NRVpSahoPh35Zo4Ns5hCv7A@mail.gmail.com>
Precedence: bulk


--eKl0vpmQLAWLx6Dq
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Apr 17, 2022 at 12:51:38PM +0800, Guo Ren wrote:
> Hi Boqun & Andrea,
>=20
> On Sun, Apr 17, 2022 at 10:26 AM Boqun Feng <boqun.feng@gmail.com> 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 mo=
re 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.
>=20

Thanks for the confirmation, btw, where can I find the RVWMO spec?

> 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
>=20
> The purpose of the whole patchset is to reduce the usage of
> independent fence rw, rw instructions, and maximize the usage of the
> .aq/.rl/.aqrl aonntation of RISC-V.
>=20
>                 __asm__ __volatile__ (                                  \
>                         "0:     lr.w %0, %2\n"                          \
>                         "       bne  %0, %z3, 1f\n"                     \
>                         "       sc.w.rl %1, %z4, %2\n"                  \
>                         "       bnez %1, 0b\n"                          \
>                         "       fence rw, rw\n"                         \
>                         "1:\n"                                          \
>=20
> > we end up with u =3D=3D 1, v =3D=3D 1, r1 on P0 is 0 and r1 on P1 is 0,=
 for the
> > following litmus test?
> >
> >     C lr-sc-aqrl-pair-vs-full-barrier
> >
> >     {}
> >
> >     P0(int *x, int *y, atomic_t *u)
> >     {
> >             int r0;
> >             int r1;
> >
> >             WRITE_ONCE(*x, 1);
> >             r0 =3D atomic_cmpxchg(u, 0, 1);
> >             r1 =3D READ_ONCE(*y);
> >     }
> >
> >     P1(int *x, int *y, atomic_t *v)
> >     {
> >             int r0;
> >             int r1;
> >
> >             WRITE_ONCE(*y, 1);
> >             r0 =3D atomic_cmpxchg(v, 0, 1);
> >             r1 =3D READ_ONCE(*x);
> >     }
> >
> >     exists (u=3D1 /\ v=3D1 /\ 0:r1=3D0 /\ 1:r1=3D0)
> I think my patchset won't affect the above sequence guarantee. Current
> RISC-V implementation only gives RCsc when the original value is the
> same at least once. So I prefer RISC-V cmpxchg should be:
>=20
>=20
> -                       "0:     lr.w %0, %2\n"                          \
> +                      "0:     lr.w.rl %0, %2\n"                         =
 \
>                         "       bne  %0, %z3, 1f\n"                     \
>                         "       sc.w.rl %1, %z4, %2\n"                  \
>                         "       bnez %1, 0b\n"                          \
> -                       "       fence rw, rw\n"                         \
>                         "1:\n"                                          \
> +                        "       fence w, rw\n"                    \
>=20
> To give an unconditional RSsc for atomic_cmpxchg.
>=20

Note that Linux kernel doesn't require cmpxchg() to provide any order if
cmpxchg() fails to update the memory location. So you won't need to
strengthen the atomic_cmpxchg().

Regards,
Boqun

> >
> > Regards,
> > Boqun
>=20
>=20
>=20
> --=20
> Best Regards
>  Guo Ren
>=20
> ML: https://lore.kernel.org/linux-csky/

--eKl0vpmQLAWLx6Dq
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCAAdFiEEj5IosQTPz8XU1wRHSXnow7UH+rgFAmJbtBcACgkQSXnow7UH
+rhsSgf/Yes71uP9q+qTNbrCUmjS5/wiBJPUbWlgx7JKTXgawyX72Raimw4rjlFo
9UT8KiZPQ76BEEd74fq8paTgy4GHu8XzAdtOQBPJ/CbAkfuDc0Z9/4e4ZwsXDaBc
/a6krWcGbouzS+2RVi/k0G6+EXbz8cBY6ORQ//7nseylrzaDcZtA7oAEx09PpVUq
7C8p/O+JLlCRj+W/CgOew3j1yK10HZ6BGkohfIM92r6GA3ArlHvuGmgYMrO0xuzZ
PZTx3HDwZbh98wiwBc3/xL+CnH8R7ffS9eTHvNO2GWebwIAiP7a/iQPbvGvyhwrf
xGcVLQRVB3NzKuTCASxcpR91L+r9FA==
=pKCs
-----END PGP SIGNATURE-----

--eKl0vpmQLAWLx6Dq--