Re: [PATCH net 2/2] net: avoid false positive warnings in __net_mp_close_rxq()

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 2 Apr 2025 16:24:28 -0700 Jakub Kicinski wrote:
> On Wed, 2 Apr 2025 11:52:50 -0700 Mina Almasry wrote:
> > >         netdev_lock(dev);
> > > -       __net_mp_close_rxq(dev, ifq_idx, old_p);
> > > +       /* Callers holding a netdev ref may get here after we already
> > > +        * went thru shutdown via dev_memory_provider_uninstall().
> > > +        */
> > > +       if (dev->reg_state <= NETREG_REGISTERED)
> > > +               __net_mp_close_rxq(dev, ifq_idx, old_p);    
> > 
> > Not obvious to me why this check was moved. Do you expect to call
> > __net_mp_close_rxq on an unregistered netdev and expect it to succeed
> > in io_uring binding or something?  
> 
> Yes, iouring state is under spin lock it can't call in here atomically.
> device unregister may race with iouring shutdown.
> 
> Now that I look at it I think we need
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index be17e0660144..0a70080a1209 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -11947,6 +11947,7 @@ void unregister_netdevice_many_notify(struct list_head *head,
>                 unlist_netdevice(dev);
>                 netdev_lock(dev);
>                 WRITE_ONCE(dev->reg_state, NETREG_UNREGISTERING);
> +               dev_memory_provider_uninstall(dev);
>                 netdev_unlock(dev);
>         }
>         flush_all_backlogs();
> @@ -11961,7 +11962,6 @@ void unregister_netdevice_many_notify(struct list_head *head,
>                 dev_tcx_uninstall(dev);
>                 netdev_lock_ops(dev);
>                 dev_xdp_uninstall(dev);
> -               dev_memory_provider_uninstall(dev);
>                 netdev_unlock_ops(dev);
>                 bpf_dev_bound_netdev_unregister(dev);
> 
> since 1d22d3060b9b ("net: drop rtnl_lock for queue_mgmt operations")
> we drop the lock after setting UNREGISTERING so we may call .uninstall
> after iouring torn down its side.
> 
> Right, Stan?

Actually, if I don't split the check here things will just work. 
Let me do that in v2. Thanks for flagging!