[PATCH v2 1/2] irq/matrix: Split out the CPU finding code into a helper

[PATCH v2 1/2] irq/matrix: Split out the CPU finding code into a helper

[Dou Liyang]

From: Dou Liyang <douly.fnst@cn.fujitsu.com>

Linux finds the CPU which has the lowest vector allocation count to spread
out the non managed interrupt across the possible target CPUs.

This common CPU finding code will also be used in managed case,

So Split it out into a helper for preparation.

Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
Changelog v2 --> v1

 -Avoid a churn and the extra indentation suggested by tglx

 kernel/irq/matrix.c | 60 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 25 deletions(-)

diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
index 5092494bf261..0cbcdef9337a 100644
--- a/kernel/irq/matrix.c
+++ b/kernel/irq/matrix.c
@@ -124,6 +124,26 @@ static unsigned int matrix_alloc_area(struct irq_matrix *m, struct cpumap *cm,
 	return area;
 }
 
+/* Find the best CPU which has the lowest vector allocation count */
+static int matrix_find_best_cpu(struct irq_matrix *m,
+				const struct cpumask *msk, int *best_cpu)
+{
+	unsigned int cpu, maxavl = 0;
+	struct cpumap *cm;
+
+	for_each_cpu(cpu, msk) {
+		cm = per_cpu_ptr(m->maps, cpu);
+
+		if (!cm->online || cm->available <= maxavl)
+			continue;
+
+		*best_cpu = cpu;
+		maxavl = cm->available;
+	}
+
+	return maxavl;
+}
+
 /**
  * irq_matrix_assign_system - Assign system wide entry in the matrix
  * @m:		Matrix pointer
@@ -322,35 +342,25 @@ void irq_matrix_remove_reserved(struct irq_matrix *m)
 int irq_matrix_alloc(struct irq_matrix *m, const struct cpumask *msk,
 		     bool reserved, unsigned int *mapped_cpu)
 {
-	unsigned int cpu, best_cpu, maxavl = 0;
+	unsigned int best_cpu = UINT_MAX;
 	struct cpumap *cm;
 	unsigned int bit;
 
-	best_cpu = UINT_MAX;
-	for_each_cpu(cpu, msk) {
-		cm = per_cpu_ptr(m->maps, cpu);
-
-		if (!cm->online || cm->available <= maxavl)
-			continue;
-
-		best_cpu = cpu;
-		maxavl = cm->available;
-	}
+	if (!matrix_find_best_cpu(m, msk, &best_cpu))
+		return -ENOSPC;
 
-	if (maxavl) {
-		cm = per_cpu_ptr(m->maps, best_cpu);
-		bit = matrix_alloc_area(m, cm, 1, false);
-		if (bit < m->alloc_end) {
-			cm->allocated++;
-			cm->available--;
-			m->total_allocated++;
-			m->global_available--;
-			if (reserved)
-				m->global_reserved--;
-			*mapped_cpu = best_cpu;
-			trace_irq_matrix_alloc(bit, best_cpu, m, cm);
-			return bit;
-		}
+	cm = per_cpu_ptr(m->maps, best_cpu);
+	bit = matrix_alloc_area(m, cm, 1, false);
+	if (bit < m->alloc_end) {
+		cm->allocated++;
+		cm->available--;
+		m->total_allocated++;
+		m->global_available--;
+		if (reserved)
+			m->global_reserved--;
+		*mapped_cpu = best_cpu;
+		trace_irq_matrix_alloc(bit, best_cpu, m, cm);
+		return bit;
 	}
 	return -ENOSPC;
 }
-- 
2.14.3

[PATCH v2 2/2] irq/matrix: Spread managed interrupts on allocation

[Dou Liyang]

From: Dou Liyang <douly.fnst@cn.fujitsu.com>

Linux has spread out the non managed interrupt across the possible
target CPUs to avoid vector space exhaustion.

But, the same situation may happen on the managed interrupts.

Spread managed interrupt on allocation as well.

Fixes: a0c9259dc4e1("irq/matrix: Spread interrupts on allocation")
Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com>
---
Changelog v2 --> v1

 -Avoid a churn and the extra indentation suggested by tglx

 arch/x86/kernel/apic/vector.c |  8 +++-----
 include/linux/irq.h           |  3 ++-
 kernel/irq/matrix.c           | 14 +++++++++++---
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 9f148e3d45b4..b7fc290b4b98 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -313,14 +313,12 @@ assign_managed_vector(struct irq_data *irqd, const struct cpumask *dest)
 	struct apic_chip_data *apicd = apic_chip_data(irqd);
 	int vector, cpu;
 
-	cpumask_and(vector_searchmask, vector_searchmask, affmsk);
-	cpu = cpumask_first(vector_searchmask);
-	if (cpu >= nr_cpu_ids)
-		return -EINVAL;
+	cpumask_and(vector_searchmask, dest, affmsk);
+
 	/* set_affinity might call here for nothing */
 	if (apicd->vector && cpumask_test_cpu(apicd->cpu, vector_searchmask))
 		return 0;
-	vector = irq_matrix_alloc_managed(vector_matrix, cpu);
+	vector = irq_matrix_alloc_managed(vector_matrix, vector_searchmask, &cpu);
 	trace_vector_alloc_managed(irqd->irq, vector, vector);
 	if (vector < 0)
 		return vector;
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 201de12a9957..c9bffda04a45 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -1151,7 +1151,8 @@ void irq_matrix_offline(struct irq_matrix *m);
 void irq_matrix_assign_system(struct irq_matrix *m, unsigned int bit, bool replace);
 int irq_matrix_reserve_managed(struct irq_matrix *m, const struct cpumask *msk);
 void irq_matrix_remove_managed(struct irq_matrix *m, const struct cpumask *msk);
-int irq_matrix_alloc_managed(struct irq_matrix *m, unsigned int cpu);
+int irq_matrix_alloc_managed(struct irq_matrix *m, const struct cpumask *msk,
+				unsigned int *mapped_cpu);
 void irq_matrix_reserve(struct irq_matrix *m);
 void irq_matrix_remove_reserved(struct irq_matrix *m);
 int irq_matrix_alloc(struct irq_matrix *m, const struct cpumask *msk,
diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
index 0cbcdef9337a..753b8b0ba045 100644
--- a/kernel/irq/matrix.c
+++ b/kernel/irq/matrix.c
@@ -259,11 +259,18 @@ void irq_matrix_remove_managed(struct irq_matrix *m, const struct cpumask *msk)
  * @m:		Matrix pointer
  * @cpu:	On which CPU the interrupt should be allocated
  */
-int irq_matrix_alloc_managed(struct irq_matrix *m, unsigned int cpu)
+int irq_matrix_alloc_managed(struct irq_matrix *m, const struct cpumask *msk,
+				unsigned int *mapped_cpu)
 {
-	struct cpumap *cm = per_cpu_ptr(m->maps, cpu);
 	unsigned int bit, end = m->alloc_end;
+	unsigned int best_cpu = UINT_MAX;
+	struct cpumap *cm;
 
+	if (!matrix_find_best_cpu(m, msk, &best_cpu))
+		return -ENOSPC;
+
+	cm = per_cpu_ptr(m->maps, best_cpu);
+	end = m->alloc_end;
 	/* Get managed bit which are not allocated */
 	bitmap_andnot(m->scratch_map, cm->managed_map, cm->alloc_map, end);
 	bit = find_first_bit(m->scratch_map, end);
@@ -272,7 +279,8 @@ int irq_matrix_alloc_managed(struct irq_matrix *m, unsigned int cpu)
 	set_bit(bit, cm->alloc_map);
 	cm->allocated++;
 	m->total_allocated++;
-	trace_irq_matrix_alloc_managed(bit, cpu, m, cm);
+	*mapped_cpu = best_cpu;
+	trace_irq_matrix_alloc_managed(bit, best_cpu, m, cm);
 	return bit;
 }
 
-- 
2.14.3

Re: [PATCH v2 1/2] irq/matrix: Split out the CPU finding code into a helper

[Thomas Gleixner]

On Sat, 8 Sep 2018, Dou Liyang wrote:
> +/* Find the best CPU which has the lowest vector allocation count */
> +static int matrix_find_best_cpu(struct irq_matrix *m,
> +				const struct cpumask *msk, int *best_cpu)
> +{
> +	unsigned int cpu, maxavl = 0;
> +	struct cpumap *cm;
> +
> +	for_each_cpu(cpu, msk) {
> +		cm = per_cpu_ptr(m->maps, cpu);
> +
> +		if (!cm->online || cm->available <= maxavl)
> +			continue;
> +
> +		*best_cpu = cpu;
> +		maxavl = cm->available;
> +	}
> +
> +	return maxavl;

This return value makes no sense whatsoever. Why not doing the obvious

static unsigned int matrix_find_best_cpu(struct irq_matrix *m,
	                             	 const struct cpumask *msk)
{
	unsigned int cpu, best_cpu, maxavl = 0;

	best_cpu = UINT_MAX;
	.....
	return best_cpu;
}

and at the call site:

       cpu = matrix_find_best_cpu();
       if (cpu == UINT_MAX)
       		return -ENOSPC;

Hmm?

> +	cm = per_cpu_ptr(m->maps, best_cpu);
> +	bit = matrix_alloc_area(m, cm, 1, false);
> +	if (bit < m->alloc_end) {

So if you're removing one indentation level, then it'd be consequent to
remove this one as well while at it :)

Thanks,

	tglx

Re: [PATCH v2 2/2] irq/matrix: Spread managed interrupts on allocation

[Thomas Gleixner]

On Sat, 8 Sep 2018, Dou Liyang wrote:
> Spread managed interrupt on allocation as well.
> 
> Fixes: a0c9259dc4e1("irq/matrix: Spread interrupts on allocation")

No. This is an enhancement and not a fix.

> -	cpumask_and(vector_searchmask, vector_searchmask, affmsk);
> -	cpu = cpumask_first(vector_searchmask);
> -	if (cpu >= nr_cpu_ids)
> -		return -EINVAL;

Removing this check changes the return value for the empty search mask case
from EINVAL to ENOSPC. Doesn't matter much I think, but it might be worth
to mention in the changelog.

Thanks,

	tglx