Re: [PATCH v2 1/2] drm/imagination: Switch the GPUVM to immediate mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/imagination: infinite loop in pvr_vm_unmap_all() on unmap failure
--

commit e8ae922e82d0545dbcb5536fc88e9f62e91d62ac
Author: Gyeyoung Baek <gye976@gmail.com>

    drm/imagination: Switch the GPUVM to immediate mode
    
    Use DRM_GPUVM_IMMEDIATE_MODE so that each GEM's GPUVA list is protected
    by its gpuva.lock instead of its dma_resv. This prepares for
    asynchronous VM_BIND, whose run_job() will update the GPUVA lists
    before signalling its fence. That section imposes three rules:

[ ... ]

> diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
> index cbdd15ed74fdb..807a01aab1dae 100644
> --- a/drivers/gpu/drm/imagination/pvr_vm.c
> +++ b/drivers/gpu/drm/imagination/pvr_vm.c

[ ... ]

> @@ -906,6 +925,8 @@ pvr_vm_unmap_all(struct pvr_vm_context *vm_ctx)
>  	}
>  
>  	mutex_unlock(&vm_ctx->lock);
> +
> +	drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
>  }

[Severity: High]
Can this result in an infinite loop if the unmap operation fails?

Looking at the loop just above this context in pvr_vm_unmap_all():

	for (;;) {
		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);
		if (!va)
			break;

		pvr_obj = gem_to_pvr_gem(va->gem.obj);

		WARN_ON(pvr_vm_unmap_obj_locked(vm_ctx, pvr_obj,
						va->va.addr, va->va.range));
	}

If pvr_vm_unmap_obj_locked() returns an error (which is highly possible, for
example when kzalloc_obj() or pvr_mmu_op_context_create() return -ENOMEM
inside pvr_vm_bind_op_unmap_init() during process teardown), the VA remains
in the tree.

Since the loop evaluates WARN_ON() but does not break on error, it appears
drm_gpuva_find_first() will find the exact same VA again, causing the loop to
spin indefinitely. This could lead to a kernel soft lockup.

It looks like the series end commit removes the drm_gpuvm_exec_lock but
leaves the -ENOMEM failure paths intact, so the lockup potential remains.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-pvr-gpuvm-immediate-v2-0-744ddc6644c3@gmail.com?part=1