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

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

[Gyeyoung Baek]

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@kernel.org>
Closes: https://lore.kernel.org/r/sashiko-outbox-167146@kernel.org/
Cc: stable@vger.kernel.org
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
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,
-- 
Gyeyoung Baek <gye976@gmail.com>

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@kernel.org>
> Closes: https://lore.kernel.org/r/sashiko-outbox-167146@kernel.org/
> Cc: stable@vger.kernel.org
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>

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

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,