On Tue, Jul 11, 2023 at 10:56:02PM +0800, Alan Huang wrote:
>
> > 2023年7月11日 04:30，Paul E. McKenney <paulmck@xxxxxxxxxx> 写道：
> >
> > On Mon, Jul 10, 2023 at 04:01:07PM -0400, Joel Fernandes wrote:
> >> On Thu, Jul 6, 2023 at 6:29 AM Alan Huang <mmpgouride@xxxxxxxxx> wrote:
> >>>
> >>> When the objects managed by rculist_nulls are allocated with
> >>> SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
> >>> object that is being added, which means the modification of ->next
> >>> is visible to readers. So, this patch uses WRITE_ONCE() for assignments
> >>> to ->next.
> >>>
> >>> Signed-off-by: Alan Huang <mmpgouride@xxxxxxxxx>
> >>> ---
> >>> include/linux/rculist_nulls.h | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> >>> index ba4c00dd8005..89186c499dd4 100644
> >>> --- a/include/linux/rculist_nulls.h
> >>> +++ b/include/linux/rculist_nulls.h
> >>> @@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
> >>> {
> >>> struct hlist_nulls_node *first = h->first;
> >>>
> >>> - n->next = first;
> >>> + WRITE_ONCE(n->next, first);
> >>> WRITE_ONCE(n->pprev, &h->first);
> >>> rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
> >>> if (!is_a_nulls(first))
> >>> @@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
> >>> last = i;
> >>>
> >>> if (last) {
> >>> - n->next = last->next;
> >>> + WRITE_ONCE(n->next, last->next);
> >>> n->pprev = &last->next;
> >>> rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
> >>> } else {
> >>
> >> Don't you need READ_ONCE() for the read-side accesses as well?
> >
> > Both hlist_nulls_for_each_entry_rcu() and hlist_nulls_for_each_entry_safe()
> > use rcu_dereference_raw(). To your point, both hlist_nulls_first_rcu()
> > and hlist_nulls_next_rcu() look rather strange. I would have expected
> > them to also use rcu_dereference_raw().
>
> hlist_nulls_first_rcu() and hlist_nulls_next_rcu() were added in commit:
>
> 67bdbffd696f(“rculist: avoid __rcu annotations”)
>
> According to the commit message, this avoids warnings from missing __rcu annotations.
Indeed it does, apologies for the noise!
Thanx, Paul
