[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y4Tt2NIcPFeoss4s@kroah.com>
Date: Mon, 28 Nov 2022 18:20:24 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Yang Yingliang <yangyingliang@...wei.com>
Cc: tglx@...utronix.de, kraig@...gle.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] genirq/irqdesc: fix WARNING in irq_sysfs_del()
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@...wei.com>
> ---
> 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
Powered by blists - more mailing lists