Re: [PATCH] kobject: hide illegible sysfs warning of kobject_del()

Re: [PATCH] kobject: hide illegible sysfs warning of kobject_del()

[Greg Kroah-Hartman]

On Fri, Nov 11, 2022 at 02:58:07PM +0800, Liu Shixin wrote:
> Some consumers do not care whether kobject_add() succeed or failed such as
> irqdesc. They call kobject_del() all the time even if kobject_add() failed.
> Then kernel will report some illegible sysfs warning like this:
> 
>  kernfs: can not remove 'actions', no directory
>  WARNING: CPU: 0 PID: 277 at fs/kernfs/dir.c:1615 kernfs_remove_by_name_ns+0xd5/0xe0

Why not fix the caller here?  Is that somehow not possible?

thanks,

greg k-h

[PATCH] kobject: hide illegible sysfs warning of kobject_del()

[Liu Shixin]

Some consumers do not care whether kobject_add() succeed or failed such as
irqdesc. They call kobject_del() all the time even if kobject_add() failed.
Then kernel will report some illegible sysfs warning like this:

 kernfs: can not remove 'actions', no directory
 WARNING: CPU: 0 PID: 277 at fs/kernfs/dir.c:1615 kernfs_remove_by_name_ns+0xd5/0xe0
[...]
 Call Trace:
  <TASK>
  remove_files.isra.0+0x3f/0xb0
  sysfs_remove_group+0x68/0xe0
  sysfs_remove_groups+0x41/0x70
  __kobject_del+0x45/0xc0
  kobject_del+0x2a/0x40
  free_desc+0x44/0x70
  irq_free_descs+0x5d/0x90
[...]

Check whether kobject is added successfully by using kobj->state_in_sysfs
in kobject_del() and skip deleting it if not added at all.

Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 lib/kobject.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index a0b2dbfcfa23..f6163a3a41c2 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -604,7 +604,7 @@ void kobject_del(struct kobject *kobj)
 {
 	struct kobject *parent;
 
-	if (!kobj)
+	if (!kobj || !kobj->state_in_sysfs)
 		return;
 
 	parent = kobj->parent;
-- 
2.25.1

Re: [PATCH] kobject: hide illegible sysfs warning of kobject_del()

[Liu Shixin]

On 2022/11/11 14:26, Greg Kroah-Hartman wrote:
> On Fri, Nov 11, 2022 at 02:58:07PM +0800, Liu Shixin wrote:
>> Some consumers do not care whether kobject_add() succeed or failed such as
>> irqdesc. They call kobject_del() all the time even if kobject_add() failed.
>> Then kernel will report some illegible sysfs warning like this:
>>
>>  kernfs: can not remove 'actions', no directory
>>  WARNING: CPU: 0 PID: 277 at fs/kernfs/dir.c:1615 kernfs_remove_by_name_ns+0xd5/0xe0
> Why not fix the caller here?  Is that somehow not possible?
The caller should be freed by kobject_put() if kobject_add() failed. But in fact, the failure does not affect
the function of the caller. So the caller do not call kobject_put() Immediately.
If want to fix the caller, we can check konj->state_in_sysfs before call kobject_del(). This way has no difference
with check kobj->state_in_sysfs in kobject_del().
By the way, I'm not sure how many callers have this problem. So I think it's better to fix in kobject_del().

thanks,

>
> thanks,
>
> greg k-h
> .
>

Re: [PATCH] kobject: hide illegible sysfs warning of kobject_del()

[Greg Kroah-Hartman]

On Fri, Nov 11, 2022 at 04:27:03PM +0800, Liu Shixin wrote:
> 
> 
> On 2022/11/11 14:26, Greg Kroah-Hartman wrote:
> > On Fri, Nov 11, 2022 at 02:58:07PM +0800, Liu Shixin wrote:
> >> Some consumers do not care whether kobject_add() succeed or failed such as
> >> irqdesc. They call kobject_del() all the time even if kobject_add() failed.
> >> Then kernel will report some illegible sysfs warning like this:
> >>
> >>  kernfs: can not remove 'actions', no directory
> >>  WARNING: CPU: 0 PID: 277 at fs/kernfs/dir.c:1615 kernfs_remove_by_name_ns+0xd5/0xe0
> > Why not fix the caller here?  Is that somehow not possible?
> The caller should be freed by kobject_put() if kobject_add() failed. But in fact, the failure does not affect
> the function of the caller. So the caller do not call kobject_put() Immediately.
> If want to fix the caller, we can check konj->state_in_sysfs before call kobject_del(). This way has no difference
> with check kobj->state_in_sysfs in kobject_del().

No, no code should ever be checking the state_in_sysfs flag before
calling this function.

When a kobject is done with, by the creator of it, then it can call
kobject_del().  It can NOT call that function multiple times, as that is
just wrong and goes against the whole way the kobject should be used.

So there is something very wrong with the caller code here, THAT should
be fixed.

> By the way, I'm not sure how many callers have this problem. So I think it's better to fix in kobject_del().

As we have never had this report before, I don't know of many problem
users.

Let's fix the root of the problem here please, do not paper over it by
allowing this function to be called multiple times, as that is an
indication that the reference counting logic of the caller is very
wrong.

thanks,

greg k-h