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

[Florian Westphal <fw@strlen.de>]

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.