[PATCH] powerpc/irq: Improve/fix migrate_irqs()

migrate_irqs() is used by some platforms to migrate interrupts
away from a CPU about to be offlined.

The current implementation had various issues such as not taking
the descriptor lock before manipulating it. This refactors it
a bit, fixing that problem at the same time.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/kernel/irq.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index a018f5c..52308b4 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c

@@ -456,22 +456,31 @@ void migrate_irqs(void)
 	for_each_irq_desc(irq, desc) {
 		struct irq_data *data;
 		struct irq_chip *chip;
+		const struct cpumask *affinity;
 
-		data = irq_desc_get_irq_data(desc);
-		if (irqd_is_per_cpu(data))
-			continue;
+		/* Interrupts are already disabled */
+		raw_spin_lock(&desc->lock);
 
+		data = irq_desc_get_irq_data(desc);
+		affinity = irq_data_get_affinity_mask(data);
 		chip = irq_data_get_irq_chip(data);
+		if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
+		    cpumask_subset(affinity, cpu_online_mask) ||
+		    !chip) {
+			raw_spin_unlock(&desc->lock);
+			continue;
+		}
 
-		cpumask_and(mask, irq_data_get_affinity_mask(data), map);
-		if (cpumask_any(mask) >= nr_cpu_ids) {
+		cpumask_and(mask, affinity, map);
+		if (cpumask_empty(mask)) {
 			pr_warn("Breaking affinity for irq %i\n", irq);
 			cpumask_copy(mask, map);
 		}
 		if (chip->irq_set_affinity)
 			chip->irq_set_affinity(data, mask, true);
 		else if (desc->action && !(warned++))
-			pr_err("Cannot set affinity for irq %i\n", irq);
+			pr_debug("Cannot set affinity for irq %i\n", irq);
+		raw_spin_unlock(&desc->lock);
 	}
 
 	free_cpumask_var(mask);

Benjamin Herrenschmidt [off-list ref] writes:

migrate_irqs() is used by some platforms to migrate interrupts
away from a CPU about to be offlined.

The current implementation had various issues such as not taking
the descriptor lock before manipulating it.

... and not checking for a NULL chip (but presumably that's never
happened), and always calling irq_set_affinity() even for IRQs not on
the CPU.

But, any reason we can't use irq_migrate_all_off_this_cpu() ?
Which is in generic code.

cheers

On Wed, 2017-02-08 at 21:02 +1100, Michael Ellerman wrote:

Benjamin Herrenschmidt [off-list ref] writes:

quoted

migrate_irqs() is used by some platforms to migrate interrupts
away from a CPU about to be offlined.

The current implementation had various issues such as not taking
the descriptor lock before manipulating it.

... and not checking for a NULL chip (but presumably that's never
happened), and always calling irq_set_affinity() even for IRQs not on
the CPU.

But, any reason we can't use irq_migrate_all_off_this_cpu() ?
Which is in generic code.

Nope, I didn't notice it. I'll give that a spin.

Cheers,
Ben.