Re: [PATCH v3 5/5] watchdog: qcom: Register pretimeout interrupt as NMI

[Mayank Rungta]

On Fri, Sep 4, 2026 at 7:38 AM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> On Thu, Sep 3, 2026 at 1:58 PM Mayank Rungta <mrungta@xxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > > > When a system is completely unresponsive due to an interrupt storm or
> > > > deadlocked CPU cores with standard interrupts disabled, a standard
> > > > watchdog pretimeout bark interrupt will fail to execute, preventing the
> > > > pretimeout governor from capturing CPU backtraces before the hardware
> > > > reset bite.
> > >
> > > [...]
> > >
> > > > -                     ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > > > -                                            "wdt_bark", &wdt->wdd);
> > > > -                     if (ret)
> > > > -                             return ret;
> > > > +                     wdt->irq = irq;
> > > > +                     irq_flags = IRQF_PERCPU | IRQF_NOBALANCING |
> > > > +                                 IRQF_NO_AUTOEN | IRQF_NO_THREAD;
> > > > +
> > > > +                     ret = request_nmi(irq, qcom_wdt_isr, irq_flags,
> > > > +                                       "wdt_bark", &wdt->wdd);
> > > > +                     if (ret) {
> > > > +                             /* Fallback to normal interrupt if NMI not supported */
> > > > +                             ret = devm_request_irq(dev, irq, qcom_wdt_isr, 0,
> > > > +                                                    "wdt_bark", &wdt->wdd);
> > >
> > > GPT noticed this IRQ is requested but never enabled (NO_AUTOEN)
> >
> > For the NMI path: request_nmi() requires IRQF_NO_AUTOEN. To balance
> > this, enable_nmi(irq) is called immediately upon successful
> > registration in the probe function:
> >
> > For the IRQ fallback: devm_request_irq() passes 0 (not irq_flags). In
> > __setup_irq(), when IRQF_NO_AUTOEN is absent, the core calls
> > irq_startup(), automatically enabling the interrupt line as it has
> > always done in mainline.
> >
> > Both paths are therefore enabled upon probe.
>
> I wonder if it would make sense to either:
>
> 1. Rename the variable to 'nmi_flags'?
>
> 2. Get rid of the variable and just directly pass the flags to the request_nmi()
>
> That would avoid the confusion. While I agree that your code now is
> correct, I can understand why GPT (and humans) would get confused...
>
>
> -Doug

Agreed, renaming `irq_flags` to `nmi_flags` is a nice readability improvement.

However, since the way NMIs are requested is likely going to change
based on the ongoing discussion in patch 1, specifically regarding the
`IRQF_PERCPU` flag, I'll hold off on making changes here until we have
an agreement in place.

Thanks,
Mayank