[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <137bd5cc-0be9-6228-82e2-a75b53e5a2ae@huawei.com>
Date: Sat, 12 Nov 2022 17:19:50 +0800
From: Liu Shixin <liushixin2@...wei.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC: Thomas Gleixner <tglx@...utronix.de>,
"Rafael J . Wysocki" <rafael@...nel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] genirq/irqdesc: hide illegible sysfs warning of
kobject_del()
On 2022/11/12 16:59, Greg Kroah-Hartman wrote:
> On Sat, Nov 12, 2022 at 05:39:39PM +0800, Liu Shixin wrote:
>> If irq_sysfs_add() failed, system will report a warning but don't call
>> kobject_put() to release the descriptor.
> I can not parse this sentance :(
irq_sysfs_add() call kobject_add(). If kobject_add() failed, will print "Failed to add kobject for irq".
But not call kobject_put().
>
>> Then in irq_sysfs_del(), we continue to call kobject_del(). In such
>> situation, kobject_del() will complains about a object with no parent
>> like this:
> Then we should not be calling irq_sysfs_del() if the call failed. That
> is the real fix here.
If so, should I add a variable to record whether kobject has alreadly added or not?
>> 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
>> [...]
>>
>> Use kobj->state_in_sysfs to check whether kobject is added succeed. And
>> if not, we should not call kobject_del().
> That does not describe what you are doing here at all.
Sorry, I forget to update...
>
>> Signed-off-by: Liu Shixin <liushixin2@...wei.com>
>> ---
>> kernel/irq/irqdesc.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>> index a91f9001103c..a820d96210d4 100644
>> --- a/kernel/irq/irqdesc.c
>> +++ b/kernel/irq/irqdesc.c
>> @@ -300,10 +300,11 @@ static void irq_sysfs_del(struct irq_desc *desc)
>> /*
>> * If irq_sysfs_init() has not yet been invoked (early boot), then
>> * irq_kobj_base is NULL and the descriptor was never added.
>> + * And the descriptor may be added failed.
>> * kobject_del() complains about a object with no parent, so make
>> * it conditional.
>> */
>> - if (irq_kobj_base)
>> + if (irq_kobj_base && desc->kobj.parent)
> How would the parent be NULL? Parent devices always stick around until
> the child is removed, otherwise something is really wrong here. You
> should never have to look at the parent.
irq_sysfs_add() call kobject_add(). If kobject_add() failed, the parent will be NULL.
You can find the same check of kobj->parent in cpuid_cpu_offline().
thanks,
Liu Shixin
.
>
> thanks,
>
> greg k-h
> .
>
Powered by blists - more mailing lists