On Thu, Jun 13, 2024 at 07:43:35AM -0700, Paul E. McKenney wrote: > On Thu, Jun 13, 2024 at 08:36:44AM -0600, Keith Busch wrote: > > On Thu, Jun 13, 2024 at 07:11:52PM +0530, Nilay Shroff wrote: > > > On 6/13/24 18:26, Keith Busch wrote: > > > > But that's not the problem for the rcu case. It's the last line that's > > > > the problem: > > > > > > > > list->prev->next = list; > > > > > > > > We can't change forward pointers for any element being detached from > > > > @head because a reader iterating the list may see that new pointer value > > > > and end up in the wrong list, breaking iteration. A synchronize rcu > > > > needs to happen before forward pointers can be mucked with, so it still > > > > needs to be done in two steps. Oh bother... > > > > > > Agree and probably we may break it down using this API: > > > static inline void list_cut_rcu(struct list_head *list, > > > struct list_head *head, struct list_head *entry, > > > void (*sync)(void)) > > > { > > > list->next = entry; > > > list->prev = head->prev; > > > __list_del(entry->prev, head); > > > sync(); > > > entry->prev = list; > > > list->prev->next = list; > > > } > > > > Yes, that's the pattern, but I think we need an _srcu() variant: the > > "sync" callback needs to know the srcu_struct. > > Just make a helper function like this: > > static void my_synchronize_srcu(void) > { > synchronize_srcu(&my_srcu_struct); > } > > Or am I missing something subtle here? That would work if we had a global srcu, but the intended usage dynamically allocates one per device the driver is attached to, so a void callback doesn't know which one to sync.