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

[Das, Nirmoy]

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.


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
>