RE: [RFC 06/12] genirq: Add per-cpu flow handler with conditional IRQ stats

[Michael Kelley]

From: Thomas Gleixner <tglx@xxxxxxxxxxxxx> Sent: Tuesday, June 4, 2024 11:14 AM
> 
> Michael!
> 
> On Mon, Jun 03 2024 at 22:09, mhkelley58@xxxxxxxxx wrote:
> > Hyper-V VMBus devices generate interrupts that are multiplexed
> > onto a single per-CPU architectural interrupt. The top-level VMBus
> > driver ISR demultiplexes these interrupts and invokes per-device
> > handlers. Currently, these per-device handlers are not modeled as
> > Linux IRQs, so /proc/interrupts shows all VMBus interrupts as accounted
> > to the top level architectural interrupt. Visibility into per-device
> > interrupt stats requires accessing VMBus-specific entries in sysfs.
> > The top-level VMBus driver ISR also handles management-related
> > interrupts that are not attributable to a particular VMBus device.
> >
> > As part of changing VMBus to model VMBus per-device handlers as
> > normal Linux IRQs, the top-level VMBus driver needs to conditionally
> > account for interrupts. If it passes the interrupt off to a
> > device-specific IRQ, the interrupt stats are done by that IRQ
> > handler, and accounting for the interrupt at the top level
> > is duplicative. But if it handles a management-related interrupt
> > itself, then it should account for the interrupt itself.
> >
> > Introduce a new flow handler that provides this functionality.
> > The new handler parallels handle_percpu_irq(), but does stats
> > only if the ISR returns other than IRQ_NONE. The existing
> > handle_untracked_irq() can't be used because it doesn't work for
> > per-cpu IRQs, and it doesn't provide conditional stats.
> 
> There is a two other options to solve this:
> 

Thanks for taking a look.  Unfortunately, unless I'm missing something,
both options you suggest have downsides.

>    1) Move the inner workings of handle_percpu_irq() out into
>       a static function which returns the 'handled' value and
>       share it between the two handler functions.

The "inner workings" aren't quite the same in the two cases.
handle_percpu_irq() uses handle_irq_event_percpu() while
handle_percpu_demux_irq() uses __handle_irq_event_percpu().
The latter doesn't do add_interrupt_randomness() because the
demultiplexed IRQ handler will do it.  Doing add_interrupt_randomness()
twice doesn't break anything, but it's more overhead in the hard irq
path, which I'm trying to avoid.  The extra functionality in the
non-double-underscore version could be hoisted up to
handle_percpu_irq(), but that offsets gains from sharing the
inner workings.

> 
>    2) Allocate a proper interrupt for the management mode and invoke it
>       via generic_handle_irq() just as any other demultiplex interrupt.
>       That spares all the special casing in the core code and just
>       works.

Yes, this would work on x86, as the top-level interrupt isn't a Linux IRQ,
and the interrupt counting is done in Hyper-V specific code that could be
removed.  The demux'ed interrupt does the counting.

But on arm64 the top-level interrupt *is* a Linux IRQ, so each
interrupt will get double-counted, which is a problem.  Having to add
handle_percpu_demux_irq() to handle arm64 correctly isn't as clean
as I wish it could be.  But I couldn't find a better approach.

Michael