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

[Christian König]

Am 20.10.21 um 14:55 schrieb Das, Nirmoy:


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 
Signed-off-by: Nirmoy Das 
---
  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, , 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(_obj[i]);
+    amdgpu_bo_unref(_obj[i]);
  out_lclean:
-    for (--i; i >= 0; --i) {
-    amdgpu_bo_unpin(gtt_obj[i]);
-    amdgpu_bo_unreserve(gtt_obj[i]);
-    amdgpu_bo_unref(_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 ?


Please double check if the amdgpu_stress utility gives you the same 
functionality, if yes we should probably remove this test here.


Thanks,
Christian.




Nirmoy




Christian.



Christian.


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

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

[Christian König]

Am 21.10.21 um 04:07 schrieb zhang:


On 2021/10/20 19:51, 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 
Signed-off-by: Nirmoy Das 
---
  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, , 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(_obj[i]);
+    amdgpu_bo_unref(_obj[i]);
  out_lclean:
-    for (--i; i >= 0; --i) {
-    amdgpu_bo_unpin(gtt_obj[i]);
-    amdgpu_bo_unreserve(gtt_obj[i]);
-    amdgpu_bo_unref(_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.


Christian.

  I found a  testsuit about "bo eviction Test"  for amdgpu . in 
libdrm  tests.


But I couldn't found  amdgpu_stress tool to test memory bandwid anywhere


That was merged just yesterday. See upstream libdrm.

Christian.





Christian.


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

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

[zhang]

On 2021/10/20 19:51, 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 
Signed-off-by: Nirmoy Das 
---
  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, , 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(_obj[i]);
+    amdgpu_bo_unref(_obj[i]);
  out_lclean:
-    for (--i; i >= 0; --i) {
-    amdgpu_bo_unpin(gtt_obj[i]);
-    amdgpu_bo_unreserve(gtt_obj[i]);
-    amdgpu_bo_unref(_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.


Christian.

  I found a  testsuit about "bo eviction Test"  for amdgpu . in libdrm  
tests.


But I couldn't found  amdgpu_stress tool to test memory bandwid anywhere



Christian.


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

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

[Das, Nirmoy]

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 
Signed-off-by: Nirmoy Das 
---
  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, , 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(_obj[i]);
+    amdgpu_bo_unref(_obj[i]);
  out_lclean:
-    for (--i; i >= 0; --i) {
-    amdgpu_bo_unpin(gtt_obj[i]);
-    amdgpu_bo_unreserve(gtt_obj[i]);
-    amdgpu_bo_unref(_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(_obj[i]);
  }
+    if (fence)
+    dma_fence_put(fence);
    amdgpu_bo_unpin(vram_obj);
  out_unres:

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

[Christian König]

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 
Signed-off-by: Nirmoy Das 
---
  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, , 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(_obj[i]);
+    amdgpu_bo_unref(_obj[i]);
  out_lclean:
-    for (--i; i >= 0; --i) {
-    amdgpu_bo_unpin(gtt_obj[i]);
-    amdgpu_bo_unreserve(gtt_obj[i]);
-    amdgpu_bo_unref(_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.


Christian.



Christian.


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

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

[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 
Signed-off-by: Nirmoy Das 
---
  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, , 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(_obj[i]);
+   amdgpu_bo_unref(_obj[i]);
  out_lclean:
-   for (--i; i >= 0; --i) {
-   amdgpu_bo_unpin(gtt_obj[i]);
-   amdgpu_bo_unreserve(gtt_obj[i]);
-   amdgpu_bo_unref(_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.

Christian.


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

  out_unres:

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

[Das, Nirmoy]

ping.

On 10/13/2021 5:09 PM, Nirmoy Das wrote:

GTT BO cleanup code is with in the test for loop and
we would skip cleaning up GTT BO on success.

Reported-by: zhang 
Signed-off-by: Nirmoy Das 
---
  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, , 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(_obj[i]);
+   amdgpu_bo_unref(_obj[i]);
  out_lclean:
-   for (--i; i >= 0; --i) {
-   amdgpu_bo_unpin(gtt_obj[i]);
-   amdgpu_bo_unreserve(gtt_obj[i]);
-   amdgpu_bo_unref(_obj[i]);
-   }
-   if (fence)
-   dma_fence_put(fence);
-   break;
+   for (--i; i >= 0; --i) {
+   amdgpu_bo_unpin(gtt_obj[i]);
+   amdgpu_bo_unreserve(gtt_obj[i]);
+   amdgpu_bo_unref(_obj[i]);
}
+   if (fence)
+   dma_fence_put(fence);
  
  	amdgpu_bo_unpin(vram_obj);

  out_unres:

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

[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 
Signed-off-by: Nirmoy Das 
---
 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, , 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(_obj[i]);
+   amdgpu_bo_unref(_obj[i]);
 out_lclean:
-   for (--i; i >= 0; --i) {
-   amdgpu_bo_unpin(gtt_obj[i]);
-   amdgpu_bo_unreserve(gtt_obj[i]);
-   amdgpu_bo_unref(_obj[i]);
-   }
-   if (fence)
-   dma_fence_put(fence);
-   break;
+   for (--i; i >= 0; --i) {
+   amdgpu_bo_unpin(gtt_obj[i]);
+   amdgpu_bo_unreserve(gtt_obj[i]);
+   amdgpu_bo_unref(_obj[i]);
}
+   if (fence)
+   dma_fence_put(fence);
 
amdgpu_bo_unpin(vram_obj);
 out_unres:
-- 
2.32.0