On Wed, Jul 08, 2009 at 03:14:54PM -0700, Joe Eykholt wrote: > Chris Leech wrote: > > On Wed, Jul 08, 2009 at 02:24:16PM -0700, Joe Eykholt wrote: > >> Robert Love wrote: > >>> On Wed, 2009-07-08 at 10:40 -0700, Chris Leech wrote: > >>>> Rather than rely on the hostlist_lock to be held while creating exchange > >>>> managers, serialize fcoe instance creation and destruction with a mutex. > >>>> This will allow the hostlist addition to be moved out of > >>>> fcoe_if_create(), > >>>> which will simplify NPIV support. > >>>> > >>> Is there a reason that we don't use the rtnl_lock for protection? It's > >>> already held when we get the NETDEV_UNREGISTER event (added in patch > >>> 13/13). I believe that doing so would remove the need to defer the > >>> fcoe_destroy() call since the deferral is only needed so that we don't > >>> grab the rtnl_lock twice. > >> I considered that, too. It seems a bit like a layering violation to use > >> it to protect create/delete/exit, but maybe it's OK to do since we > >> need to grab rtnl_lock anyway. > >> > >> In fcoe_exit() we would have to be careful because > >> unregister_netdevice_notifier() does rtnl_lock(), > >> but if we did the unregister before grabbing rtnl_lock, > >> that would be OK. > >> > >> It could completely eliminate the need for hostlist_lock and create_lock, > >> so it seems like a nice idea. > > > > I thought there was somewhere outside of create/destroy that the > > host-list was being read, but it might just have been the module exit > > case. Removing the lock and only having one mutex for all this would be > > cool. > > The only other usage besides create/destroy is the netdev notification, > and that is called with rntl_lock held already. Nice. > > This also takes care of a worry I had that the notification would > find the fcoe_softc and be using it while a delete goes on. That > can't happen currently because rtnl_lock is held in the notification > and would have to be taken at least once in fcoe_destroy(). Since > we found the softc on the list at the beginning of the notification, > we're sure fcoe_destroy() hasn't removed it yet, and won't completely > free it until it gets rtnl_lock. So, to a small degree, > create/delete are already protected by rtnl_lock. > > I think this would be a safe and clean change. So, Chris, will > you work that into your patch set?
I have new versions of the last 3 patches changed to work this way. Let me do a little testing and then I'll send them out for review. _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
