Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 31 Jan 2023 11:06:21 -0600 | Subject | Re: [PATCH 1/5] genirq: Use hlist for managing resend handlers | From | Shanker Donthineni <> |
| |
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
| ![\](/images/icornerr.gif) |