linux-kernel - Re: [PATCH net 2/2] net: core: split unregister

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aOpp7n2E9ZVS6RJh@strlen.de>
Date: Sat, 11 Oct 2025 16:30:06 +0200
From: Florian Westphal <fw@...len.de>
To: Stanislav Fomichev <stfomichev@...il.com>
Cc: netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org,
	sdf@...ichev.me
Subject: Re: [PATCH net 2/2] net: core: split unregister_netdevice list into
 smaller chunks

Stanislav Fomichev <stfomichev@...il.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.