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

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

[Benjamin Herrenschmidt]

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);

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

[Michael Ellerman]

Benjamin Herrenschmidt <benh@kernel.crashing.org> 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

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

[Benjamin Herrenschmidt]

On Wed, 2017-02-08 at 21:02 +1100, Michael Ellerman wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> 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.

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

Cheers,
Ben.