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