[Open-FCoE] [PATCH V2 11/13] fcoe: add mutex to protect create and destroy

Robert Love robert.w.love at intel.com
Wed Jul 8 21:43:12 UTC 2009


On Wed, 2009-07-08 at 14:24 -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.
> 
My understanding of rtnl_lock is that it's used all over the kernel for
a lot of networking related things. The comment says,

"/* RTNL is used as a global lock for all changes to network
configuration */"

I don't think it would be a layering violation. I talked to a few
networking engineers here and they seemed to think it would be a
reasonable thing to do.

It's used for a lot of stuff too. From the Linux cross-referencer-

"231 references in 93 files."

I poked around some of those references and although I didn't
immediately find anything as analogous as I wished I bet I could find
rtnl_lock used the way we want to with a more serious search.

> 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 didn't think it that far out, but that would be great.





More information about the devel mailing list