Skip to contentSkip to conversation

genirq/affinity: Spread IRQs to all available NUMA nodes

PATCH Superseded Long Li 3 participants 3 messages Nov 1, 2018 – Nov 2, 2018 IRQ

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Long Li

author
#0 lore raw

PATCH 1/1 genirq/affinity: Spread IRQs to all available NUMA nodes

From: Long Li <[email protected]>

On systems with large number of NUMA nodes, there may be more NUMA nodes than the number of MSI/MSI-X interrupts that device requests for. The current code always picks up the NUMA nodes starting from the node 0, up to the number of interrupts requested. This may left some later NUMA nodes unused.

For example, if the system has 16 NUMA nodes, and the device reqeusts for 8 interrupts, NUMA node 0 to 7 are assigned for those interrupts, NUMA 8 to 15 are unused.

There are several problems with this approach:
1. Later, when those managed IRQs are allocated, they can not be assigned to NUMA 8 to 15, this may create an IRQ concentration on NUMA 0 to 7.
2. Some upper layers assume affinity mask has a complete coverage over NUMA nodes. For example, block layer use the affinity mask to decide how to map CPU queues to hardware queues, missing NUMA nodes in the masks may result in an uneven mapping of queues. For the above example of 16 NUMA nodes, CPU queues on NUMA node 0 to 7 are assigned to the hardware queues 0 to 7, respectively. But CPU queues on NUMA node 8 to 15 are all assigned to the hardware queue 0.

Fix this problem by going over all NUMA nodes and assign them round-robin to all IRQs.

---
 kernel/irq/affinity.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
kernel/irq/affinity.c +4 -3
index f4f29b9d90ee..2d08b560d4b6 100644
@@ -117,12 +117,13 @@ static int irq_build_affinity_masks(const struct irq_affinity *affd,
 	 */
 	if (numvecs <= nodes) {
 		for_each_node_mask(n, nodemsk) {
-			cpumask_copy(masks + curvec, node_to_cpumask[n]);
-			if (++done == numvecs)
-				break;
+			cpumask_or(masks + curvec, masks + curvec, node_to_cpumask[n]);
+			done++;
 			if (++curvec == last_affv)
 				curvec = affd->pre_vectors;
 		}
+		if (done > numvecs)
+			done = numvecs;
 		goto out;
 	}
 
Signature · 1 line
2.14.1
#1 lore raw

From: Long Li <[email protected]> Sent: Thursday, November 1, 2018 4:52 PM

Quoted 5 lines
kernel/irq/affinity.c +0 -0
@@ -117,12 +117,13 @@ static int irq_build_affinity_masks(const struct irq_affinity *affd,
 	 */

kernel/irq/affinity.c static int irq_build_affinity_masks(const struct irq_affinity *affd, · line 124

Quoted 10 lines
kernel/irq/affinity.c +2 -3
 	if (numvecs <= nodes) {
 		for_each_node_mask(n, nodemsk) {
-			cpumask_copy(masks + curvec, node_to_cpumask[n]);
-			if (++done == numvecs)
-				break;
+			cpumask_or(masks + curvec, masks + curvec, node_to_cpumask[n]);
+			done++;
 			if (++curvec == last_affv)
 				curvec = affd->pre_vectors;
 		}

When the above for loop is exited, 'done' will always be equal to 'nodes' since there are no early exits from the loop. Hence there's no need to be incrementing 'done' in the loop.

kernel/irq/affinity.c static int irq_build_affinity_masks(const struct irq_affinity *affd, · line 126

Quoted 2 lines
kernel/irq/affinity.c +2 -0
+		if (done > numvecs)
+			done = numvecs;

And if 'done' would always be equal to 'nodes', there is no need for the test. Just always set 'done' to 'numvecs'.

Quoted 5 lines
kernel/irq/affinity.c +0 -1
 		goto out;
 	}
 
--

2.14.1

#2 lore raw
Quoted 28 lines

Subject: RE: [PATCH] genirq/affinity: Spread IRQs to all available NUMA nodes

From: Long Li <[email protected]> Sent: Thursday, November 1, 2018 4:52 PM

kernel/irq/affinity.c static int irq_build_affinity_masks(const struct

Quoted 4 lines
kernel/irq/affinity.c +0 -0
@@ -117,12 +117,13 @@ static int irq_build_affinity_masks(const struct

irq_affinity *affd,

kernel/irq/affinity.c static int irq_build_affinity_masks(const struct irq_affinity *affd,

Quoted 4 lines
kernel/irq/affinity.c +0 -1
 	 */
 	if (numvecs <= nodes) {
 		for_each_node_mask(n, nodemsk) {
-			cpumask_copy(masks + curvec,

node_to_cpumask[n]);

kernel/irq/affinity.c static int irq_build_affinity_masks(const struct irq_affinity *affd,

Quoted 3 lines
kernel/irq/affinity.c +1 -2
-			if (++done == numvecs)
-				break;
+			cpumask_or(masks + curvec, masks + curvec,

node_to_cpumask[n]);

kernel/irq/affinity.c static int irq_build_affinity_masks(const struct irq_affinity *affd, · line 124

Quoted 4 lines
kernel/irq/affinity.c +1 -0
+			done++;
 			if (++curvec == last_affv)
 				curvec = affd->pre_vectors;
 		}

When the above for loop is exited, 'done' will always be equal to 'nodes' since there are no early exits from the loop. Hence there's no need to be incrementing 'done' in the loop.

Quoted 5 lines

kernel/irq/affinity.c static int irq_build_affinity_masks(const struct irq_affinity *affd, · line 126

Quoted 2 lines
kernel/irq/affinity.c +2 -0
+		if (done > numvecs)
+			done = numvecs;

And if 'done' would always be equal to 'nodes', there is no need for the test. Just always set 'done' to 'numvecs'.

Thanks. I will fix this in v2.

Quoted 6 lines
Quoted 6 lines
kernel/irq/affinity.c +0 -1
 		goto out;
 	}
 
--

2.14.1