From: Naman Jain <[email protected]> Sent: Wednesday, October 21, 
2026 11:41 PM
> 
> On 10/20/2026 11:54 PM, Michael Kelley wrote:
> > In a system with multiple devices with the same configuration (such as
> > NVMe controllers), the affinity masks are the same for all such devices.
> > Linux typicaly must select a single CPU from each mask to be the effective
> > affinity. If a fixed algorithm is used, such as picking the first CPU
> > in the mask, interrupts from multiple devices are assigned to the same
> > CPUs, while other CPUs are not assigned any interrupts, producing an
> > interrupt load imbalance.
> >
> > On x86, the "APIC" IRQ chip avoids this problem by spreading the
> > interrupts across the CPUs in an affinity mask. It tracks the number
> > of IRQs assigned to each CPU, and chooses the CPU in the mask that
> > has the fewest assignments. Because Hyper-V vPCI device IRQs eventually
> > parent to APIC, the spreading done by APIC accrues to Hyper-V vPCI
> > devices, so all is good in x86 systems.
> >
> > On arm64, the "ITS" IRQ chip for the GICv3+ITS does the same kind of
> > spreading for LPI interrupts. But Hyper-V provides guest VMs only with
> > a virtual GICv3, not including an ITS or LPI support. IRQs for vPCI
> > devices on arm64 are mapped to SPIs and parented to the "GICv3" IRQ
> > chip. The GICv3 IRQ chip always chooses the first CPU in the affinity
> > mask as the effective affinity, producing an interrupt load imbalance
> > for Hyper-V vPCI devices in arm64 systems.
> >
> > The Hyper-V vPCI driver already has an arm64-specific IRQ chip to
> > allocate and managed SPIs for use by vPCI devices. Solve the imbalance
> > problem by extending this IRQ chip to implement a "set affinity"
> > function that tracks SPIs assigned to each CPU, and chooses the
> > least-loaded CPU as the effective affinity. Then invoke the parent
> > IRQ chip (GICv3) to actually set that single CPU as the effective
> > affinity in the virtual GICv3 hardware. As a result, IRQs are spread
> > across CPUs like on x86 and interrupt load imbalances are avoided.
> >
> > Signed-off-by: Michael Kelley <[email protected]>
> 
> The overall changes are well explained and LGTM. I saw your comment
> about Sashiko review, so that is covered.
> Mentioning some minor nits and review comments inline.
> 
> You may also want to look at some of the checkpatch "strict" warnings
> with these changes. Since you sent it from a different email id than
> what is signed-off by, you may not have seen that mismatch warning in
> local run.

I'll clean up the "strict" warnings in v2.

> 
> > ---
> > As an alternative, this spreading functionality could be implemented
> > for SPIs directly in the GICv3 IRQ chip, similar to what the ITS IRQ
> > chip already has for LPIs. While Hyper-V's scheme of using SPIs for
> > vPCI interrupts is allowed by the arm64 architecture, it's atypical.
> > So the original decision was for the Hyper-V vPCI driver to handle
> > the allocation and management of SPIs for this purpose rather than do
> > it in generic GIC code. That led of the introduction of the "MSI" IRQ
> > chip in the Hyper-V vPCI driver. This implementation of the spreading
> > functionality follows that original decision by extending the "MSI"
> > IRQ chip. If reviewers have reasons to prefer enhancing the GICv3 IRQ
> > chip driver instead, I'm open to doing that.
> >
> > I have not added any documentation to Documentation/virt/hyperv/vpci.rst.
> > That documentation needs an entirely new section on the IRQ domains
> > and chips, and their hierarchy differences on x86 and arm64. I'm
> > working on a separate patch to add that documentation. But I don't
> > want to hold up this improvement for that larger documentation task.
> >
> >   drivers/pci/controller/pci-hyperv.c | 144 +++++++++++++++++++++++++++-
> >   1 file changed, 142 insertions(+), 2 deletions(-)
> >
> 
> IRQ-affinity comparison with and without your changes:
> 
> System:
>    Azure Standard_D64plds_v6
>    64 ARM Neoverse-N2 vCPUs, no SMT, one NUMA node.
>    Four independent Microsoft NVMe Direct Disk v2 controllers,
>    approximately 945 GB each, with 14 I/O queues/controller.
>    Hyper-V GICv3 SPI routing; no ITS/LPIs.
> 
> Both kernels use next-20261021.
> 
>                               Vanilla       With patch
>    Data IRQ vectors              56              56
>    Effective target CPUs         14              56
>    Maximum data IRQs/CPU         4               1
> 
> The requested affinity masks remain unchanged. Entries below are
> measured effective target CPUs. Columns identify the four controllers
> by stable serial suffix, not their boot-dependent nvmeX names.
> 
>               Vanilla                    With patch
> Queue   0001 0002 0003 0004        0001 0002 0003 0004
> q1         0    0    0    0           4    2    1    3
> q2         5    5    5    5           8    7    5    6
> q3        10   10   10   10          13   10   12   11
> q4        15   15   15   15          18   15   17   16
> q5        20   20   20   20          23   20   22   21
> q6        25   25   25   25          28   25   27   26
> q7        30   30   30   30          33   30   31   32
> q8        35   35   35   35          38   35   36   37
> q9        40   40   40   40          43   41   42   40
> q10       44   44   44   44          47   45   44   46
> q11       48   48   48   48          51   48   49   50
> q12       52   52   52   52          55   52   53   54
> q13       56   56   56   56          59   58   56   57
> q14       60   60   60   60          63   62   61   60
> 
> This distribution could effectively prevent soft lockups from happening
> on this ARM VM when sufficiently high FIO workload was run, focussed on
> 4 CPU group (cpu 5-9 for example). I verified this by testing on the
> same VM as mentioned above.
> 
> > diff --git a/drivers/pci/controller/pci-hyperv.c 
> > b/drivers/pci/controller/pci-hyperv.c
> > index 85d826f8f68d..1706090cebeb 100644
> > --- a/drivers/pci/controller/pci-hyperv.c
> > +++ b/drivers/pci/controller/pci-hyperv.c
> 
> <snip>
> 
> > +
> > +static unsigned int hv_pci_arm64_get_min_load(struct irq_data *d,
> > +                                  const struct cpumask *cpu_mask)
> > +{
> > +   return hv_pci_arm64_get_min_load_and(d, cpu_mask, cpu_possible_mask);
> > +}
> > +
> > +static int hv_pci_arm64_select_cpu(struct irq_data *d,
> > +                     const struct cpumask *aff_mask)
> > +{
> > +   int cpu;
> > +
> > +   if (irqd_affinity_is_managed(d))
> > +           cpu = hv_pci_arm64_get_min_load(d, aff_mask);
> > +
> 
> You can remove the empty lines here, if these are not intentional.

They were somewhat intentional to cleanly separate the 3 cases so
they are easier to read (at least for me). It's a bit non-standard, but
I have a slight preference to keep them.

> 
> > +   else if (cpumask_intersects(aff_mask, cpu_online_mask))
> > +           /* Try the intersection of the affinity and online masks */
> > +           cpu = hv_pci_arm64_get_min_load_and(d, aff_mask, 
> > cpu_online_mask);
> > +
> 
> Ditto
> 
> > +   else
> > +           /* If that doesn't fly, the online mask is the last resort */
> > +           cpu = hv_pci_arm64_get_min_load(d, cpu_online_mask);
> > +
> > +   pr_debug("IRQ%d -> %*pbl CPU%d\n", d->irq, cpumask_pr_args(aff_mask), 
> > cpu);
> 
> Add an empty line here instead.

Will do.

> 
> > +   return cpu;
> > +}
> > +
> > +/*
> > + * Track the count of SPIs assigned to each CPU. Pick a single CPU from the
> > + * affinity mask based on the SPI loading data, and have the parent IRQ
> > + * chip (should be "GICv3") set the effective affinity to that single CPU.
> > + */
> 
> Could concurrent affinity updates for different IRQs select the same
> stale minimum here?
> 
> Each update holds only its own IRQ descriptor lock, so two callbacks can
> both decrement their old counts, observe the same CPU as least loaded,
> program both SPIs to it, and only then increment its count. Could the
> selection and count reservation be serialized across these IRQs, or
> could the selected CPU be reserved atomically before programming the parent?

I copied the approach used by its_set_affinity(), which doesn't provide
atomicity across decrementing the old count, choosing a new CPU,
and increment the new count. The x86 APIC equivalent code *does*
provide atomicity. The original emails for the ITS patches never discussed
atomicity, so I guess that aspect wasn't noticed at the time.

The x86 code has a different quirk in that the new CPU is determined
before decrementing the count on the old CPU. The behavior was
discussed on the ITS email thread, and the final version of the ITS patches
intentionally decrement the old count before determining the new CPU.

I'm inclined to make this patch provide the atomicity like x86 APIC.
Even though the spreading is just a heuristic, being atomic will avoid
a set of non-deterministic one-off cases where interrupts get stacked
and cause perf problems. I'll introduce a raw spin lock to provide the
atomicity. And with that spin lock, the counters themselves can be
standard integer types instead of atomics.

> 
> > +static int hv_pci_arm64_set_affinity(struct irq_data *d,
> > +                                const struct cpumask *mask_val,
> > +                                bool force)
> > +{
> > +   unsigned int cpu, prev_cpu;
> > +   int ret;
> > +
> > +   prev_cpu = cpumask_first(irq_data_get_effective_affinity_mask(d));
> > +   if (prev_cpu < nr_cpu_ids)
> > +           hv_pci_dec_spi_count(d, prev_cpu);
> > +
> > +   if (force)
> > +           cpu = hv_pci_arm64_get_min_load(d, mask_val);
> > +   else
> > +           cpu = hv_pci_arm64_select_cpu(d, mask_val);
> > +
> > +   if (cpu >= nr_cpu_ids)
> > +           goto err;
> > +
> > +   ret = irq_chip_set_affinity_parent(d, cpumask_of(cpu), force);
> > +   if (ret < 0)
> > +           goto err;
> > +
> > +   hv_pci_inc_spi_count(d, cpu);
> > +   return ret;
> > +
> > +err:
> > +   if (prev_cpu < nr_cpu_ids)
> > +           hv_pci_inc_spi_count(d, prev_cpu);
> > +   return -EINVAL;
> > +}
> > +
> >   /* Hyper-V PCI MSI IRQ chip */
> >   static struct irq_chip hv_arm64_msi_irq_chip = {
> >     .name = "MSI",
> > -   .irq_set_affinity = irq_chip_set_affinity_parent,
> > +   .irq_set_affinity = hv_pci_arm64_set_affinity,
> >     .irq_eoi = irq_chip_eoi_parent,
> >     .irq_mask = irq_chip_mask_parent,
> >     .irq_unmask = irq_chip_unmask_parent
> > @@ -888,21 +1010,39 @@ static int hv_pci_vec_irq_domain_alloc(struct 
> > irq_domain *domain,
> >    * composing MSI from the hypervisor. GIC will eventually set the right
> >    * affinity for the irq and the 'unmask' will retarget the interrupt to 
> > that
> >    * cpu.
> > + *
> > + * The first cpu is always used for initial activation because spreading 
> > the
> > + * initial CPU would have to be done without information about affinity 
> > groups
> > + * that may exist. The resulting assignments would pollute the "least 
> > loaded"
> > + * data for making the final assignments. Everything would still work, but
> > + * the spreading would be less even than it could be.
> >    */
> >   static int hv_pci_vec_irq_domain_activate(struct irq_domain *domain,
> >                                       struct irq_data *irqd, bool reserve)
> >   {
> > -   int cpu = cpumask_first(cpu_present_mask);
> > +   int cpu = cpumask_first(cpu_online_mask);
> 
> This should be harmless, but I am wondering if this is actually required
> for your changes or this is a separate independent change, addressing
> some other issue.

Using the online mask matches what the ITS code does, and it avoids
referencing per-cpu data for a possibly offline CPU. But upon researching
the question, the latter is OK since per-cpu data is allocated for all
possible CPUs at boot time. So if somehow CPU 0 could be offline, it's
still OK to select CPU 0 as the initial effective affinity and to bump the
count for CPU 0. With that in mind, I'll drop the change as unnecessary.

Thanks for the good comments ....

Michael

> 
> >
> > +   hv_pci_inc_spi_count(irqd, cpu);
> >     irq_data_update_effective_affinity(irqd, cpumask_of(cpu));
> >
> >     return 0;
> >   }
> >
> > +static void hv_pci_vec_irq_domain_deactivate(struct irq_domain *domain,
> > +                                        struct irq_data *irqd)
> > +{
> > +   int cpu;
> > +
> > +   cpu = cpumask_first(irq_data_get_effective_affinity_mask(irqd));
> > +   if (cpu < nr_cpu_ids)
> > +           hv_pci_dec_spi_count(irqd, cpu);
> > +}
> > +
> >   static const struct irq_domain_ops hv_pci_domain_ops = {
> >     .alloc  = hv_pci_vec_irq_domain_alloc,
> >     .free   = hv_pci_vec_irq_domain_free,
> >     .activate = hv_pci_vec_irq_domain_activate,
> > +   .deactivate = hv_pci_vec_irq_domain_deactivate,
> >   };
> >
> >   #ifdef CONFIG_OF
> 
> Regards,
> Naman
> 

Reply via email to