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

["Das, Nirmoy" <nirmoy.das@xxxxxxx>]

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.


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
>