On Thu, Jul 09, 2009 at 05:39:24PM -0700, Joe Eykholt wrote: > I forgot that you're now trying to destroy from the notifier. > > Is that really necessary? I can see how it's handy, but making > the administrator shut down the fcoe instances or do 'rmmod fcoe' > isn't so bad and might actually prevent accidents. > > I don't think there's a clean way of doing the destroy from the notifier > because of this flush issue and the callbacks you mentioned. What could > be possible is to disassociate with the netdev somehow and let the > destroy happen asynchronously later or just leave fcoe around but > disabled. > > If we did arrange to do a work item to do the destroy, would that > let the rmmod of the NIC driver work? Does rmmod wait long enough > for the work item to do the destroy? I guess you must've tried this > and had it work. > > I'd vote for scrapping the ability to unload the NIC driver while fcoe is > using it.
The main problem I see with this is the belief that you can prevent the netdev from going away (the device itself, not the module). Even with the driver module referenced to prevent unloading, it is possible to hot remove the device itself. Regardless, the locking around trying to solve all of these issues is certainly more complicated than I thought at first. I think it might be worth a recap of what's been learned recently. 1) The RTNL mutex cannot be held over the entire path of an FCoE interface destroy, because the destroy path needs to flush several work queue items. The exact rules to keep in mind here are: a) You can never call flush_scheduled_work() or flush_work() on the default/event queue while holding RTNL. The linkwatch events take RTNL in the workqueue thread. b) You can call cancel_work_sync() holding RTNL, if the work item you are canceling does not take RTNL. "b" doesn't help us because the FIP receive work might call fcoe_update_src_mac() which takes RTNL. 2) Using a new mutex in the fcoe module does allow for proper managing of create, destroy, module load and module unload. If it were just these cases, it might be possible to remove the hostlist lock and rely on just this mutex. This still leaves some open issues in trying to handle netdev unregistered events: a) Because the netdev notifier is called with RTNL held, the interface destroy path cannot be called directly from here. b) If some sort of delayed destroy were used here, the device should at least be removed from the hostlist right away to prevent racing between multiple destroy calls from different causes. That causes the hostlist lock to remain separate from the new create/destroy mutex in order to prevent circular locking issues with RTNL. c) The destroy can't be made asynchronous using schedule_work(), because it then attempts to flush the very workqueue thread it's running in. It might be possible if it is safe to change all those flushes to cancel_work_sync(). - Chris _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
