[PATCH 1/1] drm/amdgpu: fix the memleak caused by fence not released

[PATCH 1/1] drm/amdgpu: fix the memleak caused by fence not released

[Le Ma]

On systems with CONFIG_SLUB_DEBUG enabled, the memleak like below
will show up explicitly during driver unloading if created bo without
drm_timeline object before.

    BUG drm_sched_fence (Tainted: G           OE     ): Objects remaining in drm_sched_fence on __kmem_cache_shutdown()
    -----------------------------------------------------------------------------
    Call Trace:
    <TASK>
    dump_stack_lvl+0x4c/0x70
    dump_stack+0x14/0x20
    slab_err+0xb0/0xf0
    ? srso_alias_return_thunk+0x5/0xfbef5
    ? flush_work+0x12/0x20
    ? srso_alias_return_thunk+0x5/0xfbef5
    __kmem_cache_shutdown+0x163/0x2e0
    kmem_cache_destroy+0x61/0x170
    drm_sched_fence_slab_fini+0x19/0x900

Thus call dma_fence_put properly to avoid the memleak.

Signed-off-by: Le Ma <le.ma@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 8b67aae6c2fe..ad3c5fdf6970 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -125,8 +125,11 @@ amdgpu_gem_update_bo_mapping(struct drm_file *filp,
 	struct amdgpu_vm *vm = &fpriv->vm;
 	struct dma_fence *last_update;
 
-	if (!syncobj)
+	if (!syncobj) {
+		dma_fence_wait(fence, false);
+		dma_fence_put(fence);
 		return;
+	}
 
 	/* Find the last update fence */
 	switch (operation) {
-- 
2.43.2

Re: [PATCH 1/1] drm/amdgpu: fix the memleak caused by fence not released

[Christian König]

Am 12.02.25 um 12:30 schrieb Le Ma:
> On systems with CONFIG_SLUB_DEBUG enabled, the memleak like below
> will show up explicitly during driver unloading if created bo without
> drm_timeline object before.
>
>      BUG drm_sched_fence (Tainted: G           OE     ): Objects remaining in drm_sched_fence on __kmem_cache_shutdown()
>      -----------------------------------------------------------------------------
>      Call Trace:
>      <TASK>
>      dump_stack_lvl+0x4c/0x70
>      dump_stack+0x14/0x20
>      slab_err+0xb0/0xf0
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? flush_work+0x12/0x20
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      __kmem_cache_shutdown+0x163/0x2e0
>      kmem_cache_destroy+0x61/0x170
>      drm_sched_fence_slab_fini+0x19/0x900
>
> Thus call dma_fence_put properly to avoid the memleak.

Good catch, but there is even more broken here.

The call to amdgpu_gem_update_timeline_node() comes way to late, we need 
to make sure that this is called *before* doing the update. Probably 
best to just inline the functionality.

Then AMDGPU_VM_DELAY_UPDATE and is mutual exclusive to specifying the 
syncobj when the update is completed.

>
> Signed-off-by: Le Ma <le.ma@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 8b67aae6c2fe..ad3c5fdf6970 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -125,8 +125,11 @@ amdgpu_gem_update_bo_mapping(struct drm_file *filp,
>   	struct amdgpu_vm *vm = &fpriv->vm;
>   	struct dma_fence *last_update;
>   
> -	if (!syncobj)
> +	if (!syncobj) {
> +		dma_fence_wait(fence, false);

Please drop that wait, this is just nonsense.

> +		dma_fence_put(fence);
>   		return;
> +	}

We might want to move all this into amdgpu_gem_va_update_vm().

Regards,
Christian.

>   
>   	/* Find the last update fence */
>   	switch (operation) {