发件人："Christian König" <[email protected]> 发送日期：2020-04-21 21:02:27 收件人："赵军奎" <[email protected]> 抄送人：Alex Deucher <[email protected]>,"David (ChunMing) Zhou" <[email protected]>,David Airlie <[email protected]>,Daniel Vetter <[email protected]>,Tom St Denis <[email protected]>,Ori Messinger <[email protected]>,Sam Ravnborg <[email protected]>,[email protected],[email protected],[email protected],[email protected] 主题：Re: [PATCH] amdgpu: fixes memleak issue when init failed>Am 21.04.20 um 14:09 schrieb 赵军奎: >> From: "Christian König" <[email protected]> >> Date: 2020-04-21 19:22:49 >> To: Bernard Zhao <[email protected]>,Alex Deucher >> <[email protected]>,"David (ChunMing) Zhou" >> <[email protected]>,David Airlie <[email protected]>,Daniel Vetter >> <[email protected]>,Tom St Denis <[email protected]>,Ori Messinger >> <[email protected]>,Sam Ravnborg >> <[email protected]>,[email protected],[email protected],[email protected] >> Cc: [email protected] >> Subject: Re: [PATCH] amdgpu: fixes memleak issue when init failed>Am >> 21.04.20 um 13:17 schrieb Bernard Zhao: >>>> VRAM manager and DRM MM when init failed, there is no operaction >>>> to free kzalloc memory & remove device file. >>>> This will lead to memleak & cause stability issue. >>> NAK, failure to create sysfs nodes are not critical. >>> >>> Christian. >>> >> OK, get it. >> By the way, should i modify this patch to just handle <kfree(mgr)> in error >> branch, or that it is also unnecessary? > >What you can do is to drop the "return ret" if anything with the sysfs >nodes goes wrong and instead print the error code.
Emmm, for this part, i am not sure, my modify first print the error, secone release not free memory, and last return error, make everything clear to the system. I think it`s the same with what you mentioned, is there something that I misunderstood? > >It's really annoying that loading, unloading and loading the driver >again sometimes fails because we have a bug in the sysfs files cleanup. > >We certainly should fix those bugs as well, but they are just not >critical for correct driver functionality. > >Regards, >Christian. >> >> Regards, >> Bernard >> >>>> Signed-off-by: Bernard Zhao <[email protected]> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 24 ++++++++++++++++---- >>>> 1 file changed, 19 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >>>> index 82a3299e53c0..4c5fb153e6b4 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c >>>> @@ -175,30 +175,44 @@ static int amdgpu_vram_mgr_init(struct >>>> ttm_mem_type_manager *man, >>>> ret = device_create_file(adev->dev, >>>> &dev_attr_mem_info_vram_total); >>>> if (ret) { >>>> DRM_ERROR("Failed to create device file >>>> mem_info_vram_total\n"); >>>> - return ret; >>>> + goto VRAM_TOTAL_FAIL; >>>> } >>>> ret = device_create_file(adev->dev, >>>> &dev_attr_mem_info_vis_vram_total); >>>> if (ret) { >>>> DRM_ERROR("Failed to create device file >>>> mem_info_vis_vram_total\n"); >>>> - return ret; >>>> + goto VIS_VRAM_TOTA_FAIL; >>>> } >>>> ret = device_create_file(adev->dev, >>>> &dev_attr_mem_info_vram_used); >>>> if (ret) { >>>> DRM_ERROR("Failed to create device file >>>> mem_info_vram_used\n"); >>>> - return ret; >>>> + goto VRAM_USED_FAIL; >>>> } >>>> ret = device_create_file(adev->dev, >>>> &dev_attr_mem_info_vis_vram_used); >>>> if (ret) { >>>> DRM_ERROR("Failed to create device file >>>> mem_info_vis_vram_used\n"); >>>> - return ret; >>>> + goto VIS_VRAM_USED_FAIL; >>>> } >>>> ret = device_create_file(adev->dev, >>>> &dev_attr_mem_info_vram_vendor); >>>> if (ret) { >>>> DRM_ERROR("Failed to create device file >>>> mem_info_vram_vendor\n"); >>>> - return ret; >>>> + goto VRAM_VERDOR_FAIL; >>>> } >>>> >>>> return 0; >>>> + >>>> +VRAM_VERDOR_FAIL: >>>> + device_remove_file(adev->dev, &dev_attr_mem_info_vis_vram_used); >>>> +VIS_VRAM_USED_FAIL: >>>> + device_remove_file(adev->dev, &dev_attr_mem_info_vram_used); >>>> +RVAM_USED_FAIL: >>>> + device_remove_file(adev->dev, &dev_attr_mem_info_vis_vram_total); >>>> +VIS_VRAM_TOTA_FAIL: >>>> + device_remove_file(adev->dev, &dev_attr_mem_info_vram_total); >>>> +VRAM_TOTAL_FAIL: >>>> + kfree(mgr); >>>> + man->priv = NULL; >>>> + >>>> + return ret; >>>> } >>>> >>>> /** >> > _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
