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().
Thanx, Paul
