Re: [PATCH] drm/amdgpu: fix memory leak in wait_all_fence

[zhoucm1]

On 2017年04月07日 16:55, Christian König wrote:

Am 07.04.2017 um 10:46 schrieb zhoucm1:

On 2017年04月07日 16:36, Christian König wrote:

Am 07.04.2017 um 05:36 schrieb Chunming Zhou:

Change-Id: Ib3e271e00e49f10152c1b3eace981a6bf78820de
Signed-off-by: Chunming Zhou <david1.z...@amd.com>

NAK, that will allocate an array for the fence again, which we wanted to avoid.

I don't got your means, the **array just to store fence temporary, freed at the end.

Yeah, but that is unnecessary. We created this function to just avoid that.

We should just drop the fence reference directly after waiting for it.

How to handle err case in the middle.

just put a fence_put() directly after fence_wait_timeout(). We don't need to keep a reference to the fence till the end.

I see your mean, will send V2 soon.

Regards,
David Zhou

Regards,
Christian.

Regards,
David Zhou

Regards,
Christian.

---

drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 25 +++++++++++++++++++------

  1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

index de1c4c3..d842452 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c

@@ -1216,22 +1216,28 @@ static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev,

                       struct drm_amdgpu_fence *fences)
  {
      uint32_t fence_count = wait->in.fence_count;
+    struct fence **array;
      unsigned int i;
      long r = 1;

+ array = kcalloc(fence_count, sizeof(struct fence *), GFP_KERNEL);

+
+    if (array == NULL)
+        return -ENOMEM;
      for (i = 0; i < fence_count; i++) {
          struct fence *fence;

unsigned long timeout = amdgpu_gem_timeout(wait->in.timeout_ns);

            fence = amdgpu_cs_get_fence(adev, filp, &fences[i]);
-        if (IS_ERR(fence))
-            return PTR_ERR(fence);
-        else if (!fence)
+        if (IS_ERR(fence)) {
+            r = PTR_ERR(fence);
+            goto err;
+        } else if (!fence)
              continue;
-
+        array[i] = fence;
          r = kcl_fence_wait_timeout(fence, true, timeout);
          if (r < 0)
-            return r;
+            goto err;
            if (r == 0)
              break;

@@ -1240,7 +1246,14 @@ static int amdgpu_cs_wait_all_fences(struct amdgpu_device *adev,

      memset(wait, 0, sizeof(*wait));
      wait->out.status = (r > 0);
  -    return 0;
+    r = 0;
+
+err:
+    for (i = 0; i < fence_count; i++)
+        fence_put(array[i]);
+    kfree(array);
+
+    return r;
  }
    /**

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx