[PATCH] drm/amdgpu: fix and cleanup amdgpu_gem_object_close v2

[Christian König]

Ping? Xinhui can I get an rb for this?

Thanks,
Christian.

Am 16.03.20 um 14:22 schrieb Christian König:
> The problem is that we can't add the clear fence to the BO
> when there is an exclusive fence on it since we can't
> guarantee the the clear fence will complete after the
> exclusive one.
>
> To fix this refactor the function and wait for any potential
> exclusive fence with a small timeout before adding the
> shared clear fence.
>
> v2: fix warning
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 43 +++++++++++++++----------
>   1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 5bec66e6b1f8..49c91dac35a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -161,10 +161,11 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>   
>   	struct amdgpu_bo_list_entry vm_pd;
>   	struct list_head list, duplicates;
> +	struct dma_fence *fence = NULL;
>   	struct ttm_validate_buffer tv;
>   	struct ww_acquire_ctx ticket;
>   	struct amdgpu_bo_va *bo_va;
> -	int r;
> +	long r;
>   
>   	INIT_LIST_HEAD(&list);
>   	INIT_LIST_HEAD(&duplicates);
> @@ -178,28 +179,36 @@ void amdgpu_gem_object_close(struct drm_gem_object *obj,
>   	r = ttm_eu_reserve_buffers(&ticket, &list, false, &duplicates);
>   	if (r) {
>   		dev_err(adev->dev, "leaking bo va because "
> -			"we fail to reserve bo (%d)\n", r);
> +			"we fail to reserve bo (%ld)\n", r);
>   		return;
>   	}
>   	bo_va = amdgpu_vm_bo_find(vm, bo);
> -	if (bo_va && --bo_va->ref_count == 0) {
> -		amdgpu_vm_bo_rmv(adev, bo_va);
> +	if (!bo_va || --bo_va->ref_count)
> +		goto out_unlock;
>   
> -		if (amdgpu_vm_ready(vm)) {
> -			struct dma_fence *fence = NULL;
> +	amdgpu_vm_bo_rmv(adev, bo_va);
> +	if (!amdgpu_vm_ready(vm))
> +		goto out_unlock;
>   
> -			r = amdgpu_vm_clear_freed(adev, vm, &fence);
> -			if (unlikely(r)) {
> -				dev_err(adev->dev, "failed to clear page "
> -					"tables on GEM object close (%d)\n", r);
> -			}
>   
> -			if (fence) {
> -				amdgpu_bo_fence(bo, fence, true);
> -				dma_fence_put(fence);
> -			}
> -		}
> -	}
> +	r = amdgpu_vm_clear_freed(adev, vm, &fence);
> +	if (r || !fence)
> +		goto out_unlock;
> +
> +	r = dma_resv_wait_timeout_rcu(bo->tbo.base.resv, false, false,
> +				      msecs_to_jiffies(10));
> +	if (r == 0)
> +		r = -ETIMEDOUT;
> +	if (r)
> +		goto out_unlock;
> +
> +	amdgpu_bo_fence(bo, fence, true);
> +	dma_fence_put(fence);
> +
> +out_unlock:
> +	if (unlikely(r < 0))
> +		dev_err(adev->dev, "failed to clear page "
> +			"tables on GEM object close (%ld)\n", r);
>   	ttm_eu_backoff_reservation(&ticket, &list);
>   }
>