All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings
@ 2023-02-02 17:40 Bert Karwatzki
  2023-02-02 18:13 ` Alex Deucher
  0 siblings, 1 reply; 7+ messages in thread
From: Bert Karwatzki @ 2023-02-02 17:40 UTC (permalink / raw)
  To: amd-gfx

amdgpu_sync_get_fence deletes the returned fence from the syncobj, so
the refcount of fence needs to lowered to avoid a memory leak:
https://gitlab.freedesktop.org/drm/amd/-/issues/2360

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 0f4cb41078c1..08eced097bd8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1222,10 +1222,13 @@ static int amdgpu_cs_sync_rings(struct
amdgpu_cs_parser *p)
                 * next job actually sees the results from the previous
one
                 * before we start executing on the same scheduler
ring.
                 */
-               if (!s_fence || s_fence->sched != sched)
+               if (!s_fence || s_fence->sched != sched) {
+                       dma_fence_put(fence);
                        continue;
+               }
 
                r = amdgpu_sync_fence(&p->gang_leader->explicit_sync,
fence);
+               dma_fence_put(fence);
                if (r)
                        return r;
        }


Bert Karwatzki

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings
  2023-02-02 17:40 [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings Bert Karwatzki
@ 2023-02-02 18:13 ` Alex Deucher
  2023-02-02 19:02   ` Bert Karwatzki
  2023-02-03 16:53   ` Bert Karwatzki
  0 siblings, 2 replies; 7+ messages in thread
From: Alex Deucher @ 2023-02-02 18:13 UTC (permalink / raw)
  To: Bert Karwatzki; +Cc: amd-gfx

On Thu, Feb 2, 2023 at 1:05 PM Bert Karwatzki <spasswolf@web.de> wrote:
>
> amdgpu_sync_get_fence deletes the returned fence from the syncobj, so
> the refcount of fence needs to lowered to avoid a memory leak:
> https://gitlab.freedesktop.org/drm/amd/-/issues/2360

Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2360

Please send a proper patch using git-format-patch.  Also, please add
your Signed-off-by line.

With those fixed, the patch is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 0f4cb41078c1..08eced097bd8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1222,10 +1222,13 @@ static int amdgpu_cs_sync_rings(struct
> amdgpu_cs_parser *p)
>                  * next job actually sees the results from the previous
> one
>                  * before we start executing on the same scheduler
> ring.
>                  */
> -               if (!s_fence || s_fence->sched != sched)
> +               if (!s_fence || s_fence->sched != sched) {
> +                       dma_fence_put(fence);
>                         continue;
> +               }
>
>                 r = amdgpu_sync_fence(&p->gang_leader->explicit_sync,
> fence);
> +               dma_fence_put(fence);
>                 if (r)
>                         return r;
>         }
>
>
> Bert Karwatzki

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings
  2023-02-02 18:13 ` Alex Deucher
@ 2023-02-02 19:02   ` Bert Karwatzki
  2023-02-03 10:42     ` Mikhail Gavrilov
  2023-02-03 13:18     ` Christian König
  2023-02-03 16:53   ` Bert Karwatzki
  1 sibling, 2 replies; 7+ messages in thread
From: Bert Karwatzki @ 2023-02-02 19:02 UTC (permalink / raw)
  To: Alex Deucher; +Cc: amd-gfx

I hope I got it right this time:
Here is the fix for
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2360

From 6e064c9565ef0da890f3fcb2a4f6a8cd44a12fdb Mon Sep 17 00:00:00 2001
From: Bert Karwatzki <spasswolf@web.de>
Date: Thu, 2 Feb 2023 19:50:27 +0100
Subject: [PATCH] Fix memory leak in amdgpu_cs_sync_rings.

Signed-off-by: Bert Karwatzki <spasswolf@web.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 0f4cb41078c1..08eced097bd8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1222,10 +1222,13 @@ static int amdgpu_cs_sync_rings(struct
amdgpu_cs_parser *p)
 		 * next job actually sees the results from the
previous one
 		 * before we start executing on the same scheduler
ring.
 		 */
-		if (!s_fence || s_fence->sched != sched)
+		if (!s_fence || s_fence->sched != sched) {
+			dma_fence_put(fence);
 			continue;
+		}
 
 		r = amdgpu_sync_fence(&p->gang_leader->explicit_sync,
fence);
+		dma_fence_put(fence);
 		if (r)
 			return r;
 	}
-- 
2.39.1


Bert Karwatzki

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings
  2023-02-02 19:02   ` Bert Karwatzki
@ 2023-02-03 10:42     ` Mikhail Gavrilov
  2023-02-03 13:18     ` Christian König
  1 sibling, 0 replies; 7+ messages in thread
From: Mikhail Gavrilov @ 2023-02-03 10:42 UTC (permalink / raw)
  To: Bert Karwatzki; +Cc: Alex Deucher, amd-gfx

On Fri, Feb 3, 2023 at 12:10 AM Bert Karwatzki <spasswolf@web.de> wrote:
>
> I hope I got it right this time:
> Here is the fix for
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2360
>
> From 6e064c9565ef0da890f3fcb2a4f6a8cd44a12fdb Mon Sep 17 00:00:00 2001
> From: Bert Karwatzki <spasswolf@web.de>
> Date: Thu, 2 Feb 2023 19:50:27 +0100
> Subject: [PATCH] Fix memory leak in amdgpu_cs_sync_rings.
>
> Signed-off-by: Bert Karwatzki <spasswolf@web.de>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 0f4cb41078c1..08eced097bd8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1222,10 +1222,13 @@ static int amdgpu_cs_sync_rings(struct
> amdgpu_cs_parser *p)
>                  * next job actually sees the results from the
> previous one
>                  * before we start executing on the same scheduler
> ring.
>                  */
> -               if (!s_fence || s_fence->sched != sched)
> +               if (!s_fence || s_fence->sched != sched) {
> +                       dma_fence_put(fence);
>                         continue;
> +               }
>
>                 r = amdgpu_sync_fence(&p->gang_leader->explicit_sync,
> fence);
> +               dma_fence_put(fence);
>                 if (r)
>                         return r;
>         }
> --
> 2.39.1
>

As a bug reporter I can confirm this patch fixes a memory leak.
Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>

-- 
Best Regards,
Mike Gavrilov.

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings
  2023-02-02 19:02   ` Bert Karwatzki
  2023-02-03 10:42     ` Mikhail Gavrilov
@ 2023-02-03 13:18     ` Christian König
  1 sibling, 0 replies; 7+ messages in thread
From: Christian König @ 2023-02-03 13:18 UTC (permalink / raw)
  To: Bert Karwatzki, Alex Deucher; +Cc: amd-gfx

Am 02.02.23 um 20:02 schrieb Bert Karwatzki:
> I hope I got it right this time:
> Here is the fix for
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2360
>
>  From 6e064c9565ef0da890f3fcb2a4f6a8cd44a12fdb Mon Sep 17 00:00:00 2001
> From: Bert Karwatzki <spasswolf@web.de>
> Date: Thu, 2 Feb 2023 19:50:27 +0100
> Subject: [PATCH] Fix memory leak in amdgpu_cs_sync_rings.
>
> Signed-off-by: Bert Karwatzki <spasswolf@web.de>

Well this patch header is not properly formated, but I think Alex can 
fix this when he picks it up. If not I will take care of this next week.

Reviewed-by: Christian König <christian.koenig@amd.com> on the code change.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 0f4cb41078c1..08eced097bd8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1222,10 +1222,13 @@ static int amdgpu_cs_sync_rings(struct
> amdgpu_cs_parser *p)
>   		 * next job actually sees the results from the
> previous one
>   		 * before we start executing on the same scheduler
> ring.
>   		 */
> -		if (!s_fence || s_fence->sched != sched)
> +		if (!s_fence || s_fence->sched != sched) {
> +			dma_fence_put(fence);
>   			continue;
> +		}
>   
>   		r = amdgpu_sync_fence(&p->gang_leader->explicit_sync,
> fence);
> +		dma_fence_put(fence);
>   		if (r)
>   			return r;
>   	}


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings
  2023-02-02 18:13 ` Alex Deucher
  2023-02-02 19:02   ` Bert Karwatzki
@ 2023-02-03 16:53   ` Bert Karwatzki
  2023-02-03 19:28     ` Alex Deucher
  1 sibling, 1 reply; 7+ messages in thread
From: Bert Karwatzki @ 2023-02-03 16:53 UTC (permalink / raw)
  To: Alex Deucher; +Cc: amd-gfx

Here is the fix for (send again to test if I can get it right with
Evolution)
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2360

From 6e064c9565ef0da890f3fcb2a4f6a8cd44a12fdb Mon Sep 17 00:00:00 2001
From: Bert Karwatzki <spasswolf@web.de>
Date: Thu, 2 Feb 2023 19:50:27 +0100
Subject: [PATCH] Fix memory leak in amdgpu_cs_sync_rings.

Signed-off-by: Bert Karwatzki <spasswolf@web.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 0f4cb41078c1..08eced097bd8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1222,10 +1222,13 @@ static int amdgpu_cs_sync_rings(struct
amdgpu_cs_parser *p)
                 * next job actually sees the results from the
previous one
                 * before we start executing on the same scheduler
ring.
                 */
-               if (!s_fence || s_fence->sched != sched)
+               if (!s_fence || s_fence->sched != sched) {
+                       dma_fence_put(fence);
                        continue;
+               }
 
                r = amdgpu_sync_fence(&p->gang_leader->explicit_sync,
fence);
+               dma_fence_put(fence);
                if (r)
                        return r;
        }


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings
  2023-02-03 16:53   ` Bert Karwatzki
@ 2023-02-03 19:28     ` Alex Deucher
  0 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2023-02-03 19:28 UTC (permalink / raw)
  To: Bert Karwatzki; +Cc: amd-gfx

Applied.  Thanks.  I forgot to add it to the commit when I pushed it,
but for posterity,

Fixes: 3bd68b32c911 ("drm/amdgpu: fix pipeline sync v2:)

Alex

On Fri, Feb 3, 2023 at 11:53 AM Bert Karwatzki <spasswolf@web.de> wrote:
>
> Here is the fix for (send again to test if I can get it right with
> Evolution)
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/2360
>
> From 6e064c9565ef0da890f3fcb2a4f6a8cd44a12fdb Mon Sep 17 00:00:00 2001
> From: Bert Karwatzki <spasswolf@web.de>
> Date: Thu, 2 Feb 2023 19:50:27 +0100
> Subject: [PATCH] Fix memory leak in amdgpu_cs_sync_rings.
>
> Signed-off-by: Bert Karwatzki <spasswolf@web.de>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 0f4cb41078c1..08eced097bd8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1222,10 +1222,13 @@ static int amdgpu_cs_sync_rings(struct
> amdgpu_cs_parser *p)
>                  * next job actually sees the results from the
> previous one
>                  * before we start executing on the same scheduler
> ring.
>                  */
> -               if (!s_fence || s_fence->sched != sched)
> +               if (!s_fence || s_fence->sched != sched) {
> +                       dma_fence_put(fence);
>                         continue;
> +               }
>
>                 r = amdgpu_sync_fence(&p->gang_leader->explicit_sync,
> fence);
> +               dma_fence_put(fence);
>                 if (r)
>                         return r;
>         }
>

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-03 19:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02 17:40 [PATCH] drm/amd: fix memory leak in amdgpu_cs_sync_rings Bert Karwatzki
2023-02-02 18:13 ` Alex Deucher
2023-02-02 19:02   ` Bert Karwatzki
2023-02-03 10:42     ` Mikhail Gavrilov
2023-02-03 13:18     ` Christian König
2023-02-03 16:53   ` Bert Karwatzki
2023-02-03 19:28     ` Alex Deucher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.