Makes sense, Reviewed-by: Christian König <christian.koe...@amd.com> fro this one as well.

Regards,
Christian.

Am 14.11.2017 um 16:00 schrieb Liu, Monk:
Actually " drm_mm_takedown" below will give you the warning if some BO is still 
in MM
That means below part is totally useless:

   -    spin_lock(&mgr->lock);
-    if (!drm_mm_clean(&mgr->mm)) {
-        spin_unlock(&mgr->lock);
-        return -EBUSY;
-    }
-
And the worse thing is this "return -EBUSY" would lead to memory leak because 
it blocks following kfree() on mgr...
BR Monk

-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumer...@gmail.com]
Sent: 2017年11月14日 20:01
To: Liu, Monk <monk....@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 05/10] drm/amdgpu:fix memleak in takedown

Am 14.11.2017 um 12:52 schrieb Christian König:
Am 14.11.2017 um 10:07 schrieb Monk Liu:
this can fix the memory leak under the case that not all BO are freed
during "takedown" stage, because originally it blocks following kfree
on mgr.
NAK, all BOs must be freed before we take down at least the GTT manager.
Ah, wait a second. That is the address space manager here!

Mhm, returning -EBUSY is probably not a good idea, but we should at least print 
some warning.

Regards,
Christian.

That we run into a memory leak is intended, cause that is still better
than crashing the system because we access already freed up memory.

Regards,
Christian.

Change-Id: I56e832ac317215a4d9f58edb8e75ea722158715c
Signed-off-by: Monk Liu <monk....@amd.com>
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 6 ------
   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 -----
   2 files changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 88709f9..72819cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -76,12 +76,6 @@ static int amdgpu_gtt_mgr_fini(struct
ttm_mem_type_manager *man)
   {
       struct amdgpu_gtt_mgr *mgr = man->priv;
   -    spin_lock(&mgr->lock);
-    if (!drm_mm_clean(&mgr->mm)) {
-        spin_unlock(&mgr->lock);
-        return -EBUSY;
-    }
-
       drm_mm_takedown(&mgr->mm);
       spin_unlock(&mgr->lock);
       kfree(mgr);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 4ca622b..3e883e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -68,11 +68,6 @@ static int amdgpu_vram_mgr_fini(struct
ttm_mem_type_manager *man)
       struct amdgpu_vram_mgr *mgr = man->priv;
         spin_lock(&mgr->lock);
-    if (!drm_mm_clean(&mgr->mm)) {
-        spin_unlock(&mgr->lock);
-        return -EBUSY;
-    }
-
       drm_mm_takedown(&mgr->mm);
       spin_unlock(&mgr->lock);
       kfree(mgr);


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

Reply via email to