[PATCH 1/1] PCI: hv: Spread load across CPUs when selecting effective affinity CPU

[PATCH 1/1] PCI: hv: Spread load across CPUs when selecting effective affinity CPU

[Michael Kelley]

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 <mhklinux@outlook.com>
---
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(-)

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
@@ -746,10 +746,132 @@ struct hv_pci_chip_data {
 /* Hyper-V vPCI MSI GIC IRQ domain */
 static struct irq_domain *hv_msi_gic_irq_domain;
 
+/*
+ * Data structures and helper functions for spreading the CPU in the
+ * affinity mask that is picked for the effective affinity. Such
+ * spreading is done by the "APIC" IRQ chip on x86 and by the
+ * "ITS" chip for LPIs with GICv3+ITS on arm64. But Hyper-V provides
+ * only a GICv3 with SPIs (no ITS or LPIs) to arm64 guests. For SPIs
+ * the "GICv3" IRQ chip always picks the first CPU in the affinity mask,
+ * which can overload that CPU and reduce parallelism in processing
+ * interrupts.
+ *
+ * While the Hyper-V configuration is architecturally allowed, it's not
+ * typical, so the spreading is done here in pci-hyperv.c with the "MSI"
+ * IRQ chip parented to "GICv3". Track how many SPIs have been assigned
+ * to each CPU, and use that data to spread the IRQ load evenly across
+ * the CPUs instead of just picking the first CPU. The code follows a
+ * pattern similar to the spreading logic for the "ITS" IRQ chip.
+ */
+
+struct cpu_spi_count {
+	/* Index 0 is unmanaged. Index 1 is managed. */
+	atomic_t	count[2];
+};
+
+static DEFINE_PER_CPU(struct cpu_spi_count, cpu_spi_count);
+
+static void hv_pci_inc_spi_count(struct irq_data *d, int cpu)
+{
+	int idx = irqd_affinity_is_managed(d);
+
+	atomic_inc(&per_cpu_ptr(&cpu_spi_count, cpu)->count[idx]);
+}
+
+static void hv_pci_dec_spi_count(struct irq_data *d, int cpu)
+{
+	int idx = irqd_affinity_is_managed(d);
+
+	atomic_dec(&per_cpu_ptr(&cpu_spi_count, cpu)->count[idx]);
+}
+
+static unsigned int hv_pci_arm64_get_min_load_and(struct irq_data *d,
+					   const struct cpumask *cpu_mask1,
+					   const struct cpumask *cpu_mask2)
+{
+	unsigned int cpu = nr_cpu_ids, tmp;
+	int count = S32_MAX, this_count;
+	int idx = irqd_affinity_is_managed(d);
+
+	for_each_cpu_and(tmp, cpu_mask1, cpu_mask2) {
+		this_count = atomic_read(
+				&per_cpu_ptr(&cpu_spi_count, tmp)->count[idx]);
+		if (this_count < count) {
+			cpu = tmp;
+			count = this_count;
+		}
+	}
+
+	return cpu;
+}
+
+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);
+
+	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);
+
+	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);
+	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.
+ */
+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);
 
+	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
-- 
2.25.1

Re: [PATCH 1/1] PCI: hv: Spread load across CPUs when selecting effective affinity CPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] PCI: hv: early activation in hv_pci_vec_irq_domain_activate() skews CPU0 load
- [Medium] PCI: hv: cpu_spi_count corruption in hv_pci_arm64_set_affinity() when inactive
--

commit 9d112d599453572154cd3fbc90e3b6ea7dedf0e3
Author: Michael Kelley <mhkelley58@gmail.com>

PCI: hv: Spread load across CPUs when selecting effective affinity CPU

This patch extends the arm64 Hyper-V vPCI MSI irq chip to track per-CPU
SPI assignments and select the least-loaded CPU from the affinity mask
before forwarding the single-CPU effective affinity to the parent GICv3
chip. It also updates the domain activate and deactivate callbacks to
increment and decrement the per-CPU SPI counters.

> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 824cf27e85e67..7c293e39c9208 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c

[ ... ]

> +/*
> + * 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.
> + */
> +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;
> +}

[Severity: Medium]
Can hv_pci_arm64_set_affinity() corrupt cpu_spi_count when affinity is
changed on a deactivated interrupt?

When an allocated MSI or MSI-X interrupt is deactivated (for example, when
free_irq() -> __free_irq() calls irq_domain_deactivate_irq() while MSI
vectors remain allocated across ndo_stop/ndo_open or queue reconfiguration,
or when migrate_one_irq() calls irq_shutdown_and_deactivate()),
hv_pci_vec_irq_domain_deactivate() decrements cpu_spi_count for the CPU in
effective_affinity and IRQD_ACTIVATED is cleared, but the old CPU remains
set in the effective_affinity mask:

drivers/pci/controller/pci-hyperv.c:hv_pci_vec_irq_domain_deactivate() {
	cpu = cpumask_first(irq_data_get_effective_affinity_mask(irqd));
	if (cpu < nr_cpu_ids)
		hv_pci_dec_spi_count(irqd, cpu);
}

Because hv_pci_vec_irq_domain_alloc() does not set IRQD_AFFINITY_ON_ACTIVATE
(unlike its_irq_domain_alloc() in drivers/irqchip/irq-gic-v3-its.c),
irq_set_affinity_deactivated() does not defer affinity changes on a
deactivated interrupt:

kernel/irq/manage.c:irq_set_affinity_deactivated() {
	if (!IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) ||
	    irqd_is_activated(data) || !irqd_affinity_on_activate(data))
		return false;
	...
}

If irq_set_affinity() or irq_force_affinity() is called while the interrupt
is deactivated (for example, via /proc/irq/<irq>/smp_affinity,
irq_set_affinity_hint(), or irq_restore_affinity_of_irq()),
irq_set_affinity_locked() -> irq_do_set_affinity() invokes
hv_pci_arm64_set_affinity(). That reads the stale prev_cpu from
irq_data_get_effective_affinity_mask(d), decrements prev_cpu's
cpu_spi_count a second time, and increments the newly selected cpu's
cpu_spi_count while IRQD_ACTIVATED remains false.

If the vector is later freed via pci_free_irq_vectors() ->
__msi_domain_free_irqs() in kernel/irq/msi.c, irq_domain_deactivate_irq()
is skipped because !irqd_is_activated(irqd), leaking the count on the new
CPU; or if the vector is re-activated via request_irq() -> irq_activate()
-> hv_pci_vec_irq_domain_activate(), effective_affinity is overwritten
with cpumask_first(cpu_online_mask) without decrementing the CPU selected
while deactivated.

Should hv_pci_vec_irq_domain_alloc() set IRQD_AFFINITY_ON_ACTIVATE, or
should hv_pci_vec_irq_domain_deactivate() clear effective_affinity?

[ ... ]

> @@ -885,21 +1007,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);
>  
> +	hv_pci_inc_spi_count(irqd, cpu);
>  	irq_data_update_effective_affinity(irqd, cpumask_of(cpu));
>  
>  	return 0;
>  }

[Severity: Medium]
Won't incrementing cpu_spi_count in hv_pci_vec_irq_domain_activate() skew
the load calculation away from the first online CPU during multi-vector
MSI/MSI-X setup?

Because pci_msi_template and pci_msix_template in
drivers/pci/msi/irqdomain.c set MSI_FLAG_ACTIVATE_EARLY, allocating N
vectors via pci_alloc_irq_vectors() or pci_alloc_irq_vectors_affinity()
causes __msi_domain_alloc_irqs() to activate all N vectors in a batch via
msi_init_virq() -> irq_domain_activate_irq() ->
hv_pci_vec_irq_domain_activate() before returning and before request_irq()
is called on any vector:

kernel/irq/msi.c:__msi_domain_alloc_irqs() {
	...
	for (i = 0; i < desc->nvec_used; i++) {
		irq_set_msi_desc_off(virq, i, desc);
		irq_debugfs_copy_devname(virq + i, dev);
		ret = msi_init_virq(domain, virq + i, vflags);
		if (ret)
			return ret;
	}
}

This increments cpu_spi_count on cpumask_first(cpu_online_mask) (typically
CPU 0) for all N vectors up front, inflating CPU 0's count by N.

When the driver then calls request_irq() for each vector i = 0..N-1 in
sequence (such as in nvme_setup_io_queues()), irq_startup() ->
irq_do_set_affinity() -> hv_pci_arm64_set_affinity() decrements CPU 0's
cpu_spi_count by only 1 for vector i, leaving CPU 0's count inflated by
N - 1 - i from the remaining unstarted vectors of the same device still
parked on CPU 0. When hv_pci_arm64_select_cpu() selects a CPU for the first
queue (whose affinity mask includes CPU 0), it sees CPU 0's load inflated
by N - 1 and avoids assigning interrupts to CPU 0 for the first N - 1
devices probed.

Furthermore, if a driver allocates M vectors and only calls request_irq()
on K < M of them (or defers request_irq() until queues or network ports are
opened), the M - K unrequested vectors remain activated on CPU 0 and keep
its cpu_spi_count inflated by M - K.

Could hv_pci_vec_irq_domain_activate() leave effective_affinity empty and
avoid incrementing cpu_spi_count until hv_pci_arm64_set_affinity() selects
the actual target CPU?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020182433.336637-1-mhklinux@outlook.com?part=1