Re: [Patch v2 3/3] drm/amdgpu/userq: dont check and call dma_resv_wait_timeout

["Christian König" <christian.koenig@amd.com>]

On 3/26/26 14:23, Sunil Khatri wrote:
> In function amdgpu_userq_gem_va_unmap_validate call
> dma_resv_wait_timeout directly.
> 
> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 11 ++---------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c    |  4 +---
>  2 files changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index d29620d3d0d2..e73c69026268 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1477,7 +1477,6 @@ int amdgpu_userq_gem_va_unmap_validate(struct amdgpu_device *adev,
>  	u32 ip_mask = amdgpu_userq_get_supported_ip_mask(adev);
>  	struct amdgpu_bo_va *bo_va = mapping->bo_va;
>  	struct dma_resv *resv = bo_va->base.bo->tbo.base.resv;
> -	int ret = 0;
>  
>  	if (!ip_mask)
>  		return 0;
> @@ -1491,14 +1490,8 @@ int amdgpu_userq_gem_va_unmap_validate(struct amdgpu_device *adev,
>  	 * unmap is only for one kind of userq VAs, so at this point suppose
>  	 * the eviction fence is always unsignaled.
>  	 */
> -	if (!dma_resv_test_signaled(resv, DMA_RESV_USAGE_BOOKKEEP)) {
> -		ret = dma_resv_wait_timeout(resv, DMA_RESV_USAGE_BOOKKEEP, true,
> -					    MAX_SCHEDULE_TIMEOUT);
> -		if (ret <= 0)
> -			return -EBUSY;
> -	}
> -
> -	return 0;
> +	return dma_resv_wait_timeout(resv, DMA_RESV_USAGE_BOOKKEEP,
> +				     true, MAX_SCHEDULE_TIMEOUT);
>  }
>  
>  void amdgpu_userq_pre_reset(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 937a6dd3a4b5..7aa5b8c457c2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1978,7 +1978,6 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>  	struct amdgpu_bo_va_mapping *mapping;
>  	struct amdgpu_vm *vm = bo_va->base.vm;
>  	bool valid = true;
> -	int r;
>  
>  	saddr /= AMDGPU_GPU_PAGE_SIZE;
>  
> @@ -2004,8 +2003,7 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
>  	 * from user space.
>  	 */
>  	if (unlikely(atomic_read(&bo_va->userq_va_mapped) > 0)) {
> -		r = amdgpu_userq_gem_va_unmap_validate(adev, mapping, saddr);
> -		if (unlikely(r == -EBUSY))
> +		if (amdgpu_userq_gem_va_unmap_validate(adev, mapping, saddr))

That is actually broken. dma_resv_wait_timeout() can return -ERESTARTSYS when interrupted and in that moment you should not print an error.

Regards,
Christian.

>  			dev_warn_once(adev->dev,
>  				      "Attempt to unmap an active userq buffer\n");
>  	}