[PATCH 1/2] drm/ttm: Check ttm_debugfs_root before creating files under it

[Christian König]

Am 16.01.23 um 03:44 schrieb Ma, Jun:
> On 1/13/2023 5:37 PM, Christian König wrote:
>> Am 13.01.23 um 06:34 schrieb Ma Jun:
>>> Check the ttm_debugfs_root before creating files under it.
>>> If the ttm_debugfs_root is NULL, all the files created for
>>> ttm/ will be placed in the /sys/kerne/debug/ but not
>>> /sys/kernel/debug/ttm/
>> Well NAK for upstreaming. Why should ttm_debugfs_root be NULL here?
>>
> In my case, when the ttm/ removal fails during amdgpu uninstall and then
> we try to modprobe the amdgpu again, the ttm_debugfs_root will be NULL
> because the ttm/ already exists.

Yeah, but this is just papering over a previous bug which in turn is not 
a valid argument for a code change.

What should happen instead is that the original bug needs to get fixed.

Regards,
Christian.

>
> Regards,
> Ma Jun
>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Ma Jun <Jun.Ma2 at amd.com>
>>> ---
>>>    drivers/gpu/drm/ttm/ttm_device.c |  3 ++-
>>>    drivers/gpu/drm/ttm/ttm_pool.c   | 10 ++++++----
>>>    drivers/gpu/drm/ttm/ttm_tt.c     |  5 +++--
>>>    3 files changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
>>> index e7147e304637..967bc2244df3 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>>> @@ -105,7 +105,8 @@ 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,
>>> +	if(ttm_debugfs_root)
>>> +		debugfs_create_atomic_t("buffer_objects", 0444, ttm_debugfs_root,
>>>    				&glob->bo_count);
>>>    out:
>>>    	if (ret && ttm_debugfs_root)
>>> diff --git a/drivers/gpu/drm/ttm/ttm_pool.c b/drivers/gpu/drm/ttm/ttm_pool.c
>>> index 21b61631f73a..d95a65f759df 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_pool.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_pool.c
>>> @@ -713,10 +713,12 @@ int ttm_pool_mgr_init(unsigned long num_pages)
>>>    	}
>>>    
>>>    #ifdef CONFIG_DEBUG_FS
>>> -	debugfs_create_file("page_pool", 0444, ttm_debugfs_root, NULL,
>>> -			    &ttm_pool_debugfs_globals_fops);
>>> -	debugfs_create_file("page_pool_shrink", 0400, ttm_debugfs_root, NULL,
>>> -			    &ttm_pool_debugfs_shrink_fops);
>>> +	if(ttm_debugfs_root) {
>>> +		debugfs_create_file("page_pool", 0444, ttm_debugfs_root, NULL,
>>> +				    &ttm_pool_debugfs_globals_fops);
>>> +		debugfs_create_file("page_pool_shrink", 0400, ttm_debugfs_root, NULL,
>>> +				    &ttm_pool_debugfs_shrink_fops);
>>> +	}
>>>    #endif
>>>    
>>>    	mm_shrinker.count_objects = ttm_pool_shrinker_count;
>>> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
>>> index d505603930a7..fec443494ef0 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_tt.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
>>> @@ -394,8 +394,9 @@ DEFINE_SHOW_ATTRIBUTE(ttm_tt_debugfs_shrink);
>>>    void ttm_tt_mgr_init(unsigned long num_pages, unsigned long num_dma32_pages)
>>>    {
>>>    #ifdef CONFIG_DEBUG_FS
>>> -	debugfs_create_file("tt_shrink", 0400, ttm_debugfs_root, NULL,
>>> -			    &ttm_tt_debugfs_shrink_fops);
>>> +	if(ttm_debugfs_root)
>>> +		debugfs_create_file("tt_shrink", 0400, ttm_debugfs_root, NULL,
>>> +				    &ttm_tt_debugfs_shrink_fops);
>>>    #endif
>>>    
>>>    	if (!ttm_pages_limit)