| Message ID | 1314802086-26133-1-git-send-email-tom.leiming@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 31, 2011 at 10:48:06PM +0800, tom.leiming@gmail.com wrote: > Current gic_set_affinity doesn't support to route irq to all cpu, > so fix it. That is correct - we don't support routing an IRQ to _all_ CPUs because that's silly - by doing so you end up with a galloping hurd problem. As soon as such an interrupt is triggered, it will be delivered to all CPUs, and all CPUs will be woken up. All CPUs will then enter the kernel, and one will win the spinlock race. The remainder will spin on the lock while the winner sorts out what to do. That is inefficient and needlessly causes other CPUs to wake up. The point of the current code is that we route IRQs to exactly one CPU at a time to avoid that problem. > Also remove the unnecessary check for 'cpu' since cpu_online_mask > is already ADDed to produce correct one. Not everywhere - take a look at migrate_one_irq() in arch/arm/kernel/irq.c, where we want to change the current routing but without actually changing the user set affinity.
Hi, On Thu, Sep 1, 2011 at 3:04 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Aug 31, 2011 at 10:48:06PM +0800, tom.leiming@gmail.com wrote: >> Current gic_set_affinity doesn't support to route irq to all cpu, >> so fix it. > > That is correct - we don't support routing an IRQ to _all_ CPUs because > that's silly - by doing so you end up with a galloping hurd problem. As > soon as such an interrupt is triggered, it will be delivered to all CPUs, > and all CPUs will be woken up. All CPUs will then enter the kernel, and Yes, the irq will make 'WFI' exit from all CPUs, which will be wake up, but the irq is only routed into one of CPUs for irq handling. Looks like WFI is stupid designed, :-) Buf for the CPU waken up by irq handled on other CPU, it will run wfi and sleep immediately. > one will win the spinlock race. The remainder will spin on the lock while > the winner sorts out what to do. I have added a trace point in the entry of arch_irq_handler_default, and observed one irq is only routed into one of CPUs, so only one CPU will handle the irq even if gic is set to route an IRQ to _all_ CPUs. > > That is inefficient and needlessly causes other CPUs to wake up. The > point of the current code is that we route IRQs to exactly one CPU at a > time to avoid that problem. Maybe the current implementation should allow user to route IRQ to all CPUs, and no cause to just forbid it, but the default_smp_affinity can be set as only routing to one of CPUs. > >> Also remove the unnecessary check for 'cpu' since cpu_online_mask >> is already ADDed to produce correct one. > > Not everywhere - take a look at migrate_one_irq() in arch/arm/kernel/irq.c, > where we want to change the current routing but without actually changing > the user set affinity. > thanks,
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c index 3227ca9..20890f4 100644 --- a/arch/arm/common/gic.c +++ b/arch/arm/common/gic.c @@ -173,14 +173,14 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val, { void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3); unsigned int shift = (d->irq % 4) * 8; - unsigned int cpu = cpumask_any_and(mask_val, cpu_online_mask); - u32 val, mask, bit; + unsigned int cpu; + u32 val, mask, bit, cpuset = 0; - if (cpu >= 8 || cpu >= nr_cpu_ids) - return -EINVAL; + for_each_cpu_and(cpu, mask_val, cpu_online_mask) + cpuset |= 1 << cpu; mask = 0xff << shift; - bit = 1 << (cpu + shift); + bit = cpuset << shift; spin_lock(&irq_controller_lock); val = readl_relaxed(reg) & ~mask;