[PATCH 9/9] drm/amdgpu: Lock the attached dmabuf in unpopulate

[Christian König]

Am 14.04.21 um 08:46 schrieb Felix Kuehling:
> amdgpu_ttm_tt_unpopulate can be called during bo_destroy. The dmabuf->resv
> must not be held by the caller or dma_buf_detach will deadlock. This is
> probably not the right fix. I get a recursive lock warning with the
> reservation held in ttm_bo_release. Should unmap_attachment move to
> backend_unbind instead?

Yes probably, but I'm really wondering if we should call unpopulate 
without holding the reservation lock.

Christian.

>
> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 936b3cfdde55..257750921eed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1216,9 +1216,22 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev,
>   
>   	if (ttm->sg && gtt->gobj->import_attach) {
>   		struct dma_buf_attachment *attach;
> +		bool locked;
>   
>   		attach = gtt->gobj->import_attach;
> +		/* FIXME: unpopulate can be called during bo_destroy.
> +		 * The dmabuf->resv must not be held by the caller or
> +		 * dma_buf_detach will deadlock. This is probably not
> +		 * the right fix. I get a recursive lock warning with the
> +		 * reservation held in ttm_bo_releas.. Should
> +		 * unmap_attachment move to backend_unbind instead?
> +		 */
> +		locked = dma_resv_is_locked(attach->dmabuf->resv);
> +		if (!locked)
> +			dma_resv_lock(attach->dmabuf->resv, NULL);
>   		dma_buf_unmap_attachment(attach, ttm->sg, DMA_BIDIRECTIONAL);
> +		if (!locked)
> +			dma_resv_unlock(attach->dmabuf->resv);
>   		ttm->sg = NULL;
>   		return;
>   	}