Re: [PATCH net 2/2] net: core: split unregister_netdevice list into smaller chunks

Florian Westphal
2025-10-11T14:30:08Z

Stanislav Fomichev <stfomichev@gmail.com> wrote:
> On 10/10, Florian Westphal wrote:
> > +static void unregister_netdevice_close_many_lockdep(struct list_head *head)
> > +{
> > +#ifdef CONFIG_LOCKDEP
> > + unsigned int lock_depth = lockdep_depth(current);
> > + unsigned int lock_count = lock_depth;
> > + struct net_device *dev, *tmp;
> > + LIST_HEAD(done_head);
> > +
> > + list_for_each_entry_safe(dev, tmp, head, unreg_list) {
> > + if (netdev_need_ops_lock(dev))
> > + lock_count++;
> > +
> > + /* we'll run out of lockdep keys, reduce size. */
> > + if (lock_count >= MAX_LOCK_DEPTH - 1) {
> > + LIST_HEAD(tmp_head);
> > +
> > + list_cut_before(&tmp_head, head, &dev->unreg_list);
> > + unregister_netdevice_close_many(&tmp_head);
> > + lock_count = lock_depth;
> > + list_splice_tail(&tmp_head, &done_head);
> > + }
> > + }
> > +
> > + unregister_netdevice_close_many(head);
> > +
> > + list_for_each_entry_safe_reverse(dev, tmp, &done_head, unreg_list)
> > + list_move(&dev->unreg_list, head);
> > +#else
> > + unregister_netdevice_close_many(head);
> > +#endif
>
>
> Any reason not to morph the original code to add this 'no more than 8 at a
> time' constraint? Having a separate lockdep path with list juggling
> seems a bit fragile.
>
> 1. add all ops locked devs to the list
> 2. for each MAX_LOCK_DEPTH (or 'infinity' in the case of non-lockdep)
> 2.1 lock N devs
> 2.2 netif_close_many
> 2.3 unlock N devs
> 3. ... do the non-ops-locked ones
>
> This way the code won't diverge too much I hope.

I think that having extra code for LOCKDEP (which means debug kernel
that often also includes k?san, kmemleak etc. is ok.

I was more concerned with having no changes to normal (non-lockdep)
kernel.

Let me try again, I tried to do your solution above before going with
this extra lockdep-only juggling but I ended up making a mess.