[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0804e457-27ae-a046-a3f5-73497511c841@nvidia.com>
Date: Tue, 31 Jan 2023 11:06: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 10:17, Shanker Donthineni wrote:
>
>
> 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)) {
Sorry typo "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