Re: [PATCH] drm/imagination: Don't retry a failed unmap in pvr_vm_unmap_all()

[Alexandru Dadu]

On Thu, 2026-10-15 at 18:10 +0900, Gyeyoung Baek wrote:

Hi Gyeyoung,

> pvr_vm_unmap_all() repeatedly looks up the first VA and unmaps it.
> The
> VA is removed from the tree at the end of this path:
> 
>   pvr_vm_unmap_all()
>     pvr_vm_unmap_obj_locked()
>       drm_gpuvm_sm_unmap()
>         pvr_vm_gpuva_unmap()
>           drm_gpuva_unmap()   <- removes the VA from the tree
> 
> If this path fails before drm_gpuva_unmap(), the same VA is found
> again. If the failure persists, e.g. memory allocation failure under
> memory pressure, the loop never terminates and the task hangs.
> 
> Always advance to the next VA, whether or not the unmap succeeded. A
> VA that fails to unmap is left in place with a warning.
> 
> Fixes: a5c4c3ba95a5 ("drm/imagination: Hold drm_gem_gpuva lock for
> unmap")
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Closes: https://lore.kernel.org/r/sashiko-outbox-167146@xxxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Gyeyoung Baek <gye976@xxxxxxxxx>

Reviewed-by: Alexandru Dadu <alexandru.dadu@xxxxxxxxxx>

Thanks,
Alexandru Dadu

> ---
> Tested with the IGT pvr_* tests.
> ---
>  drivers/gpu/drm/imagination/pvr_vm.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c
> b/drivers/gpu/drm/imagination/pvr_vm.c
> index cbdd15ed74fd..8ea6e88c8bd7 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c
> @@ -887,22 +887,25 @@ pvr_vm_unmap(struct pvr_vm_context *vm_ctx, u64
> device_addr, u64 size)
>  void
>  pvr_vm_unmap_all(struct pvr_vm_context *vm_ctx)
>  {
> +	u64 addr = vm_ctx->gpuvm_mgr.mm_start;
> +	u64 end = vm_ctx->gpuvm_mgr.mm_start + vm_ctx-
> >gpuvm_mgr.mm_range;
> +	u64 va_addr, va_range;
> +
>  	mutex_lock(&vm_ctx->lock);
>  
> -	for (;;) {
> +	for (; addr < end; addr = va_addr + va_range) {
>  		struct pvr_gem_object *pvr_obj;
>  		struct drm_gpuva *va;
>  
> -		va = drm_gpuva_find_first(&vm_ctx->gpuvm_mgr,
> -					  vm_ctx-
> >gpuvm_mgr.mm_start,
> -					  vm_ctx-
> >gpuvm_mgr.mm_range);
> +		va = drm_gpuva_find_first(&vm_ctx->gpuvm_mgr, addr,
> end - addr);
>  		if (!va)
>  			break;
>  
>  		pvr_obj = gem_to_pvr_gem(va->gem.obj);
> +		va_addr = va->va.addr;
> +		va_range = va->va.range;
>  
> -		WARN_ON(pvr_vm_unmap_obj_locked(vm_ctx, pvr_obj,
> -						va->va.addr, va-
> >va.range));
> +		WARN_ON(pvr_vm_unmap_obj_locked(vm_ctx, pvr_obj,
> va_addr, va_range));
>  	}
>  
>  	mutex_unlock(&vm_ctx->lock);
> 
> ---
> base-commit: c347fef5f8f8927985bd67835331ce4cf7ff43a4
> change-id: 20261015-pvr-unmap-all-loop-99fe2f2b7e3f
> 
> Best regards,