Re: [PATCH] drm/amdgpu: grab a BO reference in vm_lock_done_list.

["Khatri, Sunil" <sukhatri@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]


On 31-10-2025 01:55 pm, Christian König wrote:
> Otherwise it is possible that between dropping the status lock and
> locking the BO that the BO is freed up.
>
> Signed-off-by: Christian König<christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index db66b4232de0..c3dfb949a9b8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -484,15 +484,19 @@ int amdgpu_vm_lock_done_list(struct amdgpu_vm *vm, struct drm_exec *exec,
>   	spin_lock(&vm->status_lock);
>   	while (!list_is_head(prev->next, &vm->done)) {
>   		bo_va = list_entry(prev->next, typeof(*bo_va), base.vm_status);
> -		spin_unlock(&vm->status_lock);
>   
>   		bo = bo_va->base.bo;
>   		if (bo) {
> +			amdgpu_bo_ref(bo);
> +			spin_unlock(&vm->status_lock);
> +
>   			ret = drm_exec_prepare_obj(exec, &bo->tbo.base, 1);

Just for my understanding, there is a possibility that a bo is free 
after spin_unlock and we might be using a stale/NULL ptr of bo.  So we 
are taking a reference before releasing the lock to make

sure the bo is valid. Now calling drm_exec_prepare_obj take a recount to 
make sure the bo is always from now on. Reviewed-by: Sunil Khatri 
<sunil.khatri@amd.com>

> +			amdgpu_bo_unref(&bo);
>   			if (unlikely(ret))
>   				return ret;
> +
> +			spin_lock(&vm->status_lock);
>   		}
> -		spin_lock(&vm->status_lock);
>   		prev = prev->next;
>   	}
>   	spin_unlock(&vm->status_lock);

[-- Attachment #2: Type: text/html, Size: 2380 bytes --]