mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking
@ 2026-08-10  6:21 Naman Jain
  2026-08-10  8:47 ` Naman Jain
  2026-08-23 15:47 ` Michael Kelley
  0 siblings, 2 replies; 4+ messages in thread
From: Naman Jain @ 2026-08-10  6:21 UTC (permalink / raw)
  To: Andrew Morton, Thomas Gleixner, Ming Lei, Ming Lei
  Cc: Wangyang Guo, Tianyou Li, Tim Chen, Long Li, linux-kernel,
	linux-hyperv, Michael Kelley

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking
  2026-08-10  6:21 [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking Naman Jain
@ 2026-08-10  8:47 ` Naman Jain
  2026-08-23 15:47 ` Michael Kelley
  1 sibling, 0 replies; 4+ messages in thread
From: Naman Jain @ 2026-08-10  8:47 UTC (permalink / raw)
  To: Andrew Morton, Thomas Gleixner, Ming Lei, Ming Lei
  Cc: Wangyang Guo, Tianyou Li, Tim Chen, Long Li, linux-kernel,
	linux-hyperv, Michael Kelley



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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking
  2026-08-10  6:21 [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking Naman Jain
  2026-08-10  8:47 ` Naman Jain
@ 2026-08-23 15:47 ` Michael Kelley
  2026-08-24 14:35   ` Naman Jain
  1 sibling, 1 reply; 4+ messages in thread
From: Michael Kelley @ 2026-08-23 15:47 UTC (permalink / raw)
  To: Naman Jain, Andrew Morton, Thomas Gleixner, Ming Lei, Ming Lei
  Cc: Wangyang Guo, Tianyou Li, Tim Chen, Long Li, linux-kernel,
	linux-hyperv, 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.
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking
  2026-08-23 15:47 ` Michael Kelley
@ 2026-08-24 14:35   ` Naman Jain
  0 siblings, 0 replies; 4+ messages in thread
From: Naman Jain @ 2026-08-24 14:35 UTC (permalink / raw)
  To: Michael Kelley, Andrew Morton, Thomas Gleixner, Ming Lei, Ming Lei
  Cc: Wangyang Guo, Tianyou Li, Tim Chen, Long Li, linux-kernel, linux-hyperv



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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-24 14:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-10  6:21 [PATCH v2] lib/group_cpus: rotate extra groups to avoid IRQ stacking Naman Jain
2026-08-10  8:47 ` Naman Jain
2026-08-23 15:47 ` Michael Kelley
2026-08-24 14:35   ` Naman Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®