Re: [PATCH net-next v1 4/4] net: drop rtnl_lock for queue_mgmt operations

[Stanislav Fomichev]

On 03/07, Jakub Kicinski wrote:
> On Fri,  7 Mar 2025 07:57:25 -0800 Stanislav Fomichev wrote:
> > All drivers that use queue API are already converted to use
> > netdev instance lock. Move netdev instance lock management to
> > the netlink layer and drop rtnl_lock.
> 
> > @@ -860,12 +854,11 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
> >  	}
> >  
> >  	mutex_lock(&priv->lock);
> > -	rtnl_lock();
> >  
> > -	netdev = __dev_get_by_index(genl_info_net(info), ifindex);
> > +	netdev = netdev_get_by_index_lock(genl_info_net(info), ifindex);
> >  	if (!netdev || !netif_device_present(netdev)) {
> >  		err = -ENODEV;
> > -		goto err_unlock;
> > +		goto err_unlock_sock;
> >  	}
> >  
> >  	if (dev_xdp_prog_count(netdev)) {
> > @@ -918,14 +911,15 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
> >  	if (err)
> >  		goto err_unbind;
> >  
> > -	rtnl_unlock();
> > +	netdev_unlock(netdev);
> 
> Ah, here's the unlock :)

mutex_unlock(&priv->lock) is still missing :(

> Looks good for the devmem binding, I think, the other functions will
> need a bit more careful handling. So perhaps drop the queue get changes?
> I'm cooking some patches for the queue get and queue stats.
> AFAIU we need helpers which will go over netdevs and either take rtnl
> lock or instance lock, depending on whether the driver is "ops locked"

Here is what I was tentatively playing with (rtnl_netdev_{,un}lock_ops
abomination):
https://github.com/fomichev/linux/commit/f791a23c358c7db0e798bc4181dc6c243c8ff77d

Which sort of does what you're suggesting in:
https://github.com/fomichev/linux/commit/392ae1f3ca823dc412a2dac2263b6c8355f6925d

Although I'm still unconditionally holding rtnl_lock during
for_each_netdev_dump..