On Thu, Dec 06, 2018 at 04:58:16PM -0600, Steve Wise wrote: > > On 12/6/2018 4:21 PM, Jason Gunthorpe wrote: > > On Thu, Dec 06, 2018 at 03:16:57PM -0600, Steve Wise wrote: > >> Jason, This feature took a step back, imo, by using > >> ib_device_get_by_index() in nldev_dellink() and passing the struct > >> ib_device ptr vs simply passing the ibdev name string to the driver's > >> dellink() function and letting the driver deal with it all. I'm > >> inclined to go back to that approach. I don't see things getting > >> cleaner and simpler on this current path. > > It doesn't solve anything to have the driver do the lookup, it still > > has to have proper locking and prevent the races. If it can do that, > > then it can do it with an already looked-up device just the same. > > It is easier by just passing the name to the driver because the driver > can easily determine if the device has been removed via a > driver-specific lock and lookup. Looking up the device via > ib_device_get_by_index() uses a core-private mechanism that isn't public > to the driver, so it makes life more difficult. I could export > ib_device_put() but that doesn't seem clean to me. > > > I'm still wondering if there is some core functions we could create to > > handle this common case for dellink drivers. > > Please noodle on it - I could use your help. Thanks... Maybe the simplest is to have dellink return 0 and then the core code directly does the unregister. Then the core code can handle the special locking with internal locks. To make this work we probably need to add a 'close' callback into ib_device so that the core can sequence destruction properly - ie everything that happens in the driver after unregister should be done in close. This is sort of what netdev does. Jason
