On Tue, Jun 11, 2024 at 06:39:55PM +0200, Christoph Hellwig wrote:
> On Tue, Jun 11, 2024 at 08:20:55AM -0700, Keith Busch wrote:
> > mutex_lock(&ctrl->namespaces_lock);
> > list_for_each_entry_safe(ns, next, &ctrl->namespaces, list) {
> > - if (ns->head->ns_id > nsid)
> > - list_splice_init_rcu(&ns->list, &rm_list,
> > - synchronize_rcu);
> > + if (ns->head->ns_id > nsid) {
> > + list_del_rcu(&ns->list);
> > + list_add_tail_rcu(&ns->list, &rm_list);
> > + }
>
> Is this actually valid for a (S)RCU protected list? If the entry gets
> added to the new list before the grace period has completed, we could
> trick a concurrent traversal into following the new list unless I'm
> mistaken (although chances I'm mistaken on RCU corner cases aren't that
> low..).
Good call, you are absolutely right that a sync should happen between
the del and the add for readers to consistently iterate this list.
I might be able to weasel out of this though: our namespace list is
sorted, and this function wants to append everything from this element
all the way to the end to the "rm_list": a reader should get the same
result either way, whether if it was torn at this element or the move
happened without the reader seeing it.
Now if there was a way to rcu split the list so we don't have to do it
element by element...
