[AMD Official Use Only]
> -----邮件原件-----
> 发件人: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> 发送时间: Tuesday, October 19, 2021 4:46 PM
> 收件人: Yang, Stanley <Stanley.Yang@xxxxxxx>; Das, Nirmoy
> <Nirmoy.Das@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> 主题: Re: 回复: [PATCH Review 1/1] drm/ttm: fix debugfs node create failed
>
> Am 19.10.21 um 10:02 schrieb Yang, Stanley:
> > [AMD Official Use Only]
> >
> >
> >> -----邮件原件-----
> >> 发件人: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> 代表 Das,
> Nirmoy
> >> 发送时间: Thursday, October 14, 2021 2:11 AM
> >> 收件人: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>; amd-
> >> gfx@xxxxxxxxxxxxxxxxxxxxx
> >> 主题: Re: [PATCH Review 1/1] drm/ttm: fix debugfs node create failed
> >>
> >>
> >> On 10/13/2021 2:29 PM, Christian König wrote:
> >>> Am 12.10.21 um 15:12 schrieb Das, Nirmoy:
> >>>> On 10/12/2021 1:58 PM, Stanley.Yang wrote:
> >>>>> Test scenario:
> >>>>> modprobe amdgpu -> rmmod amdgpu -> modprobe amdgpu Error
> log:
> >>>>> [ 54.396807] debugfs: File 'page_pool' in directory 'amdttm'
> >>>>> already present!
> >>>>> [ 54.396833] debugfs: File 'page_pool_shrink' in directory
> >>>>> 'amdttm' already present!
> >>>>> [ 54.396848] debugfs: File 'buffer_objects' in directory
> >>>>> 'amdttm' already present!
> >>>>
> >>>> We should instead add a check if those debugfs files already
> >>>> exist/created in ttm debugfs dir using debugfs_lookup() before creating.
> >>> No, IIRC the Intel guys had fixed that already by adding/removing
> >>> the debugfs file on module load/unload.
> >>
> >> Adding/removing on ttm module load/unload is nicer.
> > The point is that page_pool, page_pool_shrink and buffer_objects are
> > created by amdgpu driver,
>
> Yeah, but the debugfs files are not created by the driver. Those are global to
> TTM and can trivially be created during module load/unload.
[Yang, Stanley] Thanks Christian, I double check ttm related code the ttm load will create those debugfs file.
Stanley
>
> Christian.
>
> > I think it's better to remove them by amdgpu module due to amdgpu
> > module create them, otherwise, there will be a scene create them failed
> only reload amdgpu module.
> >
> > Stanley
> >>
> >> Nirmoy
> >>
> >>>
> >>> Christian.
> >>>
> >>>>
> >>>> Regards,
> >>>>
> >>>> Nirmoy
> >>>>
> >>>>
> >>>>
> >>>>> Reason:
> >>>>> page_pool, page_pool_shrink and buffer_objects can be
> >>>>> removed when
> >>>>> rmmod amdttm, in the above test scenario only rmmod amdgpu,
> >>>>> so those
> >>>>> debugfs node will not be removed, this caused file create failed.
> >>>>> Soultion:
> >>>>> create ttm_page directory under ttm_root directory when
> >>>>> insmod amdgpu,
> >>>>> page_pool, page_pool_shrink and buffer_objects are stored in
> >>>>> ttm_page directiry,
> >>>>> remove ttm_page directory when do rmmod amdgpu, this can fix
> >>>>> above issue.
> >>>>>
> >>>>> Signed-off-by: Stanley.Yang <Stanley.Yang@xxxxxxx>
> >>>>> ---
> >>>>> drivers/gpu/drm/ttm/ttm_device.c | 12 +++++++++++-
> >>>>> drivers/gpu/drm/ttm/ttm_module.c | 1 +
> >>>>> drivers/gpu/drm/ttm/ttm_module.h | 1 +
> >>>>> drivers/gpu/drm/ttm/ttm_pool.c | 4 ++--
> >>>>> 4 files changed, 15 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
> >>>>> b/drivers/gpu/drm/ttm/ttm_device.c
> >>>>> index 1de23edbc182..ad170328f0c8 100644
> >>>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
> >>>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> >>>>> @@ -55,6 +55,10 @@ static void ttm_global_release(void)
> >>>>> ttm_pool_mgr_fini();
> >>>>> +#ifdef CONFIG_DEBUG_FS
> >>>>> + debugfs_remove(ttm_debugfs_page); #endif
> >>>>> +
> >>>>> __free_page(glob->dummy_read_page);
> >>>>> memset(glob, 0, sizeof(*glob));
> >>>>> out:
> >>>>> @@ -85,6 +89,10 @@ static int ttm_global_init(void)
> >>>>> >> PAGE_SHIFT;
> >>>>> num_dma32 = min(num_dma32, 2UL << (30 - PAGE_SHIFT));
> >>>>> +#ifdef CONFIG_DEBUG_FS
> >>>>> + ttm_debugfs_page = debugfs_create_dir("ttm_page",
> >>>>> ttm_debugfs_root);
> >>>>> +#endif
> >>>>> +
> >>>>> ttm_pool_mgr_init(num_pages);
> >>>>> ttm_tt_mgr_init(num_pages, num_dma32);
> >>>>> @@ -98,8 +106,10 @@ static int ttm_global_init(void)
> >>>>> INIT_LIST_HEAD(&glob->device_list);
> >>>>> atomic_set(&glob->bo_count, 0);
> >>>>> - debugfs_create_atomic_t("buffer_objects", 0444,
> >>>>> ttm_debugfs_root,
> >>>>> +#ifdef CONFIG_DEBUG_FS
> >>>>> + debugfs_create_atomic_t("buffer_objects", 0444,
> >>>>> +ttm_debugfs_page,
> >>>>> &glob->bo_count);
> >>>>> +#endif
> >>>>> out:
> >>>>> mutex_unlock(&ttm_global_mutex);
> >>>>> return ret;
> >>>>> diff --git a/drivers/gpu/drm/ttm/ttm_module.c
> >>>>> b/drivers/gpu/drm/ttm/ttm_module.c
> >>>>> index 88970a6b8e32..66595e6e7087 100644
> >>>>> --- a/drivers/gpu/drm/ttm/ttm_module.c
> >>>>> +++ b/drivers/gpu/drm/ttm/ttm_module.c
> >>>>> @@ -38,6 +38,7 @@
> >>>>> #include "ttm_module.h"
> >>>>> struct dentry *ttm_debugfs_root;
> >>>>> +struct dentry *ttm_debugfs_page;
> >>>>> static int __init ttm_init(void)
> >>>>> {
> >>>>> diff --git a/drivers/gpu/drm/ttm/ttm_module.h
> >>>>> b/drivers/gpu/drm/ttm/ttm_module.h
> >>>>> index d7cac5d4b835..6007dc66f44e 100644
> >>>>> --- a/drivers/gpu/drm/ttm/ttm_module.h
> >>>>> +++ b/drivers/gpu/drm/ttm/ttm_module.h
> >>>>> @@ -36,5 +36,6 @@
> >>>>> struct dentry;
> >>>>> extern struct dentry *ttm_debugfs_root;
> >>>>> +extern struct dentry *ttm_debugfs_page;
> >>>>> #endif /* _TTM_MODULE_H_ */
> >>>>> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
> >>>>> b/drivers/gpu/drm/ttm/ttm_pool.c index
> 8be7fd7161fd..ecb33daad7b5
> >>>>> 100644
> >>>>> --- a/drivers/gpu/drm/ttm/ttm_pool.c
> >>>>> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> >>>>> @@ -709,9 +709,9 @@ int ttm_pool_mgr_init(unsigned long
> num_pages)
> >>>>> }
> >>>>> #ifdef CONFIG_DEBUG_FS
> >>>>> - debugfs_create_file("page_pool", 0444, ttm_debugfs_root,
> >>>>> NULL,
> >>>>> + debugfs_create_file("page_pool", 0444, ttm_debugfs_page,
> >>>>> +NULL,
> >>>>> &ttm_pool_debugfs_globals_fops);
> >>>>> - debugfs_create_file("page_pool_shrink", 0400,
> >>>>> ttm_debugfs_root, NULL,
> >>>>> + debugfs_create_file("page_pool_shrink", 0400,
> >>>>> +ttm_debugfs_page,
> >>>>> NULL,
> >>>>> &ttm_pool_debugfs_shrink_fops);
> >>>>> #endif
