Re: [PATCH 1/1] drm/amdgpu: fix BO leak after successful move test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/20/2021 1:51 PM, Christian König wrote:
Am 20.10.21 um 13:50 schrieb Christian König:


Am 13.10.21 um 17:09 schrieb Nirmoy Das:
GTT BO cleanup code is with in the test for loop and
we would skip cleaning up GTT BO on success.

Reported-by: zhang <botton_zhang@xxxxxxx>
Signed-off-by: Nirmoy Das <nirmoy.das@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_test.c | 25 ++++++++++++------------
  1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
index 909d830b513e..5fe7ff680c29 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_test.c
@@ -35,6 +35,7 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
      struct amdgpu_bo *vram_obj = NULL;
      struct amdgpu_bo **gtt_obj = NULL;
      struct amdgpu_bo_param bp;
+    struct dma_fence *fence = NULL;
      uint64_t gart_addr, vram_addr;
      unsigned n, size;
      int i, r;
@@ -82,7 +83,6 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)
          void *gtt_map, *vram_map;
          void **gart_start, **gart_end;
          void **vram_start, **vram_end;
-        struct dma_fence *fence = NULL;
            bp.domain = AMDGPU_GEM_DOMAIN_GTT;
          r = amdgpu_bo_create(adev, &bp, gtt_obj + i);
@@ -212,24 +212,23 @@ static void amdgpu_do_test_moves(struct amdgpu_device *adev)             DRM_INFO("Tested GTT->VRAM and VRAM->GTT copy for GTT offset 0x%llx\n",
               gart_addr - adev->gmc.gart_start);
-        continue;
+    }
  +    --i;
  out_lclean_unpin:
-        amdgpu_bo_unpin(gtt_obj[i]);
+    amdgpu_bo_unpin(gtt_obj[i]);
  out_lclean_unres:
-        amdgpu_bo_unreserve(gtt_obj[i]);
+    amdgpu_bo_unreserve(gtt_obj[i]);
  out_lclean_unref:
-        amdgpu_bo_unref(&gtt_obj[i]);
+    amdgpu_bo_unref(&gtt_obj[i]);
  out_lclean:
-        for (--i; i >= 0; --i) {
-            amdgpu_bo_unpin(gtt_obj[i]);
-            amdgpu_bo_unreserve(gtt_obj[i]);
-            amdgpu_bo_unref(&gtt_obj[i]);
-        }
-        if (fence)
-            dma_fence_put(fence);
-        break;
+    for (--i; i >= 0; --i) {

The usual idiom for cleanups like that is "while (i--)..." because that also works with an unsigned i.

Apart from that looks good to me.

But I'm not sure that we would want to keep the in kernel tests around anyway.

We now have my amdgpu_stress tool to test memory bandwidth and mesa has an option for that for a long time as well.


Shall I then remove amdgpu_test.c ?


Nirmoy



Christian.


Christian.

+        amdgpu_bo_unpin(gtt_obj[i]);
+        amdgpu_bo_unreserve(gtt_obj[i]);
+        amdgpu_bo_unref(&gtt_obj[i]);
      }
+    if (fence)
+        dma_fence_put(fence);
        amdgpu_bo_unpin(vram_obj);
  out_unres:





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux