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

Stanislav Fomichev
2025-03-07T23:50:29Z

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..