On Wed, Nov 29, 2017 at 10:50:44AM +0100, Cornelia Huck wrote: > On Wed, 29 Nov 2017 09:23:01 +0800 > weiping zhang <zwp10758@xxxxxxxxx> wrote: > > > index can be reused by other virtio device. > > > > Signed-off-by: weiping zhang <zhangweiping@xxxxxxxxxxxxxxx> > > Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx> > > > --- > > drivers/virtio/virtio.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c > > index 48230a5..bf7ff39 100644 > > --- a/drivers/virtio/virtio.c > > +++ b/drivers/virtio/virtio.c > > @@ -333,6 +333,8 @@ int register_virtio_device(struct virtio_device *dev) > > /* device_register() causes the bus infrastructure to look for a > > * matching driver. */ > > err = device_register(&dev->dev); > > + if (err) > > + ida_simple_remove(&virtio_index_ida, dev->index); > > out: > > if (err) > > virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED); > > I think your patch is correct (giving up the index if we failed to > register), but this made me look at our error handling if a virtio > device failed to register. > > 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. -- Thanks weiping _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
