Messages in this thread | | | From | Vincent Whitchurch <> | Subject | Re: [PATCH v2] genirq: Fix nested thread vs synchronize_hardirq() deadlock | Date | Mon, 3 Jul 2023 09:28:52 +0000 |
| |
On Fri, 2023-06-30 at 11:07 +0200, Thomas Gleixner wrote: > On Tue, Jun 20 2023 at 13:16, Vincent Whitchurch wrote: > > --- a/kernel/irq/chip.c > > +++ b/kernel/irq/chip.c > > @@ -476,7 +476,7 @@ void handle_nested_irq(unsigned int irq) > > } > > > > > > kstat_incr_irqs_this_cpu(desc); > > - irqd_set(&desc->irq_data, IRQD_IRQ_INPROGRESS); > > + atomic_inc(&desc->threads_active); > > raw_spin_unlock_irq(&desc->lock); > > > > > > action_ret = IRQ_NONE; > > @@ -487,7 +487,8 @@ void handle_nested_irq(unsigned int irq) > > note_interrupt(desc, action_ret); > > > > > > raw_spin_lock_irq(&desc->lock); > > - irqd_clear(&desc->irq_data, IRQD_IRQ_INPROGRESS); > > + if (atomic_dec_and_test(&desc->threads_active)) > > + wake_up(&desc->wait_for_threads); > > This breaks on RT. The wakeup cannot be inside the raw spin-locked > region.
OK. I will remove the acquisition of the spin lock at the end of this function since it was only used for protecting the irqd flags.
> Also this is open coding wake_threads_waitq().
OK, that's in manage.c so I'll make it non-static and add it to internals.h and use it from here.
> > +static void __synchronize_irq(struct irq_desc *desc) > > +{ > > + __synchronize_hardirq(desc, true); > > + /* > > + * We made sure that no hardirq handler is > > + * running. Now verify that no threaded handlers are > > + * active. > > + */ > > + wait_event(desc->wait_for_threads, > > + !atomic_read(&desc->threads_active)); > > Splitting this out is fine. Not reformatting it not so much.
Will fix.
| |