Re: [PATCH] drm/amdgpu: release allocated memory

2019-09-23 Thread Koenig, Christian
Yeah, we already documented the dma_fence_array() behavior on that function.

But having another comment here would probably be a good idea.

Christian.

Am 23.09.19 um 14:35 schrieb Deucher, Alexander:
Maybe add a comment here in the code to avoid confusion in the future.

Alex

From: Koenig, Christian 
<mailto:christian.koe...@amd.com>
Sent: Saturday, September 21, 2019 7:32 AM
To: Navid Emamdoost 
<mailto:navid.emamdo...@gmail.com>
Cc: emamd...@umn.edu<mailto:emamd...@umn.edu> 
<mailto:emamd...@umn.edu>; 
smcca...@umn.edu<mailto:smcca...@umn.edu> 
<mailto:smcca...@umn.edu>; k...@umn.edu<mailto:k...@umn.edu> 
<mailto:k...@umn.edu>; Deucher, Alexander 
<mailto:alexander.deuc...@amd.com>; Zhou, 
David(ChunMing) <mailto:david1.z...@amd.com>; David Airlie 
<mailto:airl...@linux.ie>; Daniel Vetter 
<mailto:dan...@ffwll.ch>; Sam Ravnborg 
<mailto:s...@ravnborg.org>; Rex Zhu 
<mailto:rex@amd.com>; 
amd-...@lists.freedesktop.org<mailto:amd-...@lists.freedesktop.org> 
<mailto:amd-...@lists.freedesktop.org>; 
dri-devel@lists.freedesktop.org<mailto:dri-devel@lists.freedesktop.org> 
<mailto:dri-devel@lists.freedesktop.org>; 
linux-ker...@vger.kernel.org<mailto:linux-ker...@vger.kernel.org> 
<mailto:linux-ker...@vger.kernel.org>
Subject: Re: [PATCH] drm/amdgpu: release allocated memory

Am 21.09.19 um 00:49 schrieb Navid Emamdoost:
> In amdgpu_vmid_grab_idle, fences is being leaked in one execution path.
> The missing kfree was added.

NAK, the array is freed by the dma_fence_array.  This is a double free.

Regards,
Christian.

>
> Signed-off-by: Navid Emamdoost 
> <mailto:navid.emamdo...@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index 57b3d8a9bef3..9063cd36fa94 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -244,6 +244,7 @@ static int amdgpu_vmid_grab_idle(struct amdgpu_vm *vm,
>r = amdgpu_sync_fence(adev, sync, &array->base, false);
>dma_fence_put(ring->vmid_wait);
>ring->vmid_wait = &array->base;
> + kfree(fences);
>return r;
>}
>kfree(fences);


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amdgpu: release allocated memory

2019-09-23 Thread Deucher, Alexander
Maybe add a comment here in the code to avoid confusion in the future.

Alex

From: Koenig, Christian 
Sent: Saturday, September 21, 2019 7:32 AM
To: Navid Emamdoost 
Cc: emamd...@umn.edu ; smcca...@umn.edu ; 
k...@umn.edu ; Deucher, Alexander ; 
Zhou, David(ChunMing) ; David Airlie ; 
Daniel Vetter ; Sam Ravnborg ; Rex Zhu 
; amd-...@lists.freedesktop.org 
; dri-devel@lists.freedesktop.org 
; linux-ker...@vger.kernel.org 

Subject: Re: [PATCH] drm/amdgpu: release allocated memory

Am 21.09.19 um 00:49 schrieb Navid Emamdoost:
> In amdgpu_vmid_grab_idle, fences is being leaked in one execution path.
> The missing kfree was added.

NAK, the array is freed by the dma_fence_array.  This is a double free.

Regards,
Christian.

>
> Signed-off-by: Navid Emamdoost 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index 57b3d8a9bef3..9063cd36fa94 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -244,6 +244,7 @@ static int amdgpu_vmid_grab_idle(struct amdgpu_vm *vm,
>r = amdgpu_sync_fence(adev, sync, &array->base, false);
>dma_fence_put(ring->vmid_wait);
>ring->vmid_wait = &array->base;
> + kfree(fences);
>return r;
>}
>kfree(fences);

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/amdgpu: release allocated memory

2019-09-22 Thread Navid Emamdoost
In amdgpu_vmid_grab_idle, fences is being leaked in one execution path.
The missing kfree was added.

Signed-off-by: Navid Emamdoost 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index 57b3d8a9bef3..9063cd36fa94 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -244,6 +244,7 @@ static int amdgpu_vmid_grab_idle(struct amdgpu_vm *vm,
r = amdgpu_sync_fence(adev, sync, &array->base, false);
dma_fence_put(ring->vmid_wait);
ring->vmid_wait = &array->base;
+   kfree(fences);
return r;
}
kfree(fences);
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amdgpu: release allocated memory

2019-09-22 Thread Koenig, Christian
Am 21.09.19 um 00:49 schrieb Navid Emamdoost:
> In amdgpu_vmid_grab_idle, fences is being leaked in one execution path.
> The missing kfree was added.

NAK, the array is freed by the dma_fence_array.  This is a double free.

Regards,
Christian.

>
> Signed-off-by: Navid Emamdoost 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> index 57b3d8a9bef3..9063cd36fa94 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
> @@ -244,6 +244,7 @@ static int amdgpu_vmid_grab_idle(struct amdgpu_vm *vm,
>   r = amdgpu_sync_fence(adev, sync, &array->base, false);
>   dma_fence_put(ring->vmid_wait);
>   ring->vmid_wait = &array->base;
> + kfree(fences);
>   return r;
>   }
>   kfree(fences);

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel