2017-12-04 17:38 GMT+08:00 Cornelia Huck [off-list ref]:

On Sat, 2 Dec 2017 00:55:39 +0800
weiping zhang [off-list ref] wrote:

quoted

On Wed, Nov 29, 2017 at 10:50:44AM +0100, Cornelia Huck wrote:

quoted

quoted

We hold an extra reference to the struct device, even after a failed
register, and it is the responsibility of the caller to give up that
reference once no longer needed. As callers toregister_virtio_device()
embed the struct virtio_device, it needs to be their responsibility.
Looking at the existing callers,

- ccw does a put_device
- pci, mmio and remoteproc do nothing, causing a leak
- vop does a free on the embedding structure, which is a big no-no

Thoughts?

Sorry to relay late and thanks for your review.
Do you mean the "extra reference to the struct device" caused by the
following code?

err = device_register(&dev->dev);
      device_add(dev)
              get_device(dev)
If I'm understand right, I think there is no extra reference if we fail
virtio_register_device, because if device_register we don't get a
reference.

The device_initialize() already gives you a reference. If device_add()
fails, it has cleaned up any additional reference it might have
obtained, but the initial reference is still there and needs to be
released by the caller.

Thanks your clarify, I also notice the comments at device_register,
device_initialize, device_add,

 * NOTE: _Never_ directly free @dev after calling this function, even
 * if it returned an error! Always use put_device() to give up the
 * reference initialized in this function instead.

--
Thanks
weiping