Messages in this thread | | | Date | Tue, 31 Jan 2023 10:17:21 -0600 | Subject | Re: [PATCH 1/5] genirq: Use hlist for managing resend handlers | From | Shanker Donthineni <> |
| |
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
| |