genirq/matrix: Clarify CPU selection logic

[PATCH v2] genirq/matrix: Clarify CPU selection logic

Posted by Zhan Xusheng 1 week, 4 days ago

The CPU selection logic in matrix_find_best_cpu() and
matrix_find_best_cpu_managed() mixes eligibility checks with update
conditions, making the actual selection criteria harder to reason
about during review.

Refactor both loops to separate the online check from the comparison
itself and make the selection rules explicit. In
matrix_find_best_cpu(), this is a pure readability change with no
behavioral impact.

In matrix_find_best_cpu_managed(), the refactoring also avoids updating
best_cpu when CPUs have identical managed_allocated counts, removing an
implicit tie-breaking behavior based on CPU iteration order.

The intended selection policy is unchanged, except that equal cases in
the managed path no longer trigger redundant best_cpu updates.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/irq/matrix.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
index a50f2305a8dc..ed4b1e44dc1e 100644
--- a/kernel/irq/matrix.c
+++ b/kernel/irq/matrix.c
@@ -140,14 +140,17 @@ static unsigned int matrix_find_best_cpu(struct irq_matrix *m,
 
 	best_cpu = UINT_MAX;
 
+	/* Select the online CPU with the most available vectors */
 	for_each_cpu(cpu, msk) {
 		cm = per_cpu_ptr(m->maps, cpu);
 
-		if (!cm->online || cm->available <= maxavl)
+		if (!cm->online)
 			continue;
 
-		best_cpu = cpu;
-		maxavl = cm->available;
+		if (cm->available > maxavl) {
+			best_cpu = cpu;
+			maxavl = cm->available;
+		}
 	}
 	return best_cpu;
 }
@@ -161,14 +164,17 @@ static unsigned int matrix_find_best_cpu_managed(struct irq_matrix *m,
 
 	best_cpu = UINT_MAX;
 
+	/* Select the online CPU with the fewest managed allocated vectors */
 	for_each_cpu(cpu, msk) {
 		cm = per_cpu_ptr(m->maps, cpu);
 
-		if (!cm->online || cm->managed_allocated > allocated)
+		if (!cm->online)
 			continue;
 
-		best_cpu = cpu;
-		allocated = cm->managed_allocated;
+		if (cm->managed_allocated < allocated) {
+			best_cpu = cpu;
+			allocated = cm->managed_allocated;
+		}
 	}
 	return best_cpu;
 }
-- 
2.43.0

Re: [PATCH v2] genirq/matrix: Clarify CPU selection logic

Posted by Thomas Gleixner 1 week, 4 days ago

On Tue, Jan 27 2026 at 10:41, Zhan Xusheng wrote:
> The CPU selection logic in matrix_find_best_cpu() and
> matrix_find_best_cpu_managed() mixes eligibility checks with update
> conditions, making the actual selection criteria harder to reason
> about during review.
>
> Refactor both loops to separate the online check from the comparison
> itself and make the selection rules explicit. In
> matrix_find_best_cpu(), this is a pure readability change with no
> behavioral impact.
>
> In matrix_find_best_cpu_managed(), the refactoring also avoids updating
> best_cpu when CPUs have identical managed_allocated counts, removing an
> implicit tie-breaking behavior based on CPU iteration order.

... by replacing it with a different tie-breaking behaviour based on CPU
iteration order. What's the actual improvement here?

> The intended selection policy is unchanged, except that equal cases in
> the managed path no longer trigger redundant best_cpu updates.

You're doing two things at once. The selection logic change is
completely separate from the "polishing" and it's clearly documented
that functional changes have to be separated from others.

If your main objective is to adjust the tie-breaking logic, then you can
do that with a single character insertion plus a reasonable explanation
why it matters and leave the otherwise perfectly readable and
understandable code alone.

Thanks,

        tglx

[PATCH v3 0/2] genirq/matrix: CPU selection cleanup and tie-breaking fix

Posted by Zhan Xusheng 1 week, 3 days ago

This version splits the previous change into two patches:
Patch 1 contains only a readability refactoring with no functional change.
Patch 2 contains the intentional tie-breaking behavior change.

This addresses the review feedback on separating functional changes
from code cleanups.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
-- 
2.43.0

[PATCH v3 1/2] genirq/matrix: Clarify CPU selection logic

Posted by Zhan Xusheng 1 week, 3 days ago

The CPU selection loops in matrix_find_best_cpu() and
matrix_find_best_cpu_managed() mix the online check with the
comparison that determines whether a CPU becomes the current
best candidate.

Restructure the loops to filter out offline CPUs first and then
perform the comparison separately. This makes the selection
criteria easier to read and reason about.

No functional change intended.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/irq/matrix.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
index a50f2305a8dc..c363e918087c 100644
--- a/kernel/irq/matrix.c
+++ b/kernel/irq/matrix.c
@@ -140,14 +140,17 @@ static unsigned int matrix_find_best_cpu(struct irq_matrix *m,
 
 	best_cpu = UINT_MAX;
 
+	/* Select the online CPU with the most available vectors */
 	for_each_cpu(cpu, msk) {
 		cm = per_cpu_ptr(m->maps, cpu);
 
-		if (!cm->online || cm->available <= maxavl)
+		if (!cm->online)
 			continue;
 
-		best_cpu = cpu;
-		maxavl = cm->available;
+		if (cm->available > maxavl) {
+			best_cpu = cpu;
+			maxavl = cm->available;
+		}
 	}
 	return best_cpu;
 }
@@ -161,14 +164,17 @@ static unsigned int matrix_find_best_cpu_managed(struct irq_matrix *m,
 
 	best_cpu = UINT_MAX;
 
+	/* Select the online CPU with the fewest managed allocated vectors */
 	for_each_cpu(cpu, msk) {
 		cm = per_cpu_ptr(m->maps, cpu);
 
-		if (!cm->online || cm->managed_allocated > allocated)
+		if (!cm->online)
 			continue;
 
-		best_cpu = cpu;
-		allocated = cm->managed_allocated;
+		if (cm->managed_allocated <= allocated) {
+			best_cpu = cpu;
+			allocated = cm->managed_allocated;
+		}
 	}
 	return best_cpu;
 }
-- 
2.43.0

Re: [PATCH v3 1/2] genirq/matrix: Clarify CPU selection logic

Posted by Thomas Gleixner 1 week, 2 days ago

On Wed, Jan 28 2026 at 11:14, Zhan Xusheng wrote:
> The CPU selection loops in matrix_find_best_cpu() and
> matrix_find_best_cpu_managed() mix the online check with the
> comparison that determines whether a CPU becomes the current
> best candidate.

What's wrong about that?

> Restructure the loops to filter out offline CPUs first and then
> perform the comparison separately. This makes the selection
> criteria easier to read and reason about.

It's just different and not any better. We are not reshuffling perfectly
correct code just to accomodate with personal taste of someone.

Thanks,

        tglx

[PATCH v3 2/2] genirq/matrix: Avoid implicit tie-breaking by CPU iteration order

Posted by Zhan Xusheng 1 week, 3 days ago

matrix_find_best_cpu_managed() updates best_cpu even when two CPUs
have the same managed_allocated count. As a result, the final
selection implicitly depends on CPU iteration order.

Update the comparison to replace the current best CPU only when a
CPU has a strictly smaller managed_allocated value. This removes
the iteration-order-based tie-breaking for equal cases.

This patch intentionally changes the selection behavior.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/irq/matrix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
index c363e918087c..ed4b1e44dc1e 100644
--- a/kernel/irq/matrix.c
+++ b/kernel/irq/matrix.c
@@ -171,7 +171,7 @@ static unsigned int matrix_find_best_cpu_managed(struct irq_matrix *m,
 		if (!cm->online)
 			continue;
 
-		if (cm->managed_allocated <= allocated) {
+		if (cm->managed_allocated < allocated) {
 			best_cpu = cpu;
 			allocated = cm->managed_allocated;
 		}
-- 
2.43.0

Re: [PATCH v3 2/2] genirq/matrix: Avoid implicit tie-breaking by CPU iteration order

Posted by Thomas Gleixner 1 week, 2 days ago

On Wed, Jan 28 2026 at 11:14, Zhan Xusheng wrote:
> matrix_find_best_cpu_managed() updates best_cpu even when two CPUs
> have the same managed_allocated count. As a result, the final
> selection implicitly depends on CPU iteration order.

And?

> Update the comparison to replace the current best CPU only when a
> CPU has a strictly smaller managed_allocated value. This removes
> the iteration-order-based tie-breaking for equal cases.

And replaces it by a different iteration order based decision, no?

What are you actually trying to solve here and why does it matter at
all? If it solves nothing then what's the point?

> This patch intentionally changes the selection behavior.

1) # git grep 'This patch' Documentation/process/

2) It's already clear from the above that this changes the behaviour, so
   no point in repeating the obvious.

   The phrase "No functional change intended" is very different because
   it tells the reviewer that this is a pure code refactoring.

Thanks,

        tglx