[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <182e3214-0f60-8b65-5981-23d2a2ea54b9@nvidia.com>
Date: Tue, 31 Jan 2023 10:17:21 -0600
From: Shanker Donthineni <sdonthineni@...dia.com>
To: Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>, Michael Walle <michael@...le.cc>
Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Hans de Goede <hdegoede@...hat.com>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] genirq: Use hlist for managing resend handlers
On 1/31/23 02:59, Thomas Gleixner wrote:
> On Sun, Jan 29 2023 at 18:57, Shanker Donthineni wrote:
>> +/* hlist_head to handle software resend of interrupts: */
>> +static HLIST_HEAD(irq_resend_list);
>> +static DEFINE_RAW_SPINLOCK(irq_resend_lock);
>>
>> /*
>> * Run software resends of IRQ's
>> @@ -30,18 +31,16 @@ static DECLARE_BITMAP(irqs_resend, IRQ_BITMAP_BITS);
>> static void resend_irqs(struct tasklet_struct *unused)
>> {
>> struct irq_desc *desc;
>> - int irq;
>> -
>> - while (!bitmap_empty(irqs_resend, nr_irqs)) {
>> - irq = find_first_bit(irqs_resend, nr_irqs);
>> - clear_bit(irq, irqs_resend);
>> - desc = irq_to_desc(irq);
>> - if (!desc)
>> - continue;
>> + struct hlist_node *n;
>> +
>> + raw_spin_lock_irq(&irq_resend_lock);
>> + hlist_for_each_entry_safe(desc, n, &irq_resend_list, resend_node) {
>> + hlist_del_init(&desc->resend_node);
>> local_irq_disable();
>> desc->handle_irq(desc);
>> local_irq_enable();
>> }
>> + raw_spin_unlock_irq(&irq_resend_lock);
> The lock ordering is broken here. irq_sw_resend() is invoked with
> desc->lock held and takes irq_resend_lock.
>
> Lockdep clearly would have told you...
>
> raw_spin_lock_irq(&irq_resend_lock);
> while (!hlist_empty(...)) {
> desc = hlist_entry(irq_resend_list.first, ...);
> hlist_del_init(&desc->resend_node);
> raw_spin_unlock(&...);
> desc->handle_irq();
> raw_spin_lock(&...);
> }
Thanks for catching mistakes, I'll change it to this, please correct me if
I'm doing another mistake.
static void resend_irqs(struct tasklet_struct *unused)
{
struct irq_desc *desc;
raw_spin_lock_irq(&irq_resend_lock);
while (hlist_empty(&irq_resend_list)) {
desc = hlist_entry(irq_resend_list.first, struct irq_desc,
resend_node);
hlist_del_init(&desc->resend_node);
raw_spin_unlock(&irq_resend_lock);
desc->handle_irq(desc);
raw_spin_lock(&irq_resend_lock);
}
raw_spin_unlock_irq(&irq_resend_lock);
}
Thanks,
Shanker
Powered by blists - more mailing lists