Re: [PATCH nf 1/3] rculist: add list_splice_rcu() for private lists

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Paul,

Thanks for your review.

On Tue, Apr 14, 2026 at 04:35:57PM -0700, Paul E. McKenney wrote:
> On Tue, Apr 14, 2026 at 12:04:15AM +0200, Pablo Neira Ayuso wrote:
> > This patch adds a helper function, list_splice_rcu(), to safely splice
> > a private (non-RCU-protected) list into an RCU-protected list.
> > 
> > The function ensures that only the pointer visible to RCU readers
> > (prev->next) is updated using rcu_assign_pointer(), while the rest of
> > the list manipulations are performed with regular assignments, as the
> > source list is private and not visible to concurrent RCU readers.
> > 
> > This is useful for moving elements from a private list into a global
> > RCU-protected list, ensuring safe publication for RCU readers.
> > Subsystems with some sort of batching mechanism from userspace can
> > benefit from this new function.
> > 
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> Looks plausible and useful.  Please see some comments inline below.

Thanks, see below.

> > ---
> > @I need this to fix a unsafe list_splice() of a private list to an
> > existing RCU-protected list. This is based on an existing idiom in
> > __list_splice_init_rcu().
> > 
> >  include/linux/rculist.h | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> > index 2abba7552605..3c18c3336459 100644
> > --- a/include/linux/rculist.h
> > +++ b/include/linux/rculist.h
> > @@ -261,6 +261,41 @@ static inline void list_replace_rcu(struct list_head *old,
> >  	old->prev = LIST_POISON2;
> >  }
> >  
> > +/**
> > + * __list_splice_rcu - join a non-RCU list into an existing list.
> > + * @list:	the RCU-protected list to splice
> 
> This is actually not RCU-protected, correct?  Sure, its elements
> (aside from the list header) are RCU-protected upon exit from this
> function, but by then they are in the {@prev,@next} list, not in
> this @list.

Correct, I can fix this.

> > + * @prev:	points to the last element of the existing list
> > + * @next:	points to the first element of the existing list
> > + *
> > + * The list pointed to by @prev and @next can be RCU-read traversed
> > + * concurrently with this function.
> 
> Doesn't this last sentence also need to go into the list_splice_rcu()
> function's kernel-doc header?  But please see below.

OK.

> > + */
> > +static inline void __list_splice_rcu(struct list_head *list,
> > +				     struct list_head *prev,
> > +				     struct list_head *next)
> > +{
> > +	struct list_head *first = list->next;
> > +	struct list_head *last = list->prev;
> > +
> > +	last->next = next;
> > +	rcu_assign_pointer(list_next_rcu(prev), first);
> > +	first->prev = prev;
> > +	next->prev = last;
> 
> Although putting these last two after the rcu_assign_pointer() is safe,
> given that RCU readers do not traverse ->prev pointers, it would be
> better to place them before the rcu_assign_pointer() in order to avoid
> false sharing between this code path and any concurrent RCU readers.

I can do that.

> > +}
> > +
> > +/**
> > + * list_splice_rcu - splice a non-RCU list into an RCU-protected list,
> > + *                   designed for stacks.
> > + * @list:	the non RCU-protected list to splice
> > + * @head:	the place in the existing list to splice the first list into
> 
> Please add something about @head being RCU-protected.

Will do.

> > + */
> > +static inline void list_splice_rcu(struct list_head *list,
> > +				   struct list_head *head)
> > +{
> > +	if (!list_empty(list))
> > +		__list_splice_rcu(list, head, head->next);
> > +}
> 
> I don't understand the purpose of having __list_splice_rcu() split out
> from list_splice_rcu().  If you are planning to add more callers of
> __list_splice_rcu(), you can always do the split when you add the first
> such caller.  In the meantime, why the extra code?

I only have a use-case for list_splice_rcu(), so OK, single function
is fine with.

> Yes, we do have __list_splice_init_rcu(), but that is because it is
> called from both list_splice_init_rcu() and list_splice_tail_init_rcu().

Understood.

I will be posting a v2 asap.

Thanks!

> > +
> >  /**
> >   * __list_splice_init_rcu - join an RCU-protected list into an existing list.
> >   * @list:	the RCU-protected list to splice
> > -- 
> > 2.47.3
> >