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
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.400.51.1.1\))
Subject: Re: [PATCH rcu 6/6] rcu: Use WRITE_ONCE() for assignments to ->next
 for rculist_nulls
From:   Alan Huang <mmpgouride@gmail.com>
In-Reply-To: <50bc6e0d-7d9b-4634-999a-2ee8f503e86a@paulmck-laptop>
Date:   Thu, 20 Jul 2023 03:17:58 +0800
Cc:     Joel Fernandes <joel@joelfernandes.org>, rcu@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel-team@meta.com,
        rostedt@goodmis.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <129BFB2B-1A19-437E-B04E-16B16AA4C838@gmail.com>
References: <6127192c-da9b-4599-9738-6e8f92e6c75c@paulmck-laptop>
 <20230717180317.1097590-6-paulmck@kernel.org>
 <9651c3b4-0791-5cd7-567f-f65d28bc8fae@joelfernandes.org>
 <BE64BF3F-69AD-4522-965E-0B7E4054D737@gmail.com>
 <3f6b04b3-af22-49f0-8d3c-269640f9f762@paulmck-laptop>
 <449d0d48-ac98-659b-4d03-e75c318605d4@joelfernandes.org>
 <50bc6e0d-7d9b-4634-999a-2ee8f503e86a@paulmck-laptop>
To:     "Paul E. McKenney" <paulmck@kernel.org>
Precedence: bulk


> 2023=E5=B9=B47=E6=9C=8820=E6=97=A5 02:20=EF=BC=8CPaul E. McKenney =
<paulmck@kernel.org> =E5=86=99=E9=81=93=EF=BC=9A
>=20
> On Tue, Jul 18, 2023 at 09:48:59PM -0400, Joel Fernandes wrote:
>>=20
>>=20
>> On 7/18/23 14:32, Paul E. McKenney wrote:
>>> On Tue, Jul 18, 2023 at 10:48:07PM +0800, Alan Huang wrote:
>>>>=20
>>>>> 2023=E5=B9=B47=E6=9C=8818=E6=97=A5 21:49=EF=BC=8CJoel Fernandes =
<joel@joelfernandes.org> =E5=86=99=E9=81=93=EF=BC=9A
>>>>>=20
>>>>> On 7/17/23 14:03, Paul E. McKenney wrote:
>>>>>> From: Alan Huang <mmpgouride@gmail.com>
>>>>>> When the objects managed by rculist_nulls are allocated with
>>>>>> SLAB_TYPESAFE_BY_RCU, old readers may still hold references to an =
object
>>>>>> even though it is just now being added, which means the =
modification of
>>>>>> ->next is visible to readers.  This patch therefore uses =
WRITE_ONCE()
>>>>>> for assignments to ->next.
>>>>>> Signed-off-by: Alan Huang <mmpgouride@gmail.com>
>>>>>> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
>>>>>=20
>>>>> Did we ever conclude that the READ_ONCE() counterparts were not =
needed? ;-)
>>>>=20
>>>> Read-side is already protected by rcu_dereference_raw() in =
hlist_nulls_for_each_entry_{rcu, safe}.
>>>=20
>>> It turns out that different traversal synchronization designs want
>>> different pointers using WRITE_ONCE().
>>=20
>> Thank you Alan and Paul,
>>=20
>> Btw, I don't see any users of hlist_nulls_unhashed_lockless(), maybe =
it can
>> be removed?
>=20
> Either that or the people who removed uses injected bugs...

It has never been used.

That said, the data race has been there almost for four years.

And the network people use sk_unhashed() for both hlist_node and =
hlist_nulls_node.
So, I plan to use hlist_unhashed_lockless() in sk_unhashed(), that will =
be one of my future patches.

>=20
> But if this one really does go away, do we need ->pprev to be
> protected by _ONCE()?

The ->pprev thing is what I=E2=80=99m currently working on. :)

>=20
> Thanx, Paul