RE: [PATCH v1 2/2] drm/virtio: Fix missed dmabuf unpinning in error path of prepare_fb()

[Kasireddy, Vivek]

Hi Dmitry,

> Subject: [PATCH v1 2/2] drm/virtio: Fix missed dmabuf unpinning in error path
> of prepare_fb()
> 
> Unpin imported dmabuf on fence allocation failure in prepare_fb().
> 
> Fixes: 4a696a2ee646 ("drm/virtio: Add prepare and cleanup routines for
> imported dmabuf obj")
> Cc: <stable@xxxxxxxxxxxxxxx> # v6.14+
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/virtio/virtgpu_plane.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c
> b/drivers/gpu/drm/virtio/virtgpu_plane.c
> index a6f5a78f436a..dc1d91639931 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_plane.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
> @@ -342,6 +342,16 @@ static int virtio_gpu_prepare_imported_obj(struct
> drm_plane *plane,
>  	return ret;
>  }
> 
> +static void virtio_gpu_cleanup_imported_obj(struct drm_gem_object *obj)
> +{
> +	struct dma_buf_attachment *attach = obj->import_attach;
> +	struct dma_resv *resv = attach->dmabuf->resv;
> +
> +	dma_resv_lock(resv, NULL);
> +	dma_buf_unpin(attach);
> +	dma_resv_unlock(resv);
> +}
> +
>  static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
>  				       struct drm_plane_state *new_state)
>  {
> @@ -376,23 +386,16 @@ static int virtio_gpu_plane_prepare_fb(struct
> drm_plane *plane,
>  		vgplane_st->fence = virtio_gpu_fence_alloc(vgdev,
>  						     vgdev->fence_drv.context,
>  						     0);
> -		if (!vgplane_st->fence)
> +		if (!vgplane_st->fence) {
> +			if (obj->import_attach)
> +				virtio_gpu_cleanup_imported_obj(obj);
I think checking for fence allocation failure before import would be much better.
In other words, cleaning up the fence in case of any import errors would be
much simpler IMO.

Regardless,
Acked-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx> 

>  			return -ENOMEM;
> +		}
>  	}
> 
>  	return 0;
>  }
> 
> -static void virtio_gpu_cleanup_imported_obj(struct drm_gem_object *obj)
> -{
> -	struct dma_buf_attachment *attach = obj->import_attach;
> -	struct dma_resv *resv = attach->dmabuf->resv;
> -
> -	dma_resv_lock(resv, NULL);
> -	dma_buf_unpin(attach);
> -	dma_resv_unlock(resv);
> -}
> -
>  static void virtio_gpu_plane_cleanup_fb(struct drm_plane *plane,
>  					struct drm_plane_state *state)
>  {
> --
> 2.49.0
>