On Wed, 2017-12-27 at 15:56 -0700, Jason Gunthorpe wrote:
I'm a little more worried about the ib_device_register_sysfs - why was the WARN_ON ever there?
That WARN_ON() statement was introduced by commit 97a9ea848016 ("IB/core: Initialize ib_device.dev.parent earlier"):
[ ... ] diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c index c1fb545e8d78..daadf3130c9f 100644 --- a/drivers/infiniband/core/sysfs.c +++ b/drivers/infiniband/core/sysfs.c @@ -1258,7 +1258,7 @@ int ib_device_register_sysfs(struct ib_device *device, int ret; int i;
- device->dev.parent = device->dma_device; + WARN_ON_ONCE(!device->dev.parent); ret = dev_set_name(class_dev, "%s", device->name); if (ret) return ret; [ ... ]
Since ib_device_register_sysfs() does not use the parent pointer in any way I think it is sufficient to check the parent pointer in ib_register_device() and to leave out any parent pointer checks from ib_device_register_sysfs().
What do you think about this as a clarification Bart? [ ... ]
OK, I will add a parent pointer check in the code paths that dereference that pointer.
Bart.