RE: [PATCH] drm/amdgpu: skip clearing empty freed VM list on GEM close

AMD General

Hi Felix,

Thanks for the review and suggestion.

That makes sense. Moving the check to the beginning of amdgpu_vm_clear_freed() is a cleaner approach since it keeps the optimization local to the function and benefits all callers without exposing or duplicating knowledge of vm->freed internals.

I'll update the patch accordingly and send a v2.

Regards,
Bob

-----Original Message-----
From: Kuehling, Felix <[email protected]>
Sent: Wednesday, July 15, 2026 11:35 PM
To: Zhou, Bob <[email protected]>; [email protected]; Deucher, Alexander <[email protected]>
Subject: Re: [PATCH] drm/amdgpu: skip clearing empty freed VM list on GEM close


On 2026-07-15 03:45, Bob Zhou wrote:
> amdgpu_gem_object_close() calls amdgpu_vm_clear_freed() after deleting a BO VA. If vm->freed is empty, that call is a no-op but still allocates sync state and walks reservation fences before returning.
>
> Check vm->freed first to avoid the overhead on the GEM close hot path. This does not change behavior because the empty-list path leaves the fence unset and returns success.

I think you could get the same effect if you put the check at the start of amdgpu_vm_clear_freed. The function would just return with "fence"
unchanged, or NULL in this case. Then this optimization would also apply to any other situations where amgpu_vm_clear_freed gets called without anything to do. The other advantage is that the callers of amdgpu_vm_clear_freed don't need to make assumption about how it works, or know anything about internal state of struct amdgpu_vm.

Regards,
   Felix


>
> Signed-off-by: Bob Zhou <[email protected]>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 6a0699746fbcd..72811f6963a15 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -348,6 +348,8 @@ static void amdgpu_gem_object_close(struct drm_gem_object *obj,
>          amdgpu_vm_bo_update_shared(bo);
>          if (!amdgpu_vm_ready(vm))
>                  goto out_unlock;
> +       if (list_empty(&vm->freed))
> +               goto out_unlock;
>
>          r = amdgpu_vm_clear_freed(adev, vm, &fence);
>          if (unlikely(r < 0) &&
> !drm_dev_is_unplugged(adev_to_drm(adev)))
> --
> 2.34.1
>