[PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Naman Jain]

group_cpus_evenly() computes how a device's queue interrupts are spread
across CPUs. It backs managed-interrupt affinity (kernel/irq/affinity.c)
and block-multiqueue mappings (block/blk-mq-cpumap.c), and is invoked
independently by every device that uses them - NVMe, NICs, storage HBAs,
and virtio devices. Its output is deterministic, i.e. for a given
topology, two similar devices produce an identical group-to-CPU mapping.

When ngroups < ncpus, some groups end up with only a single CPU. An
interrupt whose mask has one CPU can only run there, making that CPU a
"hot" handler. Because the mapping is deterministic, identical devices
compute the same layout and stack all their single-CPU IRQs onto the
very same CPUs, leaving the rest of the system idle.

This is easy to hit in practice. On an Azure L96as_v4 VM (96 vCPUs, 2
NUMA nodes of 48 CPUs, 6 NVMe disks with 62 I/O queues each),
group_cpus_evenly() splits each disk's 62 queues into 31 per node over
48 CPUs. 48 does not divide evenly by 31:

    per NUMA node: 48 CPUs / 31 queues
      17 groups get 2 CPUs   (cover 34 CPUs)
      14 groups get 1 CPU    (cover 14 CPUs)  <- single-CPU "hot" queues

That is 14 hot queues per node, 28 per disk. All 6 disks land them on
the same 28 CPUs, so 168 hot interrupts pile onto 28 of 96 CPUs while
two-thirds of the system handles none:

    Before (per-CPU, disks whose IRQs it services):
      CPU  0: 3 disks    ...   CPU 34: 6 disks (all six)
      CPU  1: 3 disks    ...   CPU 47: 6 disks (all six)
    Summary: 28 CPUs (34-47, 82-95) served all 6 disks and the other 68
    served only 3. Those 28 CPUs cap throughput and inflate tail
    latency while most of the system is idle.

Fix this by introducing a per-caller rotation via a static atomic
counter (group_spread_cnt). Each call to group_cpus_evenly() takes a
unique spread_offset, applied to the two decisions that were previously
deterministic:

1) Cluster-level rotation in __try_group_cluster_cpus(): after
   alloc_groups_to_nodes() distributes groups proportionally across
   clusters, integer rounding leaves some clusters with one extra
   group. The extras are redistributed starting from a rotated
   position, with a stride of ncluster/total_extra to minimize overlap
   between consecutive callers. A multi-pass fallback ensures all
   extras are placed even when some clusters are at capacity.

2) Intra-cluster rotation in assign_cpus_to_groups(): the sequential
   extra assignment is replaced with a modular expression,
     (v + spread_offset) % nv->ngroups < extra_grps
   rotating which groups within a cluster receive the extra CPU.

Nothing else about the layout changes - same queue count, same NUMA
weighting, same full CPU coverage and locality. Each caller simply
starts its mapping from a different point, and each individual call
still produces a valid, fair distribution. Across callers, different
CPUs absorb the single-CPU group IRQ load:

    After (same setup, with the rotation):
      CPU  0: 4 disks    CPU  2: 4 disks    CPU 47: 4 disks
      CPU  1: 4 disks    CPU  3: 4 disks    ...
    Summary: no CPU serves more than 4 disks, and all 96 CPUs are used.

The total interrupt work is unchanged - every CPU still handles one
queue per disk; only the placement of the single-CPU hot queues moves.
This benefits every managed-IRQ, blk-mq, and virtio-vdpa / virtio-fs
device with no driver changes.

Because the offset comes from a global counter advanced once per call,
the mapping now depends on call (device probe) order. A given device's
exact layout can differ from one boot to the next, and a later recompute
(e.g. a blk-mq remap) may pick a different layout. Every such layout is
still valid, fair, and proportional - only the choice among equally good
mappings varies.

On a 96-vCPU Hyper-V VM running 4K random-read fio across 6 NVMe disks,
worst-disk degradation versus average dropped from 11% to 5%, and the
previously penalized disks gained 12% IOPS at 10% lower latency.

Fixes: 89802ca36c96 ("lib/group_cpus: make group CPU cluster aware")
Co-developed-by: Long Li <longli@microsoft.com>
Signed-off-by: Long Li <longli@microsoft.com>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
Changes since v1
(https://lore.kernel.org/all/20260324075352.2326972-1-namjain@linux.microsoft.com/):
- Cluster base is now a per-cluster proportional floor
  (ngroups * cap / ncpus) instead of the global per-cluster minimum,
  so proportional weighting is preserved on asymmetric (e.g.
  big.LITTLE) cluster topologies. (Sashiko review)
- Document that the rotation offset is call/probe-order dependent: a
  device's exact layout may vary across boots and recomputes (each
  layout is still valid, fair, and proportional).
- Rewrite the commit message with a worked example and fio numbers.

 lib/group_cpus.c | 149 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 137 insertions(+), 12 deletions(-)

diff --git a/lib/group_cpus.c b/lib/group_cpus.c
index e6e18d7a49bba..8bed0f9d2110b 100644
--- a/lib/group_cpus.c
+++ b/lib/group_cpus.c
@@ -7,6 +7,7 @@
 #include <linux/slab.h>
 #include <linux/cpu.h>
 #include <linux/sort.h>
+#include <linux/atomic.h>
 #include <linux/group_cpus.h>
 
 #ifdef CONFIG_SMP
@@ -255,12 +256,20 @@ static void alloc_nodes_groups(unsigned int numgrps,
 	alloc_groups_to_nodes(numgrps, numcpus, node_groups, nr_node_ids);
 }
 
+/*
+ * Per-caller rotation counter for group_cpus_evenly().
+ * Wrapping is harmless: the offset is only used modulo small values
+ * (ncluster or nv->ngroups), so any unsigned value works.
+ */
+static atomic_t group_spread_cnt = ATOMIC_INIT(0);
+
 static void assign_cpus_to_groups(unsigned int ncpus,
 				  struct cpumask *nmsk,
 				  struct node_groups *nv,
 				  struct cpumask *masks,
 				  unsigned int *curgrp,
-				  unsigned int last_grp)
+				  unsigned int last_grp,
+				  unsigned int spread_offset)
 {
 	unsigned int v, cpus_per_grp, extra_grps;
 	/* Account for rounding errors */
@@ -270,11 +279,15 @@ static void assign_cpus_to_groups(unsigned int ncpus,
 	for (v = 0; v < nv->ngroups; v++, *curgrp += 1) {
 		cpus_per_grp = ncpus / nv->ngroups;
 
-		/* Account for extra groups to compensate rounding errors */
-		if (extra_grps) {
+		/*
+		 * Rotate which groups get the extra CPU so that
+		 * successive callers produce different mappings,
+		 * avoiding IRQ stacking when multiple devices
+		 * share the same CPU topology.
+		 */
+		if (extra_grps &&
+		    (v + spread_offset) % nv->ngroups < extra_grps)
 			cpus_per_grp++;
-			--extra_grps;
-		}
 
 		/*
 		 * wrapping has to be considered given 'startgrp'
@@ -361,7 +374,8 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
 				     struct cpumask *node_cpumask,
 				     struct cpumask *masks,
 				     unsigned int *curgrp,
-				     unsigned int last_grp)
+				     unsigned int last_grp,
+				     unsigned int spread_offset)
 {
 	struct node_groups *cluster_groups;
 	const struct cpumask **clusters;
@@ -379,6 +393,111 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
 	if (ncluster == 0)
 		goto fail_no_clusters;
 
+	/*
+	 * Rotate which clusters receive extra groups so that different
+	 * callers of group_cpus_evenly() produce different group-to-CPU
+	 * mappings. Without this, all devices get identical affinity
+	 * masks, causing IRQ stacking on CPUs assigned single-CPU groups.
+	 *
+	 * alloc_groups_to_nodes() distributes ngroups proportionally, but
+	 * integer rounding causes some clusters to get one more group
+	 * than others. The assignment is deterministic, so every device
+	 * gets the same mapping. Fix: compute a proportional floor for
+	 * each cluster (ngroups * cap / ncpus), collect only the
+	 * rounding-induced extras, then redistribute them starting from
+	 * a rotated position. This preserves the proportional weighting
+	 * across differently-sized clusters while rotating the rounding
+	 * extras, keeping the rotation effective on both symmetric and
+	 * asymmetric cluster topologies.
+	 *
+	 * Note: after alloc_groups_to_nodes(), cluster_groups[].ngroups
+	 * holds the group count (the union no longer holds per-cluster CPU
+	 * counts), so each cluster's CPU capacity (cap) is taken from its
+	 * mask. The ncpus divisor is the function parameter, which equals
+	 * the sum of the per-cluster caps.
+	 */
+	if (ncluster > 1) {
+		unsigned int total_extra = 0;
+		unsigned int start, stride;
+
+		/*
+		 * Compute a per-cluster proportional floor and collect
+		 * only the rounding-induced extras for redistribution.
+		 *
+		 * Each cluster's floor is ngroups * cap / ncpus, which
+		 * preserves its proportional share.  Only the rounding
+		 * remainders (typically one per cluster) are collected
+		 * for rotated redistribution, keeping the rotation
+		 * effective even on asymmetric topologies (e.g.
+		 * big.LITTLE) where differently-sized clusters would
+		 * otherwise absorb all extras deterministically.
+		 */
+		for (i = 0; i < ncluster; i++) {
+			unsigned int cap, prop_floor, base;
+
+			cap = cpumask_weight_and(clusters[cluster_groups[i].id],
+						 node_cpumask);
+			prop_floor = ngroups * cap / ncpus;
+
+			/*
+			 * Use proportional floor as base.  Ensure at
+			 * least 1 group per cluster, and never exceed
+			 * alloc_groups_to_nodes()'s original allocation
+			 * (which may be less than prop_floor when small
+			 * clusters consumed groups via max(1,...)).
+			 */
+			base = prop_floor > 0 ? prop_floor : 1;
+			if (base > cluster_groups[i].ngroups)
+				base = cluster_groups[i].ngroups;
+
+			total_extra += cluster_groups[i].ngroups - base;
+			cluster_groups[i].ngroups = base;
+		}
+
+		/*
+		 * Redistribute rounding extras using a stride to scatter
+		 * them across clusters.  With stride = ncluster / extras,
+		 * consecutive callers' extra sets overlap minimally
+		 * (e.g. max 2 overlap for 6 callers with 24 clusters
+		 * and 7 extras, vs 6 overlap with stride 1).
+		 */
+		start = spread_offset % ncluster;
+		stride = (total_extra > 0 && total_extra < ncluster) ?
+			 ncluster / total_extra : 1;
+
+		for (i = 0; i < ncluster && total_extra > 0; i++) {
+			unsigned int idx =
+				(start + i * stride) % ncluster;
+			unsigned int cap;
+
+			cap = cpumask_weight_and(clusters[cluster_groups[idx].id],
+						 node_cpumask);
+			if (cluster_groups[idx].ngroups < cap) {
+				cluster_groups[idx].ngroups++;
+				total_extra--;
+			}
+		}
+
+		/* Fallback: place remaining extras wherever they fit */
+		while (total_extra > 0) {
+			unsigned int placed = 0;
+
+			for (i = 0; i < ncluster && total_extra > 0; i++) {
+				unsigned int cap;
+
+				cap = cpumask_weight_and(clusters[cluster_groups[i].id],
+							 node_cpumask);
+				if (cluster_groups[i].ngroups < cap) {
+					cluster_groups[i].ngroups++;
+					total_extra--;
+					placed++;
+				}
+			}
+			if (!placed)
+				break;
+		}
+	}
+
 	for (i = 0; i < ncluster; i++) {
 		struct node_groups *nv = &cluster_groups[i];
 
@@ -389,7 +508,8 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
 			continue;
 		WARN_ON_ONCE(nv->ngroups > nc);
 
-		assign_cpus_to_groups(nc, nmsk, nv, masks, curgrp, last_grp);
+		assign_cpus_to_groups(nc, nmsk, nv, masks, curgrp, last_grp,
+				      spread_offset);
 	}
 
 	ret = true;
@@ -404,7 +524,8 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
 static int __group_cpus_evenly(unsigned int startgrp, unsigned int numgrps,
 			       cpumask_var_t *node_to_cpumask,
 			       const struct cpumask *cpu_mask,
-			       struct cpumask *nmsk, struct cpumask *masks)
+			       struct cpumask *nmsk, struct cpumask *masks,
+			       unsigned int spread_offset)
 {
 	unsigned int i, n, nodes, done = 0;
 	unsigned int last_grp = numgrps;
@@ -455,13 +576,14 @@ static int __group_cpus_evenly(unsigned int startgrp, unsigned int numgrps,
 		WARN_ON_ONCE(nv->ngroups > ncpus);
 
 		if (__try_group_cluster_cpus(ncpus, nv->ngroups, nmsk,
-					     masks, &curgrp, last_grp)) {
+					     masks, &curgrp, last_grp,
+					     spread_offset)) {
 			done += nv->ngroups;
 			continue;
 		}
 
 		assign_cpus_to_groups(ncpus, nmsk, nv, masks, &curgrp,
-				      last_grp);
+				      last_grp, spread_offset);
 		done += nv->ngroups;
 	}
 	kfree(node_groups);
@@ -488,6 +610,7 @@ static int __group_cpus_evenly(unsigned int startgrp, unsigned int numgrps,
 struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
 {
 	unsigned int curgrp = 0, nr_present = 0, nr_others = 0;
+	unsigned int spread_offset;
 	cpumask_var_t *node_to_cpumask;
 	cpumask_var_t nmsk, npresmsk;
 	int ret = -ENOMEM;
@@ -510,6 +633,8 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
 	if (!masks)
 		goto fail_node_to_cpumask;
 
+	spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
+
 	build_node_to_cpumask(node_to_cpumask);
 
 	/*
@@ -528,7 +653,7 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
 
 	/* grouping present CPUs first */
 	ret = __group_cpus_evenly(curgrp, numgrps, node_to_cpumask,
-				  npresmsk, nmsk, masks);
+				  npresmsk, nmsk, masks, spread_offset);
 	if (ret < 0)
 		goto fail_node_to_cpumask;
 	nr_present = ret;
@@ -545,7 +670,7 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
 		curgrp = nr_present;
 	cpumask_andnot(npresmsk, cpu_possible_mask, npresmsk);
 	ret = __group_cpus_evenly(curgrp, numgrps, node_to_cpumask,
-				  npresmsk, nmsk, masks);
+				  npresmsk, nmsk, masks, spread_offset);
 	if (ret >= 0)
 		nr_others = ret;
 
-- 
2.43.0

Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Integer overflow in intra-cluster group rotation breaks CPU allocation permutation, causing empty CPU masks.
--

--- Patch [1]: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking ---
commit 3ea99a586f68e414d616ef9d2a0f13f390a05e7d
Author: Naman Jain <namjain@linux.microsoft.com>

lib/group_cpus: rotate extra groups to avoid IRQ stacking

group_cpus_evenly() computes how a device's queue interrupts are spread
across CPUs. This commit introduces a per-caller rotation counter to
shift which groups get extra CPUs. The rotation ensures that single-CPU
groups from multiple identical devices do not stack on the same CPUs,
improving throughput and reducing tail latency.

> diff --git a/lib/group_cpus.c b/lib/group_cpus.c
> index e6e18d7a49bba..8bed0f9d2110b 100644
> --- a/lib/group_cpus.c
> +++ b/lib/group_cpus.c
[ ... ]
> @@ -270,11 +279,15 @@ static void assign_cpus_to_groups(unsigned int ncpus,
>  	for (v = 0; v < nv->ngroups; v++, *curgrp += 1) {
>  		cpus_per_grp = ncpus / nv->ngroups;
>  
> -		/* Account for extra groups to compensate rounding errors */
> -		if (extra_grps) {
> +		/*
> +		 * Rotate which groups get the extra CPU so that
> +		 * successive callers produce different mappings,
> +		 * avoiding IRQ stacking when multiple devices
> +		 * share the same CPU topology.
> +		 */
> +		if (extra_grps &&
> +		    (v + spread_offset) % nv->ngroups < extra_grps)
>  			cpus_per_grp++;

[Severity: Medium]
Can this addition overflow and break the CPU allocation?

The commit message and comments state that wrapping of the unsigned
counter is harmless. However, if spread_offset is very close to UINT_MAX,
v + spread_offset can wrap around mid-loop.

Since 2^32 is generally not a multiple of nv->ngroups, the overflow causes
the modulo result to skip some values and duplicate others.

This means the condition (v + spread_offset) % nv->ngroups < extra_grps
might evaluate to true more times than there are extra groups.

If cpus_per_grp is incremented too many times, the loop will consume more
CPUs than available, prematurely exhausting nmsk and leaving subsequent
masks empty.

Would it be safer to apply the modulo before the addition, such as:
(v + (spread_offset % nv->ngroups)) % nv->ngroups ?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810062144.2108758-1-namjain@linux.microsoft.com?part=1

Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Naman Jain]

On 8/10/2026 11:51 AM, Naman Jain wrote:
> group_cpus_evenly() computes how a device's queue interrupts are spread
> across CPUs. It backs managed-interrupt affinity (kernel/irq/affinity.c)
> and block-multiqueue mappings (block/blk-mq-cpumap.c), and is invoked
> independently by every device that uses them - NVMe, NICs, storage HBAs,
> and virtio devices. Its output is deterministic, i.e. for a given
> topology, two similar devices produce an identical group-to-CPU mapping.
> 
> When ngroups < ncpus, some groups end up with only a single CPU. An
> interrupt whose mask has one CPU can only run there, making that CPU a
> "hot" handler. Because the mapping is deterministic, identical devices
> compute the same layout and stack all their single-CPU IRQs onto the
> very same CPUs, leaving the rest of the system idle.
> 
> This is easy to hit in practice. On an Azure L96as_v4 VM (96 vCPUs, 2
> NUMA nodes of 48 CPUs, 6 NVMe disks with 62 I/O queues each),
> group_cpus_evenly() splits each disk's 62 queues into 31 per node over
> 48 CPUs. 48 does not divide evenly by 31:
> 
>      per NUMA node: 48 CPUs / 31 queues
>        17 groups get 2 CPUs   (cover 34 CPUs)
>        14 groups get 1 CPU    (cover 14 CPUs)  <- single-CPU "hot" queues
> 
> That is 14 hot queues per node, 28 per disk. All 6 disks land them on
> the same 28 CPUs, so 168 hot interrupts pile onto 28 of 96 CPUs while
> two-thirds of the system handles none:
> 
>      Before (per-CPU, disks whose IRQs it services):
>        CPU  0: 3 disks    ...   CPU 34: 6 disks (all six)
>        CPU  1: 3 disks    ...   CPU 47: 6 disks (all six)
>      Summary: 28 CPUs (34-47, 82-95) served all 6 disks and the other 68
>      served only 3. Those 28 CPUs cap throughput and inflate tail
>      latency while most of the system is idle.
> 
> Fix this by introducing a per-caller rotation via a static atomic
> counter (group_spread_cnt). Each call to group_cpus_evenly() takes a
> unique spread_offset, applied to the two decisions that were previously
> deterministic:
> 
> 1) Cluster-level rotation in __try_group_cluster_cpus(): after
>     alloc_groups_to_nodes() distributes groups proportionally across
>     clusters, integer rounding leaves some clusters with one extra
>     group. The extras are redistributed starting from a rotated
>     position, with a stride of ncluster/total_extra to minimize overlap
>     between consecutive callers. A multi-pass fallback ensures all
>     extras are placed even when some clusters are at capacity.
> 
> 2) Intra-cluster rotation in assign_cpus_to_groups(): the sequential
>     extra assignment is replaced with a modular expression,
>       (v + spread_offset) % nv->ngroups < extra_grps
>     rotating which groups within a cluster receive the extra CPU.
> 
> Nothing else about the layout changes - same queue count, same NUMA
> weighting, same full CPU coverage and locality. Each caller simply
> starts its mapping from a different point, and each individual call
> still produces a valid, fair distribution. Across callers, different
> CPUs absorb the single-CPU group IRQ load:
> 
>      After (same setup, with the rotation):
>        CPU  0: 4 disks    CPU  2: 4 disks    CPU 47: 4 disks
>        CPU  1: 4 disks    CPU  3: 4 disks    ...
>      Summary: no CPU serves more than 4 disks, and all 96 CPUs are used.
> 
> The total interrupt work is unchanged - every CPU still handles one
> queue per disk; only the placement of the single-CPU hot queues moves.
> This benefits every managed-IRQ, blk-mq, and virtio-vdpa / virtio-fs
> device with no driver changes.
> 
> Because the offset comes from a global counter advanced once per call,
> the mapping now depends on call (device probe) order. A given device's
> exact layout can differ from one boot to the next, and a later recompute
> (e.g. a blk-mq remap) may pick a different layout. Every such layout is
> still valid, fair, and proportional - only the choice among equally good
> mappings varies.
> 
> On a 96-vCPU Hyper-V VM running 4K random-read fio across 6 NVMe disks,
> worst-disk degradation versus average dropped from 11% to 5%, and the
> previously penalized disks gained 12% IOPS at 10% lower latency.
> 
> Fixes: 89802ca36c96 ("lib/group_cpus: make group CPU cluster aware")
> Co-developed-by: Long Li <longli@microsoft.com>
> Signed-off-by: Long Li <longli@microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>

Sashiko pointed to a minor issue in this patch, which can be addressed 
in the next version. It was not seen when I ran Sashiko locally.
I would also want to add CC: stable tag and stable list in the next version.

But I will wait for any reviews on this patch before sending the next 
version.

Regards,
Naman

Link: 
https://sashiko.dev/#/patchset/20260810062144.2108758-1-namjain%40linux.microsoft.com

RE: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Michael Kelley]

From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
> 
> group_cpus_evenly() computes how a device's queue interrupts are spread
> across CPUs. It backs managed-interrupt affinity (kernel/irq/affinity.c)
> and block-multiqueue mappings (block/blk-mq-cpumap.c), and is invoked
> independently by every device that uses them - NVMe, NICs, storage HBAs,
> and virtio devices. Its output is deterministic, i.e. for a given
> topology, two similar devices produce an identical group-to-CPU mapping.
> 
> When ngroups < ncpus, some groups end up with only a single CPU. An
> interrupt whose mask has one CPU can only run there, making that CPU a
> "hot" handler. Because the mapping is deterministic, identical devices
> compute the same layout and stack all their single-CPU IRQs onto the
> very same CPUs, leaving the rest of the system idle.
> 
> This is easy to hit in practice. On an Azure L96as_v4 VM (96 vCPUs, 2
> NUMA nodes of 48 CPUs, 6 NVMe disks with 62 I/O queues each),
> group_cpus_evenly() splits each disk's 62 queues into 31 per node over
> 48 CPUs. 48 does not divide evenly by 31:
> 
>     per NUMA node: 48 CPUs / 31 queues
>       17 groups get 2 CPUs   (cover 34 CPUs)
>       14 groups get 1 CPU    (cover 14 CPUs)  <- single-CPU "hot" queues
> 
> That is 14 hot queues per node, 28 per disk. All 6 disks land them on
> the same 28 CPUs, so 168 hot interrupts pile onto 28 of 96 CPUs while
> two-thirds of the system handles none:
> 
>     Before (per-CPU, disks whose IRQs it services):
>       CPU  0: 3 disks    ...   CPU 34: 6 disks (all six)
>       CPU  1: 3 disks    ...   CPU 47: 6 disks (all six)
>     Summary: 28 CPUs (34-47, 82-95) served all 6 disks and the other 68
>     served only 3. Those 28 CPUs cap throughput and inflate tail
>     latency while most of the system is idle.
> 
> Fix this by introducing a per-caller rotation via a static atomic
> counter (group_spread_cnt). Each call to group_cpus_evenly() takes a
> unique spread_offset, applied to the two decisions that were previously
> deterministic:
> 
> 1) Cluster-level rotation in __try_group_cluster_cpus(): after
>    alloc_groups_to_nodes() distributes groups proportionally across
>    clusters, integer rounding leaves some clusters with one extra
>    group. The extras are redistributed starting from a rotated
>    position, with a stride of ncluster/total_extra to minimize overlap
>    between consecutive callers. A multi-pass fallback ensures all
>    extras are placed even when some clusters are at capacity.
> 
> 2) Intra-cluster rotation in assign_cpus_to_groups(): the sequential
>    extra assignment is replaced with a modular expression,
>      (v + spread_offset) % nv->ngroups < extra_grps
>    rotating which groups within a cluster receive the extra CPU.
> 

I've finally been able to review this. It took me a while to get up
to speed on the overall approach of the existing code, and then your
changes. I'm sure there are subtleties that I don't yet grok, so my
comments might be off base.

My first question is about the placement of your (1) change above.
It comes after alloc_groups_to_nodes() is called by
alloc_cluster_groups(), and it modifies what alloc_groups_to_nodes()
set up. I had expected that your (1) change would be included in
alloc_groups_to_nodes() so that it would also be applied at the
NUMA node level. There are cases where the NUMA node count
might be relatively large, but the cluster count is 0 or 1. In that
case, your (1) change is never invoked. Maybe there's a reason
for not applying your updates at the NUMA node level, but that
reason isn't evident to me.

> Nothing else about the layout changes - same queue count, same NUMA
> weighting, same full CPU coverage and locality. Each caller simply
> starts its mapping from a different point, and each individual call
> still produces a valid, fair distribution. Across callers, different
> CPUs absorb the single-CPU group IRQ load:
> 
>     After (same setup, with the rotation):
>       CPU  0: 4 disks    CPU  2: 4 disks    CPU 47: 4 disks
>       CPU  1: 4 disks    CPU  3: 4 disks    ...
>     Summary: no CPU serves more than 4 disks, and all 96 CPUs are used.
> 
> The total interrupt work is unchanged - every CPU still handles one
> queue per disk; only the placement of the single-CPU hot queues moves.
> This benefits every managed-IRQ, blk-mq, and virtio-vdpa / virtio-fs
> device with no driver changes.
> 
> Because the offset comes from a global counter advanced once per call,
> the mapping now depends on call (device probe) order. A given device's
> exact layout can differ from one boot to the next, and a later recompute
> (e.g. a blk-mq remap) may pick a different layout. Every such layout is
> still valid, fair, and proportional - only the choice among equally good
> mappings varies.
> 
> On a 96-vCPU Hyper-V VM running 4K random-read fio across 6 NVMe disks,
> worst-disk degradation versus average dropped from 11% to 5%, and the
> previously penalized disks gained 12% IOPS at 10% lower latency.

My second question is about the range of NUMA and cluster
configurations to which you expect your changes to be applicable. You've
cited an example above where the changes are very effective. But I
did some experiments on other configurations, and found them to be
less effective than I had expected. Maybe my expectations are wrong,
or the changes have a bug or incompleteness. In at least one case,
algorithm change (1) not being applied at the NUMA node level may
be the cause of reduced effectiveness, though I didn't fully investigate
the details.

Here are the three configuration I tried:

1)  Azure L48s v2 VM. This VM has 6 NUMA nodes, each with 8
vCPUs. It has 24 clusters, each with two vCPUs that are a hyper-
threaded pair. It has 6 NVMe controllers, each with 32 queues, so
there are 192 IRQs to be assigned. With existing code, all 48 vCPUs
are assigned IRQs:  32 vCPUs get 3 IRQs and 16 vCPUs get 6 IRQs,
which is somewhat unbalanced but not terrible. With your patch,
all vCPUs get between 3 and 5 IRQs, which is an improvement, but
not as good as the theoretical best of 4 IRQs/vCPU. In both cases,
the NUMA nodes are slightly unbalanced -- 2 NUMA nodes
get 36 IRQs each, and 4 NUMA nodes get 30 IRQs each.

2) Azure D16plds v6 VM. This is an arm64 VM with a single
NUMA node. It has 1 cluster with all 16 vCPUs because arm64
uses CONFIG_GENERIC_ARCH_TOPOLOGY, which makes
clusters degenerate. It has 2 NVMe controllers, each with 6 queues.
As expected, existing code assigns 2 IRQs each to 6 vCPUs.
With your patch, 4 vCPUs still have 2 IRQs, while 4 vCPUs have
1 IRQ. I had expected that 12 vCPUs would each be assigned
1 IRQ, but didn't investigate why that didn't happen.

3) Azure D96plds v6 VM. Also an arm64 VM, but with 2 NUMA
nodes. Again, it has 1 cluster with all 96 vCPUs. It has 6 NVMe
controllers, each with 14 queues. As expected, existing code
assigns 6 IRQs to each of 14 vCPUs. With your patch, the IRQs
are spread across 26 vCPUs (13 in each NUMA node) with
counts ranging from 1 to 6. I can't discern a pattern in the
IRQ counts, except that the pattern for each NUMA node
is the same.

At this point, I'm just calling out my top-level observations.
I may look more closely at the details of "why" some of
these cases don't get much improvement.

Michael


> 
> Fixes: 89802ca36c96 ("lib/group_cpus: make group CPU cluster aware")
> Co-developed-by: Long Li <longli@microsoft.com>
> Signed-off-by: Long Li <longli@microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> Changes since v1
> (https://lore.kernel.org/all/20260324075352.2326972-1-namjain@linux.microsoft.com/):
> - Cluster base is now a per-cluster proportional floor
>   (ngroups * cap / ncpus) instead of the global per-cluster minimum,
>   so proportional weighting is preserved on asymmetric (e.g.
>   big.LITTLE) cluster topologies. (Sashiko review)
> - Document that the rotation offset is call/probe-order dependent: a
>   device's exact layout may vary across boots and recomputes (each
>   layout is still valid, fair, and proportional).
> - Rewrite the commit message with a worked example and fio numbers.
> 

Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Naman Jain]

On 8/23/2026 9:17 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
>>
>> group_cpus_evenly() computes how a device's queue interrupts are spread
>> across CPUs. It backs managed-interrupt affinity (kernel/irq/affinity.c)
>> and block-multiqueue mappings (block/blk-mq-cpumap.c), and is invoked
>> independently by every device that uses them - NVMe, NICs, storage HBAs,
>> and virtio devices. Its output is deterministic, i.e. for a given
>> topology, two similar devices produce an identical group-to-CPU mapping.
>>
>> When ngroups < ncpus, some groups end up with only a single CPU. An
>> interrupt whose mask has one CPU can only run there, making that CPU a
>> "hot" handler. Because the mapping is deterministic, identical devices
>> compute the same layout and stack all their single-CPU IRQs onto the
>> very same CPUs, leaving the rest of the system idle.
>>
>> This is easy to hit in practice. On an Azure L96as_v4 VM (96 vCPUs, 2
>> NUMA nodes of 48 CPUs, 6 NVMe disks with 62 I/O queues each),
>> group_cpus_evenly() splits each disk's 62 queues into 31 per node over
>> 48 CPUs. 48 does not divide evenly by 31:
>>
>>      per NUMA node: 48 CPUs / 31 queues
>>        17 groups get 2 CPUs   (cover 34 CPUs)
>>        14 groups get 1 CPU    (cover 14 CPUs)  <- single-CPU "hot" queues
>>
>> That is 14 hot queues per node, 28 per disk. All 6 disks land them on
>> the same 28 CPUs, so 168 hot interrupts pile onto 28 of 96 CPUs while
>> two-thirds of the system handles none:
>>
>>      Before (per-CPU, disks whose IRQs it services):
>>        CPU  0: 3 disks    ...   CPU 34: 6 disks (all six)
>>        CPU  1: 3 disks    ...   CPU 47: 6 disks (all six)
>>      Summary: 28 CPUs (34-47, 82-95) served all 6 disks and the other 68
>>      served only 3. Those 28 CPUs cap throughput and inflate tail
>>      latency while most of the system is idle.
>>
>> Fix this by introducing a per-caller rotation via a static atomic
>> counter (group_spread_cnt). Each call to group_cpus_evenly() takes a
>> unique spread_offset, applied to the two decisions that were previously
>> deterministic:
>>
>> 1) Cluster-level rotation in __try_group_cluster_cpus(): after
>>     alloc_groups_to_nodes() distributes groups proportionally across
>>     clusters, integer rounding leaves some clusters with one extra
>>     group. The extras are redistributed starting from a rotated
>>     position, with a stride of ncluster/total_extra to minimize overlap
>>     between consecutive callers. A multi-pass fallback ensures all
>>     extras are placed even when some clusters are at capacity.
>>
>> 2) Intra-cluster rotation in assign_cpus_to_groups(): the sequential
>>     extra assignment is replaced with a modular expression,
>>       (v + spread_offset) % nv->ngroups < extra_grps
>>     rotating which groups within a cluster receive the extra CPU.
>>
> 
> I've finally been able to review this. It took me a while to get up
> to speed on the overall approach of the existing code, and then your
> changes. I'm sure there are subtleties that I don't yet grok, so my
> comments might be off base.
>

Hi Michael,
Thank you so much for reviewing the code and testing those scenarios.


> My first question is about the placement of your (1) change above.
> It comes after alloc_groups_to_nodes() is called by
> alloc_cluster_groups(), and it modifies what alloc_groups_to_nodes()
> set up. I had expected that your (1) change would be included in
> alloc_groups_to_nodes() so that it would also be applied at the
> NUMA node level. There are cases where the NUMA node count
> might be relatively large, but the cluster count is 0 or 1. In that
> case, your (1) change is never invoked. Maybe there's a reason
> for not applying your updates at the NUMA node level, but that
> reason isn't evident to me.
> 

No, there was no reason to leave it. I coded it, and it is working in my 
initial attempt. I'll see if there are any surprises in AI review. Your 
configuration 1 would be covered with that.

>> Nothing else about the layout changes - same queue count, same NUMA
>> weighting, same full CPU coverage and locality. Each caller simply
>> starts its mapping from a different point, and each individual call
>> still produces a valid, fair distribution. Across callers, different
>> CPUs absorb the single-CPU group IRQ load:
>>
>>      After (same setup, with the rotation):
>>        CPU  0: 4 disks    CPU  2: 4 disks    CPU 47: 4 disks
>>        CPU  1: 4 disks    CPU  3: 4 disks    ...
>>      Summary: no CPU serves more than 4 disks, and all 96 CPUs are used.
>>
>> The total interrupt work is unchanged - every CPU still handles one
>> queue per disk; only the placement of the single-CPU hot queues moves.
>> This benefits every managed-IRQ, blk-mq, and virtio-vdpa / virtio-fs
>> device with no driver changes.
>>
>> Because the offset comes from a global counter advanced once per call,
>> the mapping now depends on call (device probe) order. A given device's
>> exact layout can differ from one boot to the next, and a later recompute
>> (e.g. a blk-mq remap) may pick a different layout. Every such layout is
>> still valid, fair, and proportional - only the choice among equally good
>> mappings varies.
>>
>> On a 96-vCPU Hyper-V VM running 4K random-read fio across 6 NVMe disks,
>> worst-disk degradation versus average dropped from 11% to 5%, and the
>> previously penalized disks gained 12% IOPS at 10% lower latency.
> 
> My second question is about the range of NUMA and cluster
> configurations to which you expect your changes to be applicable. You've
> cited an example above where the changes are very effective. But I
> did some experiments on other configurations, and found them to be
> less effective than I had expected. Maybe my expectations are wrong,
> or the changes have a bug or incompleteness. In at least one case,
> algorithm change (1) not being applied at the NUMA node level may
> be the cause of reduced effectiveness, though I didn't fully investigate
> the details.
> 

The idea at this moment was to cover the worst case scenarios and make 
them less bad. If you see the performance difference, its not going to 
be much, comparing to the complexity we would need to introduce in these 
functions logic.

Second design challenge here is that these APIs get called once per 
device, and one call does not know about the number of queues and number 
of devices that are going to come later.

I could make case 2 and 3 a little better, but could not immediately 
find a way to make it the best version of configuration that could be 
there. But I will think more about it, if that is possible.

Thanks for sharing your thoughts, these are good scenarios to think 
about while solving this problem.

Regards
Naman

> Here are the three configuration I tried:
> 
> 1)  Azure L48s v2 VM. This VM has 6 NUMA nodes, each with 8
> vCPUs. It has 24 clusters, each with two vCPUs that are a hyper-
> threaded pair. It has 6 NVMe controllers, each with 32 queues, so
> there are 192 IRQs to be assigned. With existing code, all 48 vCPUs
> are assigned IRQs:  32 vCPUs get 3 IRQs and 16 vCPUs get 6 IRQs,
> which is somewhat unbalanced but not terrible. With your patch,
> all vCPUs get between 3 and 5 IRQs, which is an improvement, but
> not as good as the theoretical best of 4 IRQs/vCPU. In both cases,
> the NUMA nodes are slightly unbalanced -- 2 NUMA nodes
> get 36 IRQs each, and 4 NUMA nodes get 30 IRQs each.
> 
> 2) Azure D16plds v6 VM. This is an arm64 VM with a single
> NUMA node. It has 1 cluster with all 16 vCPUs because arm64
> uses CONFIG_GENERIC_ARCH_TOPOLOGY, which makes
> clusters degenerate. It has 2 NVMe controllers, each with 6 queues.
> As expected, existing code assigns 2 IRQs each to 6 vCPUs.
> With your patch, 4 vCPUs still have 2 IRQs, while 4 vCPUs have
> 1 IRQ. I had expected that 12 vCPUs would each be assigned
> 1 IRQ, but didn't investigate why that didn't happen.
> 
> 3) Azure D96plds v6 VM. Also an arm64 VM, but with 2 NUMA
> nodes. Again, it has 1 cluster with all 96 vCPUs. It has 6 NVMe
> controllers, each with 14 queues. As expected, existing code
> assigns 6 IRQs to each of 14 vCPUs. With your patch, the IRQs
> are spread across 26 vCPUs (13 in each NUMA node) with
> counts ranging from 1 to 6. I can't discern a pattern in the
> IRQ counts, except that the pattern for each NUMA node
> is the same.
> 
> At this point, I'm just calling out my top-level observations.
> I may look more closely at the details of "why" some of
> these cases don't get much improvement.
> 
> Michael
> 

RE: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Michael Kelley]

From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM

[snip]

> @@ -510,6 +633,8 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
>  	if (!masks)
>  		goto fail_node_to_cpumask;
> 
> +	spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
> +
>  	build_node_to_cpumask(node_to_cpumask);
> 

One additional observation:  In my testing, group_cpus_evenly() is
often called with numgrps set to 1. This happens in the block "loop"
devices (drivers/block/loop.c) and for the NVMe admin queue. In
these cases, the spread_offset is never used, but group_spread_cnt
gets incremented anyway. Incrementing for NVMe admin queues
tends to dirty the spreading for multiple NVMe devices with the
same configuration because it is usually interleaved with the
spreading of the main NVMe I/O queues.

To improve this, I changed the above code to this:

+       if (numgrps == 1)
+               spread_offset = 0;
+       else
+               spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);

With this change, my configuration #1 (Azure L48s v2 VM) is noticeably
better.  All CPUs in NUMA node 1 have either 3 or 4 IRQs assigned. NUMA
node 0 ranges from 3 to 5 IRQs, but that's partly because the NUMA
nodes themselves aren't balanced, as previously discussed. With your
change to apply group_spread_cnt to the NUMA nodes, and my change
above, my config #1 is likely to work out very near optimal. Of course,
there's no guarantee that some other device won't increment
group_spread_cnt and dirty things, but for the typical case it probably
works very well.

This change to skip incrementing group_spread_cnt when numgrps == 1
doesn't help my arm64 configs. I'm still thinking about ways to do better
when there aren't any clusters. I have an idea that I'm experimenting
with, but it may be a few more days before I reach any conclusions.

Michael

Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Naman Jain]

On 8/26/2026 7:31 AM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
> 
> [snip]
> 
>> @@ -510,6 +633,8 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
>>   	if (!masks)
>>   		goto fail_node_to_cpumask;
>>
>> +	spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
>> +
>>   	build_node_to_cpumask(node_to_cpumask);
>>
> 
> One additional observation:  In my testing, group_cpus_evenly() is
> often called with numgrps set to 1. This happens in the block "loop"
> devices (drivers/block/loop.c) and for the NVMe admin queue. In
> these cases, the spread_offset is never used, but group_spread_cnt
> gets incremented anyway. Incrementing for NVMe admin queues
> tends to dirty the spreading for multiple NVMe devices with the
> same configuration because it is usually interleaved with the
> spreading of the main NVMe I/O queues.
> 
> To improve this, I changed the above code to this:
> 
> +       if (numgrps == 1)
> +               spread_offset = 0;
> +       else
> +               spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
> 
> With this change, my configuration #1 (Azure L48s v2 VM) is noticeably
> better.  All CPUs in NUMA node 1 have either 3 or 4 IRQs assigned. NUMA
> node 0 ranges from 3 to 5 IRQs, but that's partly because the NUMA
> nodes themselves aren't balanced, as previously discussed. With your
> change to apply group_spread_cnt to the NUMA nodes, and my change
> above, my config #1 is likely to work out very near optimal. Of course,
> there's no guarantee that some other device won't increment
> group_spread_cnt and dirty things, but for the typical case it probably
> works very well.
> 

Thanks for the suggestion, I tried this and it works fine.

> This change to skip incrementing group_spread_cnt when numgrps == 1
> doesn't help my arm64 configs. I'm still thinking about ways to do better
> when there aren't any clusters. I have an idea that I'm experimenting
> with, but it may be a few more days before I reach any conclusions.
> 
> Michael

Acked.

Regards,
Naman

RE: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Michael Kelley]

From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, August 27, 2026 2:11 AM
> 
> On 8/26/2026 7:31 AM, Michael Kelley wrote:
> > From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
> >
> > [snip]
> >
> >> @@ -510,6 +633,8 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
> >>   	if (!masks)
> >>   		goto fail_node_to_cpumask;
> >>
> >> +	spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
> >> +
> >>   	build_node_to_cpumask(node_to_cpumask);
> >>
> >
> > One additional observation:  In my testing, group_cpus_evenly() is
> > often called with numgrps set to 1. This happens in the block "loop"
> > devices (drivers/block/loop.c) and for the NVMe admin queue. In
> > these cases, the spread_offset is never used, but group_spread_cnt
> > gets incremented anyway. Incrementing for NVMe admin queues
> > tends to dirty the spreading for multiple NVMe devices with the
> > same configuration because it is usually interleaved with the
> > spreading of the main NVMe I/O queues.
> >
> > To improve this, I changed the above code to this:
> >
> > +       if (numgrps == 1)
> > +               spread_offset = 0;
> > +       else
> > +               spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
> >
> > With this change, my configuration #1 (Azure L48s v2 VM) is noticeably
> > better.  All CPUs in NUMA node 1 have either 3 or 4 IRQs assigned. NUMA
> > node 0 ranges from 3 to 5 IRQs, but that's partly because the NUMA
> > nodes themselves aren't balanced, as previously discussed. With your
> > change to apply group_spread_cnt to the NUMA nodes, and my change
> > above, my config #1 is likely to work out very near optimal. Of course,
> > there's no guarantee that some other device won't increment
> > group_spread_cnt and dirty things, but for the typical case it probably
> > works very well.
> >
> 
> Thanks for the suggestion, I tried this and it works fine.
> 
> > This change to skip incrementing group_spread_cnt when numgrps == 1
> > doesn't help my arm64 configs. I'm still thinking about ways to do better
> > when there aren't any clusters. I have an idea that I'm experimenting
> > with, but it may be a few more days before I reach any conclusions.
> >

I finally figured out why arm64 is different from x86. By adding a
kernel boot line parameter for controlling the number of NVMe queues
per controller (for experimentation only), I could construct identical
configs on x86 and arm64.  And even though the clustering is
different, group_cpus_evenly() returns exactly the same set of
cpumasks on the two architectures. So the clustering difference
isn't the reason for the poor spreading on arm64.

My experiments have been mostly cases where the number of NVMe
queues is small compared with the number of CPUs -- e.g., 6 NVMe
queues on each controller in a VM with 96 CPUs. In this example,
there are 16 CPUs in the cpumask for each queue. That set of 16
CPUs is the smp_affinity for the IRQ and is the same for both
architectures. But determining the single CPU that is the
effective_affinity is different. The x86 APIC vector code must load
balance assignments across CPUs because each x86 CPU has a
limited number of vectors available. At a result, x86 spreads out
which CPU in the set of 16 becomes the effective_affinity. But arm64
does not. The GICv3 function gic_set_affinity() always picks the
1st CPU in the set of 16 CPUs. So the NVMe IRQs get stacked on
the same 6 CPUs and the other 90 CPUs get none.

If the number of queues doesn't evenly divide into the number of
CPUs, then your patch provides a modest amount of spreading in
how the cpumasks are constructed, and the interrupt load gets
slightly more spread on the arm64 CPUs, but not nearly as well
as on x86.

I was previously aware of the vector load balancing done by
x86, but had never compared with what arm64 does. I had
to run a few experiments to have the light bulb come on in my
head. :-)  But I learned something so it was time well spent. 

In any case, improving the spreading in such configurations
on arm64 probably isn't the job of group_cpus_evenly(). The
arm64 GIC code would need to do some spreading when
picking the effective_affinity from the CPUs in the affinity mask.

Michael

Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Naman Jain]

On 8/31/2026 5:27 AM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, August 27, 2026 2:11 AM
>>
>> On 8/26/2026 7:31 AM, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
>>>
>>> [snip]
>>>
>>>> @@ -510,6 +633,8 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
>>>>    	if (!masks)
>>>>    		goto fail_node_to_cpumask;
>>>>
>>>> +	spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
>>>> +
>>>>    	build_node_to_cpumask(node_to_cpumask);
>>>>
>>>
>>> One additional observation:  In my testing, group_cpus_evenly() is
>>> often called with numgrps set to 1. This happens in the block "loop"
>>> devices (drivers/block/loop.c) and for the NVMe admin queue. In
>>> these cases, the spread_offset is never used, but group_spread_cnt
>>> gets incremented anyway. Incrementing for NVMe admin queues
>>> tends to dirty the spreading for multiple NVMe devices with the
>>> same configuration because it is usually interleaved with the
>>> spreading of the main NVMe I/O queues.
>>>
>>> To improve this, I changed the above code to this:
>>>
>>> +       if (numgrps == 1)
>>> +               spread_offset = 0;
>>> +       else
>>> +               spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
>>>
>>> With this change, my configuration #1 (Azure L48s v2 VM) is noticeably
>>> better.  All CPUs in NUMA node 1 have either 3 or 4 IRQs assigned. NUMA
>>> node 0 ranges from 3 to 5 IRQs, but that's partly because the NUMA
>>> nodes themselves aren't balanced, as previously discussed. With your
>>> change to apply group_spread_cnt to the NUMA nodes, and my change
>>> above, my config #1 is likely to work out very near optimal. Of course,
>>> there's no guarantee that some other device won't increment
>>> group_spread_cnt and dirty things, but for the typical case it probably
>>> works very well.
>>>
>>
>> Thanks for the suggestion, I tried this and it works fine.
>>
>>> This change to skip incrementing group_spread_cnt when numgrps == 1
>>> doesn't help my arm64 configs. I'm still thinking about ways to do better
>>> when there aren't any clusters. I have an idea that I'm experimenting
>>> with, but it may be a few more days before I reach any conclusions.
>>>
> 
> I finally figured out why arm64 is different from x86. By adding a
> kernel boot line parameter for controlling the number of NVMe queues
> per controller (for experimentation only), I could construct identical
> configs on x86 and arm64.  And even though the clustering is
> different, group_cpus_evenly() returns exactly the same set of
> cpumasks on the two architectures. So the clustering difference
> isn't the reason for the poor spreading on arm64.
> 
> My experiments have been mostly cases where the number of NVMe
> queues is small compared with the number of CPUs -- e.g., 6 NVMe
> queues on each controller in a VM with 96 CPUs. In this example,
> there are 16 CPUs in the cpumask for each queue. That set of 16
> CPUs is the smp_affinity for the IRQ and is the same for both
> architectures. But determining the single CPU that is the
> effective_affinity is different. The x86 APIC vector code must load
> balance assignments across CPUs because each x86 CPU has a
> limited number of vectors available. At a result, x86 spreads out
> which CPU in the set of 16 becomes the effective_affinity. But arm64
> does not. The GICv3 function gic_set_affinity() always picks the
> 1st CPU in the set of 16 CPUs. So the NVMe IRQs get stacked on
> the same 6 CPUs and the other 90 CPUs get none.
> 
> If the number of queues doesn't evenly divide into the number of
> CPUs, then your patch provides a modest amount of spreading in
> how the cpumasks are constructed, and the interrupt load gets
> slightly more spread on the arm64 CPUs, but not nearly as well
> as on x86.
> 
> I was previously aware of the vector load balancing done by
> x86, but had never compared with what arm64 does. I had
> to run a few experiments to have the light bulb come on in my
> head. :-)  But I learned something so it was time well spent.
> 
> In any case, improving the spreading in such configurations
> on arm64 probably isn't the job of group_cpus_evenly(). The
> arm64 GIC code would need to do some spreading when
> picking the effective_affinity from the CPUs in the affinity mask.
> 
> Michael

Thank you for looking into this and sharing this valuable information 
Michael, it means a lot.

So for now, we have two enhancements - NUMA node level balancing and 
additional (numgrps == 1) check.

I'll make these changes, evaluate them and send them in next version.

Regards,
Naman

RE: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Michael Kelley]

From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
> 
> group_cpus_evenly() computes how a device's queue interrupts are spread
> across CPUs. It backs managed-interrupt affinity (kernel/irq/affinity.c)
> and block-multiqueue mappings (block/blk-mq-cpumap.c), and is invoked
> independently by every device that uses them - NVMe, NICs, storage HBAs,
> and virtio devices. Its output is deterministic, i.e. for a given
> topology, two similar devices produce an identical group-to-CPU mapping.
> 
> When ngroups < ncpus, some groups end up with only a single CPU. An

This is not quite accurate. The condition is ncpus/2 < ngroups < ncpus.
If ngroups is <= ncpus/2, then all groups have at least 2 CPUs. If ngroups
is <= ncpus/3, then all groups have at least 3 CPUs, and so on. Rotating which
groups get the "extra" CPUs can be somewhat helpful even when all groups
have at least 2 CPUs. But as the minimum number of CPUs per group
increases, avoiding excessive stacking depends on how well the irqchip driver
does spreading when picking the effective affinity CPU from the multiple CPUs
assigned to the group (per our discussion elsewhere in this thread).

> interrupt whose mask has one CPU can only run there, making that CPU a
> "hot" handler. Because the mapping is deterministic, identical devices
> compute the same layout and stack all their single-CPU IRQs onto the
> very same CPUs, leaving the rest of the system idle.
> 
> This is easy to hit in practice. On an Azure L96as_v4 VM (96 vCPUs, 2
> NUMA nodes of 48 CPUs, 6 NVMe disks with 62 I/O queues each),
> group_cpus_evenly() splits each disk's 62 queues into 31 per node over
> 48 CPUs. 48 does not divide evenly by 31:

And FWIW, this example meets the stricter condition that I describe above ....

> 
>     per NUMA node: 48 CPUs / 31 queues
>       17 groups get 2 CPUs   (cover 34 CPUs)
>       14 groups get 1 CPU    (cover 14 CPUs)  <- single-CPU "hot" queues
> 
> That is 14 hot queues per node, 28 per disk. All 6 disks land them on
> the same 28 CPUs, so 168 hot interrupts pile onto 28 of 96 CPUs while
> two-thirds of the system handles none:
> 
>     Before (per-CPU, disks whose IRQs it services):
>       CPU  0: 3 disks    ...   CPU 34: 6 disks (all six)
>       CPU  1: 3 disks    ...   CPU 47: 6 disks (all six)
>     Summary: 28 CPUs (34-47, 82-95) served all 6 disks and the other 68
>     served only 3. Those 28 CPUs cap throughput and inflate tail
>     latency while most of the system is idle.
> 
> Fix this by introducing a per-caller rotation via a static atomic
> counter (group_spread_cnt). Each call to group_cpus_evenly() takes a
> unique spread_offset, applied to the two decisions that were previously
> deterministic:
> 
> 1) Cluster-level rotation in __try_group_cluster_cpus(): after
>    alloc_groups_to_nodes() distributes groups proportionally across
>    clusters, integer rounding leaves some clusters with one extra
>    group. The extras are redistributed starting from a rotated
>    position, with a stride of ncluster/total_extra to minimize overlap
>    between consecutive callers. A multi-pass fallback ensures all
>    extras are placed even when some clusters are at capacity.
> 
> 2) Intra-cluster rotation in assign_cpus_to_groups(): the sequential
>    extra assignment is replaced with a modular expression,
>      (v + spread_offset) % nv->ngroups < extra_grps
>    rotating which groups within a cluster receive the extra CPU.
> 
> Nothing else about the layout changes - same queue count, same NUMA
> weighting, same full CPU coverage and locality. Each caller simply
> starts its mapping from a different point, and each individual call
> still produces a valid, fair distribution. Across callers, different
> CPUs absorb the single-CPU group IRQ load:
> 
>     After (same setup, with the rotation):
>       CPU  0: 4 disks    CPU  2: 4 disks    CPU 47: 4 disks
>       CPU  1: 4 disks    CPU  3: 4 disks    ...
>     Summary: no CPU serves more than 4 disks, and all 96 CPUs are used.
> 
> The total interrupt work is unchanged - every CPU still handles one
> queue per disk; only the placement of the single-CPU hot queues moves.
> This benefits every managed-IRQ, blk-mq, and virtio-vdpa / virtio-fs
> device with no driver changes.
> 
> Because the offset comes from a global counter advanced once per call,
> the mapping now depends on call (device probe) order. A given device's
> exact layout can differ from one boot to the next, and a later recompute
> (e.g. a blk-mq remap) may pick a different layout. Every such layout is
> still valid, fair, and proportional - only the choice among equally good
> mappings varies.
> 
> On a 96-vCPU Hyper-V VM running 4K random-read fio across 6 NVMe disks,
> worst-disk degradation versus average dropped from 11% to 5%, and the
> previously penalized disks gained 12% IOPS at 10% lower latency.
> 
> Fixes: 89802ca36c96 ("lib/group_cpus: make group CPU cluster aware")
> Co-developed-by: Long Li <longli@microsoft.com>
> Signed-off-by: Long Li <longli@microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> Changes since v1
> (https://lore.kernel.org/all/20260324075352.2326972-1-namjain@linux.microsoft.com/):
> - Cluster base is now a per-cluster proportional floor
>   (ngroups * cap / ncpus) instead of the global per-cluster minimum,
>   so proportional weighting is preserved on asymmetric (e.g.
>   big.LITTLE) cluster topologies. (Sashiko review)
> - Document that the rotation offset is call/probe-order dependent: a
>   device's exact layout may vary across boots and recomputes (each
>   layout is still valid, fair, and proportional).
> - Rewrite the commit message with a worked example and fio numbers.
> 
>  lib/group_cpus.c | 149 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 137 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/group_cpus.c b/lib/group_cpus.c
> index e6e18d7a49bba..8bed0f9d2110b 100644
> --- a/lib/group_cpus.c
> +++ b/lib/group_cpus.c
> @@ -7,6 +7,7 @@
>  #include <linux/slab.h>
>  #include <linux/cpu.h>
>  #include <linux/sort.h>
> +#include <linux/atomic.h>
>  #include <linux/group_cpus.h>
> 
>  #ifdef CONFIG_SMP
> @@ -255,12 +256,20 @@ static void alloc_nodes_groups(unsigned int numgrps,
>  	alloc_groups_to_nodes(numgrps, numcpus, node_groups, nr_node_ids);
>  }
> 
> +/*
> + * Per-caller rotation counter for group_cpus_evenly().
> + * Wrapping is harmless: the offset is only used modulo small values
> + * (ncluster or nv->ngroups), so any unsigned value works.
> + */
> +static atomic_t group_spread_cnt = ATOMIC_INIT(0);
> +
>  static void assign_cpus_to_groups(unsigned int ncpus,
>  				  struct cpumask *nmsk,
>  				  struct node_groups *nv,
>  				  struct cpumask *masks,
>  				  unsigned int *curgrp,
> -				  unsigned int last_grp)
> +				  unsigned int last_grp,
> +				  unsigned int spread_offset)
>  {
>  	unsigned int v, cpus_per_grp, extra_grps;
>  	/* Account for rounding errors */
> @@ -270,11 +279,15 @@ static void assign_cpus_to_groups(unsigned int ncpus,
>  	for (v = 0; v < nv->ngroups; v++, *curgrp += 1) {
>  		cpus_per_grp = ncpus / nv->ngroups;
> 
> -		/* Account for extra groups to compensate rounding errors */
> -		if (extra_grps) {
> +		/*
> +		 * Rotate which groups get the extra CPU so that
> +		 * successive callers produce different mappings,
> +		 * avoiding IRQ stacking when multiple devices
> +		 * share the same CPU topology.
> +		 */
> +		if (extra_grps &&

Explicitly testing extra_grps for zero isn't necessary. If it is
zero, the "less than" test below will always be false anyway.

> +		    (v + spread_offset) % nv->ngroups < extra_grps)
>  			cpus_per_grp++;
> -			--extra_grps;
> -		}
> 
>  		/*
>  		 * wrapping has to be considered given 'startgrp'
> @@ -361,7 +374,8 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
>  				     struct cpumask *node_cpumask,
>  				     struct cpumask *masks,
>  				     unsigned int *curgrp,
> -				     unsigned int last_grp)
> +				     unsigned int last_grp,
> +				     unsigned int spread_offset)
>  {
>  	struct node_groups *cluster_groups;
>  	const struct cpumask **clusters;
> @@ -379,6 +393,111 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
>  	if (ncluster == 0)
>  		goto fail_no_clusters;
> 
> +	/*
> +	 * Rotate which clusters receive extra groups so that different
> +	 * callers of group_cpus_evenly() produce different group-to-CPU
> +	 * mappings. Without this, all devices get identical affinity
> +	 * masks, causing IRQ stacking on CPUs assigned single-CPU groups.

s/assigned/assigned to/

> +	 *
> +	 * alloc_groups_to_nodes() distributes ngroups proportionally, but
> +	 * integer rounding causes some clusters to get one more group
> +	 * than others. The assignment is deterministic, so every device
> +	 * gets the same mapping. Fix: compute a proportional floor for
> +	 * each cluster (ngroups * cap / ncpus), collect only the
> +	 * rounding-induced extras, then redistribute them starting from
> +	 * a rotated position. This preserves the proportional weighting
> +	 * across differently-sized clusters while rotating the rounding
> +	 * extras, keeping the rotation effective on both symmetric and
> +	 * asymmetric cluster topologies.
> +	 *
> +	 * Note: after alloc_groups_to_nodes(), cluster_groups[].ngroups
> +	 * holds the group count (the union no longer holds per-cluster CPU
> +	 * counts), so each cluster's CPU capacity (cap) is taken from its

Having to recompute "cap" three times in the code below is fairly clumsy.
Is there any reason that cluster_groups[].ncpus and .ngroups need to be a
union? If they were separate fields, then "cap" would be immediately
available when you need it. I looked back through the commit history
and LKML discussion for when the union was originally added, and I
didn't find any explanation for why it is a union. A union saves a bit
of memory, but the overall amount of memory here is small, so the
implementation doesn't need to be particularly stingy.

> +	 * mask. The ncpus divisor is the function parameter, which equals
> +	 * the sum of the per-cluster caps.
> +	 */
> +	if (ncluster > 1) {
> +		unsigned int total_extra = 0;
> +		unsigned int start, stride;
> +
> +		/*
> +		 * Compute a per-cluster proportional floor and collect
> +		 * only the rounding-induced extras for redistribution.
> +		 *
> +		 * Each cluster's floor is ngroups * cap / ncpus, which
> +		 * preserves its proportional share.  Only the rounding
> +		 * remainders (typically one per cluster) are collected
> +		 * for rotated redistribution, keeping the rotation
> +		 * effective even on asymmetric topologies (e.g.
> +		 * big.LITTLE) where differently-sized clusters would
> +		 * otherwise absorb all extras deterministically.
> +		 */
> +		for (i = 0; i < ncluster; i++) {
> +			unsigned int cap, prop_floor, base;
> +
> +			cap = cpumask_weight_and(clusters[cluster_groups[i].id],
> +						 node_cpumask);
> +			prop_floor = ngroups * cap / ncpus;
> +
> +			/*
> +			 * Use proportional floor as base.  Ensure at
> +			 * least 1 group per cluster, and never exceed
> +			 * alloc_groups_to_nodes()'s original allocation
> +			 * (which may be less than prop_floor when small
> +			 * clusters consumed groups via max(1,...)).
> +			 */
> +			base = prop_floor > 0 ? prop_floor : 1;
> +			if (base > cluster_groups[i].ngroups)
> +				base = cluster_groups[i].ngroups;
> +
> +			total_extra += cluster_groups[i].ngroups - base;
> +			cluster_groups[i].ngroups = base;
> +		}

We had a separate discussion about how your new code here should
go with alloc_groups_to_nodes() so that it is also applied at the NUMA
node level. You said you had it working. Question: Did you combine the
above "for" loop with the "for" loop in alloc_groups_to_nodes()? It
seems unnecessarily complex to do group allocations, including extras,
using the "for" loop in alloc_groups_to_nodes(), and then follow that
with another "for" loop here to figure out which nodes got extras and
strip them out. A single loop should be able to set all the nodes to
their base value and count the extras.

> +
> +		/*
> +		 * Redistribute rounding extras using a stride to scatter
> +		 * them across clusters.  With stride = ncluster / extras,
> +		 * consecutive callers' extra sets overlap minimally
> +		 * (e.g. max 2 overlap for 6 callers with 24 clusters
> +		 * and 7 extras, vs 6 overlap with stride 1).
> +		 */
> +		start = spread_offset % ncluster;
> +		stride = (total_extra > 0 && total_extra < ncluster) ?
> +			 ncluster / total_extra : 1;
> +
> +		for (i = 0; i < ncluster && total_extra > 0; i++) {
> +			unsigned int idx =
> +				(start + i * stride) % ncluster;
> +			unsigned int cap;
> +
> +			cap = cpumask_weight_and(clusters[cluster_groups[idx].id],
> +						 node_cpumask);
> +			if (cluster_groups[idx].ngroups < cap) {
> +				cluster_groups[idx].ngroups++;
> +				total_extra--;
> +			}
> +		}
> +
> +		/* Fallback: place remaining extras wherever they fit */

Just so I'm clear, you could have "remaining extras" because a stride > 1
in the previous loop could miss some clusters/nodes that have available
space, depending on how the arithmetic works out. The code below
does essentially the same thing, but with a stride of 1 so that every
node is checked.  And you might have to make multiple passes in
case some nodes have space for two or more extras. But you know
there's enough space available somewhere.

Is my understanding correct?

> +		while (total_extra > 0) {
> +			unsigned int placed = 0;
> +
> +			for (i = 0; i < ncluster && total_extra > 0; i++) {
> +				unsigned int cap;
> +
> +				cap = cpumask_weight_and(clusters[cluster_groups[i].id],
> +							 node_cpumask);
> +				if (cluster_groups[i].ngroups < cap) {
> +					cluster_groups[i].ngroups++;
> +					total_extra--;
> +					placed++;
> +				}
> +			}
> +			if (!placed)
> +				break;

Is the control variable "placed" necessary? If all the extra groups are
guaranteed to fit somewhere, then total_extra should go to zero and
the checks on total_extra > 0 will break out of the loops. Thinking about
it from the other direction, if the "for" loop ever completed without
incrementing "placed", wouldn't that mean total_extra is still
non-zero, and you have an extra group that isn't assigned to a
cluster/node?

Michael

> +		}
> +	}
> +
>  	for (i = 0; i < ncluster; i++) {
>  		struct node_groups *nv = &cluster_groups[i];
> 
> @@ -389,7 +508,8 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
>  			continue;
>  		WARN_ON_ONCE(nv->ngroups > nc);
> 
> -		assign_cpus_to_groups(nc, nmsk, nv, masks, curgrp, last_grp);
> +		assign_cpus_to_groups(nc, nmsk, nv, masks, curgrp, last_grp,
> +				      spread_offset);
>  	}
> 
>  	ret = true;
> @@ -404,7 +524,8 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
>  static int __group_cpus_evenly(unsigned int startgrp, unsigned int numgrps,
>  			       cpumask_var_t *node_to_cpumask,
>  			       const struct cpumask *cpu_mask,
> -			       struct cpumask *nmsk, struct cpumask *masks)
> +			       struct cpumask *nmsk, struct cpumask *masks,
> +			       unsigned int spread_offset)
>  {
>  	unsigned int i, n, nodes, done = 0;
>  	unsigned int last_grp = numgrps;
> @@ -455,13 +576,14 @@ static int __group_cpus_evenly(unsigned int startgrp, unsigned int numgrps,
>  		WARN_ON_ONCE(nv->ngroups > ncpus);
> 
>  		if (__try_group_cluster_cpus(ncpus, nv->ngroups, nmsk,
> -					     masks, &curgrp, last_grp)) {
> +					     masks, &curgrp, last_grp,
> +					     spread_offset)) {
>  			done += nv->ngroups;
>  			continue;
>  		}
> 
>  		assign_cpus_to_groups(ncpus, nmsk, nv, masks, &curgrp,
> -				      last_grp);
> +				      last_grp, spread_offset);
>  		done += nv->ngroups;
>  	}
>  	kfree(node_groups);
> @@ -488,6 +610,7 @@ static int __group_cpus_evenly(unsigned int startgrp, unsigned int numgrps,
>  struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
>  {
>  	unsigned int curgrp = 0, nr_present = 0, nr_others = 0;
> +	unsigned int spread_offset;
>  	cpumask_var_t *node_to_cpumask;
>  	cpumask_var_t nmsk, npresmsk;
>  	int ret = -ENOMEM;
> @@ -510,6 +633,8 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
>  	if (!masks)
>  		goto fail_node_to_cpumask;
> 
> +	spread_offset = (unsigned int)atomic_fetch_inc(&group_spread_cnt);
> +
>  	build_node_to_cpumask(node_to_cpumask);
> 
>  	/*
> @@ -528,7 +653,7 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
> 
>  	/* grouping present CPUs first */
>  	ret = __group_cpus_evenly(curgrp, numgrps, node_to_cpumask,
> -				  npresmsk, nmsk, masks);
> +				  npresmsk, nmsk, masks, spread_offset);
>  	if (ret < 0)
>  		goto fail_node_to_cpumask;
>  	nr_present = ret;
> @@ -545,7 +670,7 @@ struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
>  		curgrp = nr_present;
>  	cpumask_andnot(npresmsk, cpu_possible_mask, npresmsk);
>  	ret = __group_cpus_evenly(curgrp, numgrps, node_to_cpumask,
> -				  npresmsk, nmsk, masks);
> +				  npresmsk, nmsk, masks, spread_offset);
>  	if (ret >= 0)
>  		nr_others = ret;
> 
> --
> 2.43.0
> 

Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Naman Jain]

On 9/1/2026 5:32 AM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
>>
>> group_cpus_evenly() computes how a device's queue interrupts are spread
>> across CPUs. It backs managed-interrupt affinity (kernel/irq/affinity.c)
>> and block-multiqueue mappings (block/blk-mq-cpumap.c), and is invoked
>> independently by every device that uses them - NVMe, NICs, storage HBAs,
>> and virtio devices. Its output is deterministic, i.e. for a given
>> topology, two similar devices produce an identical group-to-CPU mapping.
>>
>> When ngroups < ncpus, some groups end up with only a single CPU. An
> 
> This is not quite accurate. The condition is ncpus/2 < ngroups < ncpus.
> If ngroups is <= ncpus/2, then all groups have at least 2 CPUs. If ngroups
> is <= ncpus/3, then all groups have at least 3 CPUs, and so on. Rotating which
> groups get the "extra" CPUs can be somewhat helpful even when all groups
> have at least 2 CPUs. But as the minimum number of CPUs per group
> increases, avoiding excessive stacking depends on how well the irqchip driver
> does spreading when picking the effective affinity CPU from the multiple CPUs
> assigned to the group (per our discussion elsewhere in this thread).
> 

You're right. I should simplify this to something like this “When the 
allocation leaves some groups with only a single CPU, those IRQs are 
forced onto the same CPUs and are stacked up.”

>> interrupt whose mask has one CPU can only run there, making that CPU a
>> "hot" handler. Because the mapping is deterministic, identical devices
>> compute the same layout and stack all their single-CPU IRQs onto the
>> very same CPUs, leaving the rest of the system idle.
>>
>> This is easy to hit in practice. On an Azure L96as_v4 VM (96 vCPUs, 2
>> NUMA nodes of 48 CPUs, 6 NVMe disks with 62 I/O queues each),
>> group_cpus_evenly() splits each disk's 62 queues into 31 per node over
>> 48 CPUs. 48 does not divide evenly by 31:
> 
> And FWIW, this example meets the stricter condition that I describe above ....

True.

> 
>>
>>      per NUMA node: 48 CPUs / 31 queues
>>        17 groups get 2 CPUs   (cover 34 CPUs)
>>        14 groups get 1 CPU    (cover 14 CPUs)  <- single-CPU "hot" queues
>>
>> That is 14 hot queues per node, 28 per disk. All 6 disks land them on
>> the same 28 CPUs, so 168 hot interrupts pile onto 28 of 96 CPUs while
>> two-thirds of the system handles none:
>>
>>      Before (per-CPU, disks whose IRQs it services):
>>        CPU  0: 3 disks    ...   CPU 34: 6 disks (all six)
>>        CPU  1: 3 disks    ...   CPU 47: 6 disks (all six)
>>      Summary: 28 CPUs (34-47, 82-95) served all 6 disks and the other 68
>>      served only 3. Those 28 CPUs cap throughput and inflate tail
>>      latency while most of the system is idle.
>>
>> Fix this by introducing a per-caller rotation via a static atomic
>> counter (group_spread_cnt). Each call to group_cpus_evenly() takes a
>> unique spread_offset, applied to the two decisions that were previously
>> deterministic:
>>
>> 1) Cluster-level rotation in __try_group_cluster_cpus(): after
>>     alloc_groups_to_nodes() distributes groups proportionally across
>>     clusters, integer rounding leaves some clusters with one extra
>>     group. The extras are redistributed starting from a rotated
>>     position, with a stride of ncluster/total_extra to minimize overlap
>>     between consecutive callers. A multi-pass fallback ensures all
>>     extras are placed even when some clusters are at capacity.
>>
>> 2) Intra-cluster rotation in assign_cpus_to_groups(): the sequential
>>     extra assignment is replaced with a modular expression,
>>       (v + spread_offset) % nv->ngroups < extra_grps
>>     rotating which groups within a cluster receive the extra CPU.
>>
>> Nothing else about the layout changes - same queue count, same NUMA
>> weighting, same full CPU coverage and locality. Each caller simply
>> starts its mapping from a different point, and each individual call
>> still produces a valid, fair distribution. Across callers, different
>> CPUs absorb the single-CPU group IRQ load:
>>
>>      After (same setup, with the rotation):
>>        CPU  0: 4 disks    CPU  2: 4 disks    CPU 47: 4 disks
>>        CPU  1: 4 disks    CPU  3: 4 disks    ...
>>      Summary: no CPU serves more than 4 disks, and all 96 CPUs are used.
>>
>> The total interrupt work is unchanged - every CPU still handles one
>> queue per disk; only the placement of the single-CPU hot queues moves.
>> This benefits every managed-IRQ, blk-mq, and virtio-vdpa / virtio-fs
>> device with no driver changes.
>>
>> Because the offset comes from a global counter advanced once per call,
>> the mapping now depends on call (device probe) order. A given device's
>> exact layout can differ from one boot to the next, and a later recompute
>> (e.g. a blk-mq remap) may pick a different layout. Every such layout is
>> still valid, fair, and proportional - only the choice among equally good
>> mappings varies.
>>
>> On a 96-vCPU Hyper-V VM running 4K random-read fio across 6 NVMe disks,
>> worst-disk degradation versus average dropped from 11% to 5%, and the
>> previously penalized disks gained 12% IOPS at 10% lower latency.
>>
>> Fixes: 89802ca36c96 ("lib/group_cpus: make group CPU cluster aware")
>> Co-developed-by: Long Li <longli@microsoft.com>
>> Signed-off-by: Long Li <longli@microsoft.com>
>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>> ---
>> Changes since v1
>> (https://lore.kernel.org/all/20260324075352.2326972-1-namjain@linux.microsoft.com/):
>> - Cluster base is now a per-cluster proportional floor
>>    (ngroups * cap / ncpus) instead of the global per-cluster minimum,
>>    so proportional weighting is preserved on asymmetric (e.g.
>>    big.LITTLE) cluster topologies. (Sashiko review)
>> - Document that the rotation offset is call/probe-order dependent: a
>>    device's exact layout may vary across boots and recomputes (each
>>    layout is still valid, fair, and proportional).
>> - Rewrite the commit message with a worked example and fio numbers.
>>
>>   lib/group_cpus.c | 149 +++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 137 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/group_cpus.c b/lib/group_cpus.c
>> index e6e18d7a49bba..8bed0f9d2110b 100644
>> --- a/lib/group_cpus.c
>> +++ b/lib/group_cpus.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/slab.h>
>>   #include <linux/cpu.h>
>>   #include <linux/sort.h>
>> +#include <linux/atomic.h>
>>   #include <linux/group_cpus.h>
>>
>>   #ifdef CONFIG_SMP
>> @@ -255,12 +256,20 @@ static void alloc_nodes_groups(unsigned int numgrps,
>>   	alloc_groups_to_nodes(numgrps, numcpus, node_groups, nr_node_ids);
>>   }
>>
>> +/*
>> + * Per-caller rotation counter for group_cpus_evenly().
>> + * Wrapping is harmless: the offset is only used modulo small values
>> + * (ncluster or nv->ngroups), so any unsigned value works.
>> + */
>> +static atomic_t group_spread_cnt = ATOMIC_INIT(0);
>> +
>>   static void assign_cpus_to_groups(unsigned int ncpus,
>>   				  struct cpumask *nmsk,
>>   				  struct node_groups *nv,
>>   				  struct cpumask *masks,
>>   				  unsigned int *curgrp,
>> -				  unsigned int last_grp)
>> +				  unsigned int last_grp,
>> +				  unsigned int spread_offset)
>>   {
>>   	unsigned int v, cpus_per_grp, extra_grps;
>>   	/* Account for rounding errors */
>> @@ -270,11 +279,15 @@ static void assign_cpus_to_groups(unsigned int ncpus,
>>   	for (v = 0; v < nv->ngroups; v++, *curgrp += 1) {
>>   		cpus_per_grp = ncpus / nv->ngroups;
>>
>> -		/* Account for extra groups to compensate rounding errors */
>> -		if (extra_grps) {
>> +		/*
>> +		 * Rotate which groups get the extra CPU so that
>> +		 * successive callers produce different mappings,
>> +		 * avoiding IRQ stacking when multiple devices
>> +		 * share the same CPU topology.
>> +		 */
>> +		if (extra_grps &&
> 
> Explicitly testing extra_grps for zero isn't necessary. If it is
> zero, the "less than" test below will always be false anyway.
> 

That is true, but the intention was to improve readability of this 
block. Unless you really feel that this should be removed, I would 
prefer to retain this.


>> +		    (v + spread_offset) % nv->ngroups < extra_grps)
>>   			cpus_per_grp++;
>> -			--extra_grps;
>> -		}
>>
>>   		/*
>>   		 * wrapping has to be considered given 'startgrp'
>> @@ -361,7 +374,8 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
>>   				     struct cpumask *node_cpumask,
>>   				     struct cpumask *masks,
>>   				     unsigned int *curgrp,
>> -				     unsigned int last_grp)
>> +				     unsigned int last_grp,
>> +				     unsigned int spread_offset)
>>   {
>>   	struct node_groups *cluster_groups;
>>   	const struct cpumask **clusters;
>> @@ -379,6 +393,111 @@ static bool __try_group_cluster_cpus(unsigned int ncpus,
>>   	if (ncluster == 0)
>>   		goto fail_no_clusters;
>>
>> +	/*
>> +	 * Rotate which clusters receive extra groups so that different
>> +	 * callers of group_cpus_evenly() produce different group-to-CPU
>> +	 * mappings. Without this, all devices get identical affinity
>> +	 * masks, causing IRQ stacking on CPUs assigned single-CPU groups.
> 
> s/assigned/assigned to/
> 
>> +	 *
>> +	 * alloc_groups_to_nodes() distributes ngroups proportionally, but
>> +	 * integer rounding causes some clusters to get one more group
>> +	 * than others. The assignment is deterministic, so every device
>> +	 * gets the same mapping. Fix: compute a proportional floor for
>> +	 * each cluster (ngroups * cap / ncpus), collect only the
>> +	 * rounding-induced extras, then redistribute them starting from
>> +	 * a rotated position. This preserves the proportional weighting
>> +	 * across differently-sized clusters while rotating the rounding
>> +	 * extras, keeping the rotation effective on both symmetric and
>> +	 * asymmetric cluster topologies.
>> +	 *
>> +	 * Note: after alloc_groups_to_nodes(), cluster_groups[].ngroups
>> +	 * holds the group count (the union no longer holds per-cluster CPU
>> +	 * counts), so each cluster's CPU capacity (cap) is taken from its
> 
> Having to recompute "cap" three times in the code below is fairly clumsy.
> Is there any reason that cluster_groups[].ncpus and .ngroups need to be a
> union? If they were separate fields, then "cap" would be immediately
> available when you need it. I looked back through the commit history
> and LKML discussion for when the union was originally added, and I
> didn't find any explanation for why it is a union. A union saves a bit
> of memory, but the overall amount of memory here is small, so the
> implementation doesn't need to be particularly stingy.

I'll remove the union, and simplify this "cap" re-computation.

> 
>> +	 * mask. The ncpus divisor is the function parameter, which equals
>> +	 * the sum of the per-cluster caps.
>> +	 */
>> +	if (ncluster > 1) {
>> +		unsigned int total_extra = 0;
>> +		unsigned int start, stride;
>> +
>> +		/*
>> +		 * Compute a per-cluster proportional floor and collect
>> +		 * only the rounding-induced extras for redistribution.
>> +		 *
>> +		 * Each cluster's floor is ngroups * cap / ncpus, which
>> +		 * preserves its proportional share.  Only the rounding
>> +		 * remainders (typically one per cluster) are collected
>> +		 * for rotated redistribution, keeping the rotation
>> +		 * effective even on asymmetric topologies (e.g.
>> +		 * big.LITTLE) where differently-sized clusters would
>> +		 * otherwise absorb all extras deterministically.
>> +		 */
>> +		for (i = 0; i < ncluster; i++) {
>> +			unsigned int cap, prop_floor, base;
>> +
>> +			cap = cpumask_weight_and(clusters[cluster_groups[i].id],
>> +						 node_cpumask);
>> +			prop_floor = ngroups * cap / ncpus;
>> +
>> +			/*
>> +			 * Use proportional floor as base.  Ensure at
>> +			 * least 1 group per cluster, and never exceed
>> +			 * alloc_groups_to_nodes()'s original allocation
>> +			 * (which may be less than prop_floor when small
>> +			 * clusters consumed groups via max(1,...)).
>> +			 */
>> +			base = prop_floor > 0 ? prop_floor : 1;
>> +			if (base > cluster_groups[i].ngroups)
>> +				base = cluster_groups[i].ngroups;
>> +
>> +			total_extra += cluster_groups[i].ngroups - base;
>> +			cluster_groups[i].ngroups = base;
>> +		}
> 
> We had a separate discussion about how your new code here should
> go with alloc_groups_to_nodes() so that it is also applied at the NUMA
> node level. You said you had it working. Question: Did you combine the
> above "for" loop with the "for" loop in alloc_groups_to_nodes()? It
> seems unnecessarily complex to do group allocations, including extras,
> using the "for" loop in alloc_groups_to_nodes(), and then follow that
> with another "for" loop here to figure out which nodes got extras and
> strip them out. A single loop should be able to set all the nodes to
> their base value and count the extras.
> 

I just did a prototype and had not combined it yet. I'll try to combine 
them into a single function now.

>> +
>> +		/*
>> +		 * Redistribute rounding extras using a stride to scatter
>> +		 * them across clusters.  With stride = ncluster / extras,
>> +		 * consecutive callers' extra sets overlap minimally
>> +		 * (e.g. max 2 overlap for 6 callers with 24 clusters
>> +		 * and 7 extras, vs 6 overlap with stride 1).
>> +		 */
>> +		start = spread_offset % ncluster;
>> +		stride = (total_extra > 0 && total_extra < ncluster) ?
>> +			 ncluster / total_extra : 1;
>> +
>> +		for (i = 0; i < ncluster && total_extra > 0; i++) {
>> +			unsigned int idx =
>> +				(start + i * stride) % ncluster;
>> +			unsigned int cap;
>> +
>> +			cap = cpumask_weight_and(clusters[cluster_groups[idx].id],
>> +						 node_cpumask);
>> +			if (cluster_groups[idx].ngroups < cap) {
>> +				cluster_groups[idx].ngroups++;
>> +				total_extra--;
>> +			}
>> +		}
>> +
>> +		/* Fallback: place remaining extras wherever they fit */
> 
> Just so I'm clear, you could have "remaining extras" because a stride > 1
> in the previous loop could miss some clusters/nodes that have available
> space, depending on how the arithmetic works out. The code below
> does essentially the same thing, but with a stride of 1 so that every
> node is checked.  And you might have to make multiple passes in
> case some nodes have space for two or more extras. But you know
> there's enough space available somewhere.
> 
> Is my understanding correct?
> 

Yes, this is correct.

>> +		while (total_extra > 0) {
>> +			unsigned int placed = 0;
>> +
>> +			for (i = 0; i < ncluster && total_extra > 0; i++) {
>> +				unsigned int cap;
>> +
>> +				cap = cpumask_weight_and(clusters[cluster_groups[i].id],
>> +							 node_cpumask);
>> +				if (cluster_groups[i].ngroups < cap) {
>> +					cluster_groups[i].ngroups++;
>> +					total_extra--;
>> +					placed++;
>> +				}
>> +			}
>> +			if (!placed)
>> +				break;
> 
> Is the control variable "placed" necessary? If all the extra groups are
> guaranteed to fit somewhere, then total_extra should go to zero and
> the checks on total_extra > 0 will break out of the loops. Thinking about
> it from the other direction, if the "for" loop ever completed without
> incrementing "placed", wouldn't that mean total_extra is still
> non-zero, and you have an extra group that isn't assigned to a
> cluster/node?
> 
> Michael

It's not required. I should remove it.

Regards,
Naman

RE: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Michael Kelley]

From: Naman Jain <namjain@linux.microsoft.com> Sent: Tuesday, September 1, 2026 10:03 PM
> 
> On 9/1/2026 5:32 AM, Michael Kelley wrote:
> > From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM

[snip]

> >> @@ -270,11 +279,15 @@ static void assign_cpus_to_groups(unsigned int ncpus,
> >>   	for (v = 0; v < nv->ngroups; v++, *curgrp += 1) {
> >>   		cpus_per_grp = ncpus / nv->ngroups;
> >>
> >> -		/* Account for extra groups to compensate rounding errors */
> >> -		if (extra_grps) {
> >> +		/*
> >> +		 * Rotate which groups get the extra CPU so that
> >> +		 * successive callers produce different mappings,
> >> +		 * avoiding IRQ stacking when multiple devices
> >> +		 * share the same CPU topology.
> >> +		 */
> >> +		if (extra_grps &&
> >
> > Explicitly testing extra_grps for zero isn't necessary. If it is
> > zero, the "less than" test below will always be false anyway.
> >
> 
> That is true, but the intention was to improve readability of this
> block. Unless you really feel that this should be removed, I would
> prefer to retain this.
> 

Fair enough. It's your call unless Thomas Gleixner as the maintainer
says otherwise. I flag these cases because I spend time looking for
some subtle reason why the test is actually needed. For example,
if the left side of the "less than" test below has side-effects, then
checking extra_gaps for zero prevents the side-effects from occurring.
To me the redundant test introduces ambiguity -- is there a reason
it is needed, or is it superfluous?

Interestingly, I had an identical conversation in another patch
review [1] three days ago.

Michael

[1] https://lore.kernel.org/linux-hyperv/8d38f7c0-f15d-4568-bc2f-559179ccb45c@redhat.com/

> 
> >> +		    (v + spread_offset) % nv->ngroups < extra_grps)
> >>   			cpus_per_grp++;
> >> -			--extra_grps;
> >> -		}

Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking

[Naman Jain]

On 9/2/2026 10:27 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Tuesday, September 1, 2026 10:03 PM
>>
>> On 9/1/2026 5:32 AM, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Sunday, August 9, 2026 11:22 PM
> 
> [snip]
> 
>>>> @@ -270,11 +279,15 @@ static void assign_cpus_to_groups(unsigned int ncpus,
>>>>    	for (v = 0; v < nv->ngroups; v++, *curgrp += 1) {
>>>>    		cpus_per_grp = ncpus / nv->ngroups;
>>>>
>>>> -		/* Account for extra groups to compensate rounding errors */
>>>> -		if (extra_grps) {
>>>> +		/*
>>>> +		 * Rotate which groups get the extra CPU so that
>>>> +		 * successive callers produce different mappings,
>>>> +		 * avoiding IRQ stacking when multiple devices
>>>> +		 * share the same CPU topology.
>>>> +		 */
>>>> +		if (extra_grps &&
>>>
>>> Explicitly testing extra_grps for zero isn't necessary. If it is
>>> zero, the "less than" test below will always be false anyway.
>>>
>>
>> That is true, but the intention was to improve readability of this
>> block. Unless you really feel that this should be removed, I would
>> prefer to retain this.
>>
> 
> Fair enough. It's your call unless Thomas Gleixner as the maintainer
> says otherwise. I flag these cases because I spend time looking for
> some subtle reason why the test is actually needed. For example,
> if the left side of the "less than" test below has side-effects, then
> checking extra_gaps for zero prevents the side-effects from occurring.
> To me the redundant test introduces ambiguity -- is there a reason
> it is needed, or is it superfluous?
> 

In my head, I was reading it like this: If there are extra groups and 
these groups satisfy this condition, do this. Other than that, it did 
not add any value.

I should remove it.

> Interestingly, I had an identical conversation in another patch
> review [1] three days ago.
>

:) Sorry for the trouble and thank you again for reviewing the patch.

Regards,
Naman