| Message ID | 1486427804.4850.104.camel@kernel.crashing.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
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
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.
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);
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(-)