On Wed, 20 Dec 2017 12:27:33 +0800
weiping zhang <zwp10...@gmail.com> 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/

> 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 <zhangweip...@didichuxing.com>
> ---
>  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.

(It also may be good to cc: the maintainers for this driver.)
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to