On Wed, 20 Dec 2017 12:26:25 +0800 weiping zhang <[email protected]> wrote:
[you used a different mail address in your From: than in your s-o-b:; same for the other patches] > In order to make caller do a simple cleanup, we split device_register > into device_initialize and device_add. device_initialize always sucess, s/success/succeeds/ > the caller can always use put_device when fail to register virtio_device "so the caller can always use put_device when register_virtio_device failed," > no matter fail at ida_simple_get or at device_add. "no matter whether it failed..." > > Signed-off-by: weiping zhang <[email protected]> > Suggested-by: Cornelia Huck <[email protected]> > --- > drivers/virtio/virtio.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c > index bf7ff39..3c9f211 100644 > --- a/drivers/virtio/virtio.c > +++ b/drivers/virtio/virtio.c > @@ -303,11 +303,21 @@ void unregister_virtio_driver(struct virtio_driver > *driver) > } > EXPORT_SYMBOL_GPL(unregister_virtio_driver); > > +/** > + * register_virtio_device - register virtio device > + * @dev : virtio device interested "virtio device to be registered" > + * > + * If an error occurs, the caller must use put_device, instead of kfree, > because > + * device_initialize and device_add will increase @dev->dev's reference > count. That's not correct: It's not because of device_add increasing the reference count (it releases it again on failure), but because another code path may have obtained a reference. What about: "On error, the caller must call put_device on &@dev->dev (and not kfree), as another code path may have obtained a reference to @dev." > + * > + * Returns: 0 on suceess, -error on failure > + */ > int register_virtio_device(struct virtio_device *dev) > { > int err; > > dev->dev.bus = &virtio_bus; > + device_initialize(&dev->dev); > > /* Assign a unique device index and hence name. */ > err = ida_simple_get(&virtio_index_ida, 0, 0, GFP_KERNEL); > @@ -330,9 +340,11 @@ int register_virtio_device(struct virtio_device *dev) > > INIT_LIST_HEAD(&dev->vqs); > > - /* device_register() causes the bus infrastructure to look for a > - * matching driver. */ > - err = device_register(&dev->dev); > + /* > + * device_add() causes the bus infrastructure to look for a matching > + * driver. FWIW, I would just have done s/device_register/device_add/ in the comment, but this is ok as well. > + */ > + err = device_add(&dev->dev); > if (err) > ida_simple_remove(&virtio_index_ida, dev->index); > out: Your code change is fine. _______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
