search for: device_regist

...; > > > 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,...

...; > > > 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,...

...- 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); -- 2.9.4

...- 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); -- 2.9.4

In order to make caller do a simple cleanup, we split device_register into device_initialize and device_add. device_initialize always succeeds, so the caller can always use put_device when register_virtio_device faild. Signed-off-by: weiping zhang <zhangweiping at didichuxing.com> Suggested-by: Cornelia Huck <cohuck at redhat.com> --- drivers/virtio/v...

On Wed, 20 Dec 2017 12:26:25 +0800 weiping zhang <zwp10758 at gmail.com> 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_sim...

On Wed, 20 Dec 2017 12:26:25 +0800 weiping zhang <zwp10758 at gmail.com> 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_sim...

...oned 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. > */ > virtio_register_device may fail before/after call device_register, the > caller should do a proper cleanup. Caller cann't use kfree directly, > if virtio_register_device has already called device_register. Caller > cann't use put_device directly, if virtio_register_device has not yet > call device_register, because kobject_put may give a war...

...oned 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. > */ > virtio_register_device may fail before/after call device_register, the > caller should do a proper cleanup. Caller cann't use kfree directly, > if virtio_register_device has already called device_register. Caller > cann't use put_device directly, if virtio_register_device has not yet > call device_register, because kobject_put may give a war...

In order to make caller do a simple cleanup, we split device_register into device_initialize and device_add. device_initialize always sucess, the caller can always use put_device when fail to register virtio_device no matter fail at ida_simple_get or at device_add. Signed-off-by: weiping zhang <zhangweiping at didichuxing.com> Suggested-by: Cornelia Huck &lt...

...- 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); -- 2.9.4

...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...

...ausing 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 de...

...- 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); -- 2.9.4

...uck at redhat.com>: > On Wed, 20 Dec 2017 12:26:25 +0800 > weiping zhang <zwp10758 at gmail.com> 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,&qu...

Hi, Patch1 add a helper to get virtio_device's status which will be used later. Patch2~4: check virtio_device's status is RTIO_CONFIG_S_ACKNOWLEDGE or not, if so use put_device otherwise use kfree. Patch5: add comments for virtio_register_device help caller do a proper cleanup if got failure. weiping zhang (5): virtio: add helper virtio_get_status virtio_pci: don't kfree device

Hi, Patch1 add a helper to get virtio_device's status which will be used later. Patch2~4: check virtio_device's status is RTIO_CONFIG_S_ACKNOWLEDGE or not, if so use put_device otherwise use kfree. Patch5: add comments for virtio_register_device help caller do a proper cleanup if got failure. weiping zhang (5): virtio: add helper virtio_get_status virtio_pci: don't kfree device

...6c22c91e340fdcdbcc7d82a51 adding these annotations went to stable, should this go there too? > Signed-off-by: weiping zhang <zhangweiping at didichuxing.com> > Reviewed-by: Cornelia Huck <cohuck at redhat.com> 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...

...6c22c91e340fdcdbcc7d82a51 adding these annotations went to stable, should this go there too? > Signed-off-by: weiping zhang <zhangweiping at didichuxing.com> > Reviewed-by: Cornelia Huck <cohuck at redhat.com> 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...

Hi, The main change is split device_register into 2 sperate calls: device_initalize() and device_add, and then the caller can use put_device safety when fail to register_virtio_device. v3->v4: * split device_register into device_initialize and devicea_add that the caller can always use put_device when fail to register virtio devi...