Re: [PATCH rcu 6/6] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

From: Alan Huang
Date: Wed Jul 19 2023 - 15:18:21 EST



> 2023年7月20日 02:20,Paul E. McKenney <paulmck@xxxxxxxxxx> 写道:
>
> On Tue, Jul 18, 2023 at 09:48:59PM -0400, Joel Fernandes wrote:
>>
>>
>> On 7/18/23 14:32, Paul E. McKenney wrote:
>>> On Tue, Jul 18, 2023 at 10:48:07PM +0800, Alan Huang wrote:
>>>>
>>>>> 2023年7月18日 21:49,Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> 写道:
>>>>>
>>>>> On 7/17/23 14:03, Paul E. McKenney wrote:
>>>>>> From: Alan Huang <mmpgouride@xxxxxxxxx>
>>>>>> 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@xxxxxxxxx>
>>>>>> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
>>>>>
>>>>> Did we ever conclude that the READ_ONCE() counterparts were not needed? ;-)
>>>>
>>>> Read-side is already protected by rcu_dereference_raw() in hlist_nulls_for_each_entry_{rcu, safe}.
>>>
>>> It turns out that different traversal synchronization designs want
>>> different pointers using WRITE_ONCE().
>>
>> Thank you Alan and Paul,
>>
>> Btw, I don't see any users of hlist_nulls_unhashed_lockless(), maybe it can
>> be removed?
>
> 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.

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

The ->pprev thing is what I’m currently working on. :)

>
> Thanx, Paul