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;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Joel Fernandes <joel@joelfernandes.org>
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC 0/2] srcu: Remove pre-flip memory barrier
Date:   Tue, 20 Dec 2022 09:20:08 -0500
Message-Id: <CA83E649-8C79-4D39-9BFE-BBEF95968B98@joelfernandes.org>
References: <20221220140714.GB22763@lothringen>
Cc:     linux-kernel@vger.kernel.org,
        Josh Triplett <josh@joshtriplett.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        "Paul E. McKenney" <paulmck@kernel.org>, rcu@vger.kernel.org,
        Steven Rostedt <rostedt@goodmis.org>
In-Reply-To: <20221220140714.GB22763@lothringen>
To:     Frederic Weisbecker <frederic@kernel.org>
Precedence: bulk


> On Dec 20, 2022, at 9:07 AM, Frederic Weisbecker <frederic@kernel.org> wro=
te:
>=20
> =EF=BB=BFOn Tue, Dec 20, 2022 at 08:44:40AM -0500, Joel Fernandes wrote:
>>> C w-depend-r
>>>=20
>>> {
>>>   PLOCK=3DLOCK0;
>>> }
>>>=20
>>> // updater
>>> P0(int *LOCK1, int **PLOCK)
>>> {
>>>   int lock1;
>>>=20
>>>   lock1 =3D READ_ONCE(*LOCK1); // READ from inactive idx
>>>   smp_mb();
>>>   WRITE_ONCE(*PLOCK, LOCK1); // Flip idx
>>> }
>>>=20
>>> // reader
>>> P1(int **PLOCK)
>>> {
>>>   int *plock;
>>>=20
>>>   plock =3D READ_ONCE(*PLOCK);    // Read active idx
>>>   WRITE_ONCE(*plock, 1); // Write to active idx
>>=20
>> I am a bit lost here, why would the reader want to write to the active id=
x?
>> The reader does not update the idx, only the lock count.
>=20
> So &ssp->sda->srcu_lock_count is the base address and idx is the offset, r=
ight?
> The write is then displayed that way:
>=20
>     this_cpu_inc(ssp->sda->srcu_lock_count[idx].counter);
>=20
> But things could be also thought the other way around with idx being the b=
ase address and
> ssp->sda->srcu_lock_count being the offset.
>=20
>     this_cpu_inc(idx[ssp->sda->srcu_lock_count].counter);
>=20
> That would require to change some high level types but the result would be=
 the same from
> the memory point of view (and even from the ASM point of view). In the end=
 we
> are dealing with the same address and access.
>=20
> Now ssp->sda->srcu_lock_count is a constant address value. It doesn't chan=
ge.
> So it can be zero for example. Then the above increment becomes:
>=20
>   this_cpu_inc(idx.counter);
>=20
> And then it can be modelized as in the above litmus test.
>=20
> I had to play that trick because litmus doesn't support arrays but I belie=
ve
> it stands. Now of course I may well have got something wrong since I've al=
ways
> been terrible at maths...

Ah ok, I get where you were going with that. Yes there is address dependency=
 between reading idx and writing lock count. But IMHO, the access on the upd=
ate side is trying to order write to index, and reads from a lock count of a=
 previous index (as far as E / B+C is concerned). So IMHO, on the read side y=
ou have to consider 2 consecutive readers and not the same reader in order t=
o pair the same accesses correctly. But I could be missing something.

>> Further, the comment does not talk about implicit memory ordering, it=E2=80=
=99s talking about explicit ordering due to B+C on one side, and E on the ot=
her.
>=20
> Not arguing I'm also still confused by the comment...

;-)

Thanks,

 - Joel


>=20
> Thanks.