All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
@ 2026-07-10 11:29 Srinivasan Shanmugam
  2026-07-10 12:34 ` Christian König
  2026-07-10 13:47 ` Zhu, Lingshan
  0 siblings, 2 replies; 8+ messages in thread
From: Srinivasan Shanmugam @ 2026-07-10 11:29 UTC (permalink / raw)
  To: Christian König, Alex Deucher
  Cc: amd-gfx, Srinivasan Shanmugam, Dan Carpenter, Zhu Lingshan,
	Felix Kuehling

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 <error27@gmail.com>
Cc: Zhu Lingshan <lingshan.zhu@amd.com>
Cc: Felix Kuehling <felix.kuehling@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
---
 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;
-	}
 
 	list_add(&entry->list, &procs);
 	kfd_debugfs_create_pasid_files(p, entry->proc_dentry);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
  2026-07-10 11:29 [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors Srinivasan Shanmugam
@ 2026-07-10 12:34 ` Christian König
  2026-07-10 13:47 ` Zhu, Lingshan
  1 sibling, 0 replies; 8+ messages in thread
From: Christian König @ 2026-07-10 12:34 UTC (permalink / raw)
  To: Srinivasan Shanmugam, Alex Deucher
  Cc: amd-gfx, Dan Carpenter, Zhu Lingshan, Felix Kuehling

On 7/10/26 13:29, 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 <error27@gmail.com>
> Cc: Zhu Lingshan <lingshan.zhu@amd.com>
> Cc: Felix Kuehling <felix.kuehling@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  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;
> -	}
>  
>  	list_add(&entry->list, &procs);
>  	kfd_debugfs_create_pasid_files(p, entry->proc_dentry);


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
  2026-07-10 11:29 [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors Srinivasan Shanmugam
  2026-07-10 12:34 ` Christian König
@ 2026-07-10 13:47 ` Zhu, Lingshan
  2026-07-10 13:57   ` Dan Carpenter
  1 sibling, 1 reply; 8+ messages in thread
From: Zhu, Lingshan @ 2026-07-10 13:47 UTC (permalink / raw)
  To: Srinivasan Shanmugam, Christian König, Alex Deucher
  Cc: amd-gfx, Dan Carpenter, Felix Kuehling

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

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 <error27@gmail.com>
> Cc: Zhu Lingshan <lingshan.zhu@amd.com>
> Cc: Felix Kuehling <felix.kuehling@amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> ---
>  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.

Thanks
Lingshan

>  
>  	list_add(&entry->list, &procs);
>  	kfd_debugfs_create_pasid_files(p, entry->proc_dentry);

[-- Attachment #2: Type: text/html, Size: 2303 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
  2026-07-10 13:47 ` Zhu, Lingshan
@ 2026-07-10 13:57   ` Dan Carpenter
  2026-07-10 14:08     ` Zhu, Lingshan
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2026-07-10 13:57 UTC (permalink / raw)
  To: Zhu, Lingshan
  Cc: Srinivasan Shanmugam, Christian König, Alex Deucher, amd-gfx,
	Felix Kuehling

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 <error27@gmail.com>
> > Cc: Zhu Lingshan <lingshan.zhu@amd.com>
> > Cc: Felix Kuehling <felix.kuehling@amd.com>
> > Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> > ---
> >  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.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
  2026-07-10 13:57   ` Dan Carpenter
@ 2026-07-10 14:08     ` Zhu, Lingshan
  2026-07-10 14:12       ` Dan Carpenter
  2026-07-10 14:45       ` Christian König
  0 siblings, 2 replies; 8+ messages in thread
From: Zhu, Lingshan @ 2026-07-10 14:08 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Srinivasan Shanmugam, Christian König, Alex Deucher, amd-gfx,
	Felix Kuehling

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

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 <error27@gmail.com>
>>> Cc: Zhu Lingshan <lingshan.zhu@amd.com>
>>> Cc: Felix Kuehling <felix.kuehling@amd.com>
>>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
>>> ---
>>>  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?

Thanks
Lingshan

>
> regards,
> dan carpenter
>

[-- Attachment #2: Type: text/html, Size: 3457 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
  2026-07-10 14:08     ` Zhu, Lingshan
@ 2026-07-10 14:12       ` Dan Carpenter
  2026-07-10 14:42         ` Zhu, Lingshan
  2026-07-10 14:45       ` Christian König
  1 sibling, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2026-07-10 14:12 UTC (permalink / raw)
  To: Zhu, Lingshan
  Cc: Srinivasan Shanmugam, Christian König, Alex Deucher, amd-gfx,
	Felix Kuehling

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 <error27@gmail.com>
> >>> Cc: Zhu Lingshan <lingshan.zhu@amd.com>
> >>> Cc: Felix Kuehling <felix.kuehling@amd.com>
> >>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> >>> ---
> >>>  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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
  2026-07-10 14:12       ` Dan Carpenter
@ 2026-07-10 14:42         ` Zhu, Lingshan
  0 siblings, 0 replies; 8+ messages in thread
From: Zhu, Lingshan @ 2026-07-10 14:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Srinivasan Shanmugam, Christian König, Alex Deucher, amd-gfx,
	Felix Kuehling

[-- Attachment #1: Type: text/plain, Size: 2829 bytes --]

On 7/10/2026 10:12 PM, Dan Carpenter wrote:

> 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 <error27@gmail.com>
>>>>> Cc: Zhu Lingshan <lingshan.zhu@amd.com>
>>>>> Cc: Felix Kuehling <felix.kuehling@amd.com>
>>>>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
>>>>> ---
>>>>>  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.

Thanks Dan for the information!

>
> regards,
> dan carpenter
>

[-- Attachment #2: Type: text/html, Size: 4139 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors
  2026-07-10 14:08     ` Zhu, Lingshan
  2026-07-10 14:12       ` Dan Carpenter
@ 2026-07-10 14:45       ` Christian König
  1 sibling, 0 replies; 8+ messages in thread
From: Christian König @ 2026-07-10 14:45 UTC (permalink / raw)
  To: Zhu, Lingshan, Dan Carpenter
  Cc: Srinivasan Shanmugam, Alex Deucher, amd-gfx, Felix Kuehling

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 <error27@gmail.com>
>>>> Cc: Zhu Lingshan <lingshan.zhu@amd.com>
>>>> Cc: Felix Kuehling <felix.kuehling@amd.com>
>>>> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
>>>> ---
>>>>  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
>>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-07-12 15:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 11:29 [PATCH] drm/amdkfd: Do not fail process debugfs setup on debugfs errors Srinivasan Shanmugam
2026-07-10 12:34 ` Christian König
2026-07-10 13:47 ` Zhu, Lingshan
2026-07-10 13:57   ` Dan Carpenter
2026-07-10 14:08     ` Zhu, Lingshan
2026-07-10 14:12       ` Dan Carpenter
2026-07-10 14:42         ` Zhu, Lingshan
2026-07-10 14:45       ` Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.