On Fri, Jul 10, 2026 at 10:08:11PM +0800, 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?
We just don't worry about it. If that happens, you probably are going
to need to do a kernel upgrade or replace your ram or something. A
small leak is the least of your worries.
This is the answer that Greg told me back in the day.
regards,
dan carpenter
