On Thu, 14 Dec 2017 21:13:28 +0200 "Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote: > On Tue, Dec 12, 2017 at 09:24:02PM +0800, weiping zhang wrote: > > As mentioned at drivers/base/core.c: > > /* > > * 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. > > */ > > so we don't free vp_dev until vp_dev->vdev.dev.release be called. > > seeing as 5739411acbaa63a6c22c91e340fdcdbcc7d82a51 adding these > annotations went to stable, should this go there too? > > > Signed-off-by: weiping zhang <zhangweiping@xxxxxxxxxxxxxxx> > > Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx> > > OK but this relies on users knowing that register_virtio_device > calls device_register. I think we want to add a comment > to register_virtio_device. > > Also the cleanup is uglified. > > I really think the right thing would be to change device_register making > it safe to kfree. People have the right to expect register on failure to > have no effect. > > That just might be too hard to implement though. Yes. The main problem is that device_register() at some point makes the structure visible to others, at which point they may obtain a reference. If that happened, you cannot clean up unless that other party gave up their reference -- which means your only chance to get this right is the current put_device() approach. It *is* problematic if all of that stuff is hidden behind too many calling layers. If you have the device_initialize() -> device_add() calling sequence, having to do a put_device() on failure is much more obvious. But as you usually don't pass in a pure struct device but something embedding it, the put_device() needs to be done on the outermost level. Commenting can help here, as would probably a static checker for that code pattern. _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
