回复: [PATCH Review 1/1] drm/ttm: fix debugfs node create failed

[Yang, Stanley]

[AMD Official Use Only]


> -----邮件原件-----
> 发件人: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> 代表 Das,
> Nirmoy
> 发送时间: Thursday, October 14, 2021 2:11 AM
> 收件人: Christian König <ckoenig.leichtzumerken at gmail.com>; amd-
> gfx at lists.freedesktop.org
> 主题: 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, 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 at amd.com>
> >>> ---
> >>>   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
> >