On 11/29/2018 10:54 PM, Jason Gunthorpe wrote: > On Fri, Nov 30, 2018 at 01:29:22AM +0000, Ruhl, Michael J wrote: > >> Should the mutex be held while calling the callback? If the newlink or dellink >> callbacks need to sleep (probably dellink specifically), I think holding the mutex >> might not be a good idea. > It would be a bit better if this lock was a rwsem, but it must be held > whenever ops is touched.. This is probably a moot point though when > dellink goes through the ib_dev not the ops list.. > I'm not sure what you mean, but my current code under test keeps the rdma_link_ops pointer cached in the struct ib_device. Then nldev_dellink() looks like this: + device = ib_device_get_by_index(index); + if (!device) + return -EINVAL; + + mutex_lock(&link_ops_mutex); + + /* Deref the device before deleting it. Otherwise we + * deadlock unregistering the device. + */ + ib_device_put(device); + + if (device->link_ops) + err = device->link_ops->dellink(device); + else + err = -ENODEV; + + mutex_unlock(&link_ops_mutex); I still think the mutex is needed... Note I had to call ib_device_put() -before- calling the driver dellink function or we deadlocked doing the device unregister. The above code doesn't really look good. Any thoughts on a better way? Steve.
