On 7/10/26 16:08, Zhu, Lingshan wrote:
> On 7/10/2026 9:57 PM, Dan Carpenter wrote:
>
>> On Fri, Jul 10, 2026 at 09:47:32PM +0800, Zhu, Lingshan wrote:
>>> On 7/10/2026 7:29 PM, Srinivasan Shanmugam wrote:
>>>
>>>> debugfs is intended for debugging only, and failures to create debugfs
>>>> entries should not affect normal operation.
>>>>
>>>> Remove the check for debugfs_create_dir() in kfd_debugfs_add_process().
>>>> If debugfs entries cannot be created, continue without them instead of
>>>> reporting an unnecessary error.
>>>>
>>>> Fixes: 22ab1bb3994a ("amdkfd: expose pasid of secondary contexts by
>>>> debugfs")
>>>> Reported-by: Dan Carpenter <[email protected]>
>>>> Cc: Zhu Lingshan <[email protected]>
>>>> Cc: Felix Kuehling <[email protected]>
>>>> Signed-off-by: Srinivasan Shanmugam <[email protected]>
>>>> ---
>>>> drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c | 4 ----
>>>> 1 file changed, 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
>>>> b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
>>>> index 02673f01b448..7c5bc9c4559a 100644
>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_debugfs.c
>>>> @@ -211,10 +211,6 @@ int kfd_debugfs_add_process(struct kfd_process *p)
>>>> entry->proc_dentry = debugfs_create_dir(name,
>>>>
>>>> primary_entry->proc_dentry);
>>>> }
>>>> - if (IS_ERR_OR_NULL(entry->proc_dentry)) {
>>>> - ret = entry->proc_dentry ? PTR_ERR(entry->proc_dentry) :
>>>> -ENOMEM;
>>>> - goto err_free_entry;
>>>> - }
>>> We need this check because debugfs_create_dir() may fail.
>>> Removing this check leads to entry leaking when fail.
>>>
>> Debugfs functions aren't supposed to be checked. Drivers aren't
>> supposed to rely on debugfs so it's not required. If debugfs is
>> failing then you are pretty much screwed anyway.
>>
>> I have a blog about the history of this:
>> https://staticthinking.wordpress.com/2023/07/24/debugfs-functions-are-not-supposed-to-be-checked/
>>
>> This code is inside #if defined(CONFIG_DEBUG_FS) so the check
>> isn't harmful except that it sets a bad example. Back in the day,
>> this used to be a source of pain for me so Greg made it hard to
>> write a correct check unless you have the #ifdef. It's to
>> discourage checking.
>
> Thanks Dan for the background, then how do we prevent the entry leaking if we
> do not check debugfs_create_dir?
As far as I can see you don't need to. When debugfs_create_dir() fails all
following entry creation fails as well.
Or what exactly is the use case here?
Regards,
Christian.
>
> Thanks
> Lingshan
>
>> regards,
>> dan carpenter
>>
