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 >
