On Fri, Nov 30, 2018 at 02:42:39PM -0600, Steve Wise wrote: > > > On 11/30/2018 2:18 PM, Jason Gunthorpe wrote: > > On Fri, Nov 30, 2018 at 02:18:00PM -0600, Steve Wise wrote: > >> > >> On 11/30/2018 2:12 PM, Jason Gunthorpe wrote: > >>> On Fri, Nov 30, 2018 at 02:07:52PM -0600, Steve Wise wrote: > >>> > >>>> So perhaps nldev_dellink() should just lookup the ib_device by index > >>>> using ib_device_get_by_index(), note the device name (copy it > >>>> locally) > >>> That just creates races with rename. > >>> > >>> The API to the driver should be pointer based and has to hold only the > >>> kref not the refcnt > >>> > >>> Jason > >> What kref? I don't see any kref in struct ib_device. /me confused... > > It is inside the embedded struct device > > > > Jason > > How's this look? > > + device = ib_device_get_by_index(index); > + if (!device) > + return -EINVAL; > + > + ops = device->link_ops; > + > + /* > + * Deref the ib_device before deleting it. Otherwise we > + * deadlock unregistering the device. Hold a ref on the > + * underlying dma_device though to keep the memory around > + * until we're done. > + */ > + dma_device = get_device(device->dma_device); > + ib_device_put(device); > + err = ops ? ops->dellink(device) : -ENODEV; > + put_device(dma_device); This is OK I was thinking about the unload problem, and I wonder if the simple solution is to add something like ib_unregister_fence() which only returns once the device is unregistered and internaly waits for any other parrallel unregisters that may be running. Not sure the locking for that would be any fun though.. Jason
