Re: [PATCH v2] genirq/irqdesc: fix WARNING in irq_sysfs_del()

From: Greg KH
Date: Mon Nov 28 2022 - 12:20:37 EST


On Mon, Nov 28, 2022 at 11:16:12PM +0800, Yang Yingliang wrote:
> I got the lots of WARNING report when doing fault injection test:
>
> kernfs: can not remove 'chip_name', no directory
> WARNING: CPU: 0 PID: 253 at fs/kernfs/dir.c:1616 kernfs_remove_by_name_ns+0xce/0xe0
> RIP: 0010:kernfs_remove_by_name_ns+0xce/0xe0
> Call Trace:
> <TASK>
> remove_files.isra.1+0x3f/0xb0
> sysfs_remove_group+0x68/0xe0
> sysfs_remove_groups+0x41/0x70
> __kobject_del+0x45/0xc0
> kobject_del+0x29/0x40
> free_desc+0x42/0x70
> irq_free_descs+0x5e/0x90
>
> kernfs: can not remove 'hwirq', no directory
> WARNING: CPU: 0 PID: 253 at fs/kernfs/dir.c:1616 kernfs_remove_by_name_ns+0xce/0xe0
> RIP: 0010:kernfs_remove_by_name_ns+0xce/0xe0
> Call Trace:
> <TASK>
> remove_files.isra.1+0x3f/0xb0
> sysfs_remove_group+0x68/0xe0
> sysfs_remove_groups+0x41/0x70
> __kobject_del+0x45/0xc0
> kobject_del+0x29/0x40
> free_desc+0x42/0x70
> irq_free_descs+0x5e/0x90
>
> If irq_sysfs_add() fails in alloc_descs(), the directory of interrupt
> informations is not added to sysfs, it causes the WARNINGs when removing
> the information files. Add 'sysfs_added' field in struct irq_desc to
> indicate if it is added, and check it before calling kobject_del() to
> avoid these WARNINGs.
>
> Fixes: ecb3f394c5db ("genirq: Expose interrupt information through sysfs")
> Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>
> ---
> v1 -> v2:
> Don't use state_in_sysfs, introduce 'sysfs_added' to indicate if it is added.
> ---
> include/linux/irqdesc.h | 1 +
> kernel/irq/irqdesc.c | 7 +++++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
> index 844a8e30e6de..fec0f3946a34 100644
> --- a/include/linux/irqdesc.h
> +++ b/include/linux/irqdesc.h
> @@ -97,6 +97,7 @@ struct irq_desc {
> #ifdef CONFIG_SPARSE_IRQ
> struct rcu_head rcu;
> struct kobject kobj;
> + bool sysfs_added;
> #endif
> struct mutex request_mutex;
> int parent_irq;
> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
> index a91f9001103c..9bf74d11bad5 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -292,6 +292,8 @@ static void irq_sysfs_add(int irq, struct irq_desc *desc)
> */
> if (kobject_add(&desc->kobj, irq_kobj_base, "%d", irq))
> pr_warn("Failed to add kobject for irq %d\n", irq);
> + else
> + desc->sysfs_added = true;

Wait, no. Why are you just not properly failing and unwinding here?
Why do you need a special flag just to say "sysfs worked" or not unlike
all other users of kobjects.

Fix this up properly please.

thanks,

greg k-h