2017-12-21 0:12 GMT+08:00 Cornelia Huck <cohuck@xxxxxxxxxx>:
> On Wed, 20 Dec 2017 12:27:33 +0800
> weiping zhang <zwp10758@xxxxxxxxx> wrote:
>
>> rproc_virtio_dev_release will be called iff virtio_device.dev's
>> refer count became to 0. Here we should check if we call device_register
>
> "reference count drops to 0"
>
> s/call/called/
rproc_add_virtio_dev is not like other virtio driver, it always call
register_virtio_device,
so commit message would be:
virtio_remoteproc: correct put_device virtio_device.dev
rproc_virtio_dev_release will be called iff virtio_device.dev's
reference count drops to 0. Here we just put vdev.dev, and then
rproc->dev's cleanup will be done in rproc_virtio_dev_release.
>> or not, if called, put vdev.dev, and then rproc->dev's cleanup will be
>> done in rproc_virtio_dev_release, otherwise we do cleanup directly.
>>
>> Signed-off-by: weiping zhang <zhangweiping@xxxxxxxxxxxxxxx>
>> ---
>> drivers/remoteproc/remoteproc_virtio.c | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
>> index 2946348..1073ea3 100644
>> --- a/drivers/remoteproc/remoteproc_virtio.c
>> +++ b/drivers/remoteproc/remoteproc_virtio.c
>> @@ -304,7 +304,7 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> {
>> struct rproc *rproc = rvdev->rproc;
>> struct device *dev = &rproc->dev;
>> - struct virtio_device *vdev = &rvdev->vdev;
>> + struct virtio_device *vdev = &rvdev->vdev, *reg_dev = NULL;
>> int ret;
>>
>> vdev->id.device = id,
>> @@ -326,15 +326,24 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev, int id)
>> kref_get(&rvdev->refcount);
>>
>> ret = register_virtio_device(vdev);
>> + reg_dev = vdev;
>> if (ret) {
>> - put_device(&rproc->dev);
>> dev_err(dev, "failed to register vdev: %d\n", ret);
>> goto out;
>> }
>>
>> dev_info(dev, "registered %s (type %d)\n", dev_name(&vdev->dev), id);
>>
>> + return 0;
>> +
>> out:
>> + if (reg_dev)
>> + put_device(&vdev->dev);
>> + else {
>> + kref_put(&rvdev->refcount, rproc_vdev_release);
>> + put_device(&rproc->dev);
>> + }
>> +
>> return ret;
>> }
>>
>
> I think in this case using the marker makes a straightforward cleanup
> way too complicated. There's a single way we can get to the out label,
> and that's when register_virtio_device() failed. Switching
> put_device(&rproc->dev) to put_device(@vdev->dev) (what your first
> patch did) seems like the way to go.
OK, put it directly at v5.
> (It also may be good to cc: the maintainers for this driver.)
OK, thanks
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
