Re: [PATCH 1/3] drm/amdgpu: fix the tlb flush fence leak

[Christian König]

On 3/19/26 09:21, Prike Liang wrote:
> The TLB flush fence leaked during walking over
> the free mapping list. Meanwhile, the TLB flush fence
> is referenced by root BOs reservation which is a false
> leak and we should mark it.
> 
> Signed-off-by: Prike Liang <prike.li...@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c           | 3 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c | 7 ++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7ef0cb6bcbda..1cd4c4217b02 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1560,6 +1560,8 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>               goto error_free;
>  
>       while (!list_empty(&vm->freed)) {
> +             struct dma_fence *old_f = f;
> +
>               mapping = list_first_entry(&vm->freed,
>                       struct amdgpu_bo_va_mapping, list);
>               list_del(&mapping->list);
> @@ -1572,6 +1574,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>                       dma_fence_put(f);
>                       goto error_free;
>               }
> +             dma_fence_put(old_f);

That is incorrect as far as I can see, you are dropping the fence without 
acquiring a reference.

>       }
>  
>       if (fence && f) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
> index 5d26797356a3..93b72289e7df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_tlb_fence.c
> @@ -23,6 +23,7 @@
>  
>  #include <linux/dma-fence.h>
>  #include <linux/workqueue.h>
> +#include <linux/kmemleak.h>
>  
>  #include "amdgpu.h"
>  #include "amdgpu_vm.h"
> @@ -106,6 +107,10 @@ void amdgpu_vm_tlb_fence_create(struct amdgpu_device 
> *adev, struct amdgpu_vm *vm
>       /* TODO: We probably need a separate wq here */
>       dma_fence_get(&f->base);
>       schedule_work(&f->work);
> -
> +     /* The TLB fence is referenced by dma_resv and
> +      * the resv ref is the remaining ref, so that's
> +      * a false positive leak.
> +      */
> +     kmemleak_not_leak(f);

That is just utterly nonsense. kmemleak is perfectly capable to detect the 
pointer in the dma_resv object.

Regards,
Christian.

>       *fence = &f->base;
>  }