On Tue, 15 Feb 2022 12:22:14 +0800 Matt Johnston wrote: > On Fri, 2022-02-11 at 14:38 -0800, Jakub Kicinski wrote: > > > > > +// Removes and unregisters a mctp-i2c netdev > > > +static void mctp_i2c_free_netdev(struct mctp_i2c_dev *midev) > > > > > You're doing a lot before the unregister call, this is likely racy. > > The usual flow is to unregister the netdev, then do uninit, then free. > > For instance you purge the queue but someone may Tx afterwards. > > needs_free_netdev is a footgun. > > Thanks Jakub. I've reworked it here to do the work before register/after > unregister, without needs_free_netdev. > > One question, the tx thread calls netif_wake_queue() - is it safe to call > that after unregister_netdev()? (before free_netdev) I don't think so. > I've moved the kthread_stop() to the post-unregister cleanup. The usual way to deal with Tx would be to quiesce the worker in ndo_stop. Maybe keep it simple and add a mutex around the worker? You can then take the same mutex around: stop queue purge queue Thanks to the mutex you'd know the worker is not running and as long as worker does its !skb_queue_empty() under the same mutex it will not wake the queue.
