irqchip: gic: Allow setting affinity to offline CPUs
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
Sometimes it is necessary to fix interrupt affinity to an offline CPU, for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible CPU if no online CPU can be found in requested CPU mask.
This fixes broken Exynos4210 support since commit
ee98d27df6 ARM: EXYNOS4: Divorce mct from local timer API
caused by timer initialization code unable to set affinity for local timer interrupts.
--- drivers/irqchip/irq-gic.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/irqchip/irq-gic.c +9 -0
@@ -250,6 +250,15 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val,unsigned int cpu =cpumask_any_and (mask_val, cpu_online_mask);u32 val, mask, bit; +/* +* If no online CPU could be found, fall back to any possible CPU. +* +* This is to allow setting affinity of some interrupts to a CPU +* before it is marked as online, i.e. local timer initialization. +*/ +if (cpu >= nr_cpu_ids) + cpu =cpumask_any_and (mask_val, cpu_possible_mask); +if (cpu >=NR_GIC_CPU_IF || cpu >= nr_cpu_ids)return -EINVAL ;
Signature · 1 line
1.8.3.2
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
Quoted 9 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU, for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible CPU if no online CPU can be found in requested CPU mask.
This fixes broken Exynos4210 support since commit
ee98d27df6ARM: EXYNOS4: Divorce mct from local timer API
caused by timer initialization code unable to set affinity for local timer interrupts.
Care to elaborate further? I don't see how the interrupt affinity is set for a CPU that isn't online because the mct code runs on the CPU that the affinity is being set to.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
Quoted 9 lines · On Tuesday 20 of August 2013 09:33:31 Stephen Boyd wrote:
Quoted 8 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU, for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible
CPU if no online CPU can be found in requested CPU mask.This fixes broken Exynos4210 support since commit
Quoted 8 lines
Quoted 4 lines
ee98d27df6ARM: EXYNOS4: Divorce mct from local timer APIcaused by timer initialization code unable to set affinity for local timer interrupts.
Care to elaborate further? I don't see how the interrupt affinity is set for a CPU that isn't online because the mct code runs on the CPU that the affinity is being set to.
Well, please look at secondary_start_kernel() in arch/arm/kernel/smp.c [1]. You can see that notify_cpu_starting() (line 348) that fires the notifier registered in MCT driver is called before set_cpu_online() (line 359) that marks the CPU as online. Also notice that, originally, local timer initialization was happening after set_cpu_online() - see line 365.
[1] - http://lxr.free-electrons.com/source/arch/arm/kernel/smp.c?a=arm#L312
Best regards,
Tomasz
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
Quoted 14 lines · Tomasz Figa
Quoted 13 lines · On Tuesday 20 of August 2013 09:33:31 Stephen Boyd wrote:
Quoted 12 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU, for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible
CPU if no online CPU can be found in requested CPU mask.This fixes broken Exynos4210 support since commit
ee98d27df6ARM: EXYNOS4: Divorce mct from local timer APIcaused by timer initialization code unable to set affinity for local timer interrupts.
Quoted 10 lines
Quoted 3 lines
Care to elaborate further? I don't see how the interrupt affinity is set for a CPU that isn't online because the mct code runs on the CPU that the affinity is being set to.
Well, please look at
secondary_start_kernel()inarch/arm/kernel/smp.c[1]. You can see thatnotify_cpu_starting()(line 348) that fires the notifier registered in MCT driver is called beforeset_cpu_online()(line 359) that marks the CPU as online. Also notice that, originally, local timer initialization was happening afterset_cpu_online()- see line 365.
Great, thank you. Please put this information in the commit text next time.
I wonder if we shouldn't make the cpumask_any_and() work on the present mask instead? If we ever support physical hotplug on ARM I think we wouldn't want to allow interrupts to go to CPUs that aren't even present (but still possible).
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
Quoted 22 lines · On Tuesday 20 of August 2013 09:48:25 Stephen Boyd wrote:
Quoted 21 lines · Tomasz Figa
Quoted 19 lines · On Tuesday 20 of August 2013 09:33:31 Stephen Boyd wrote:
Quoted 7 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU,
for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible
CPU if no online CPU can be found in requested CPU mask.Quoted 6 lines
This fixes broken Exynos4210 support since commit
ee98d27df6ARM: EXYNOS4: Divorce mct from local timer APIcaused by timer initialization code unable to set affinity for local timer interrupts.
Care to elaborate further? I don't see how the interrupt affinity is set
for a CPU that isn't online because the mct code runs on the CPU that the affinity is being set to.
Quoted 8 lines
Quoted 6 lines
Well, please look at
secondary_start_kernel()inarch/arm/kernel/smp.c[1]. You can see thatnotify_cpu_starting()(line 348) that fires the notifier registered in MCT driver is called beforeset_cpu_online()(line 359) that marks the CPU as online. Also notice that, originally, local timer initialization was happening afterset_cpu_online()- see line 365.Great, thank you. Please put this information in the commit text next time.
Right, I could have added a sentence or two about this.
Quoted 4 lines
I wonder if we shouldn't make the
cpumask_any_and()work on the present mask instead? If we ever support physical hotplug on ARM I think we wouldn't want to allow interrupts to go to CPUs that aren't even present (but still possible).
Yes, cpu_present_mask might be better indeed.
Best regards,
Tomasz
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
In reply to Tomasz FigaQuoted 4 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU, for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible CPU if no online CPU can be found in requested CPU mask.
Err, this is a bad idea. If a CPU is offline, then it must not respond to interrupts. If you bind an interrupt to an offline CPU, and that device asserts its interrupt, what happens? It doesn't get serviced until that CPU comes back online, which may be a very long time.
If, for example, that is your network device, it would mean your network stops operating. Worse, the network layer will time out and reset the ethernet device, trying to get things working (which it won't.)
I think how I used to handle this case prior to genirq is that I fell back to any online CPU if the interrupt ended up only routed to offline CPUs, but when an offline CPU comes back, it could then be re-routed back to that CPU. In other words, the mask change was non-destructive.
I think with genirq, such mask changes are destructive.
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
Quoted 16 lines · On Tuesday 20 of August 2013 22:14:42 Russell King - ARM Linux wrote:
Quoted 4 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU, for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible CPU if no online CPU can be found in requested CPU mask.
Err, this is a bad idea. If a CPU is offline, then it must not respond to interrupts. If you bind an interrupt to an offline CPU, and that device asserts its interrupt, what happens? It doesn't get serviced until that CPU comes back online, which may be a very long time.
If, for example, that is your network device, it would mean your network stops operating. Worse, the network layer will time out and reset the ethernet device, trying to get things working (which it won't.)
Quoted 6 lines
I think how I used to handle this case prior to genirq is that I fell back to any online CPU if the interrupt ended up only routed to offline CPUs, but when an offline CPU comes back, it could then be re-routed back to that CPU. In other words, the mask change was non-destructive.
I think with genirq, such mask changes are destructive.
Yes, that's correct. Although if you _explicitly_ request the interrupt to be routed to an offline CPU (i.e. only offline CPUs have bits set in passed cpumask), is it an error?
There is at least one irqchip that does not check received cpumask for this (metag) and I don't see any documentation saying what should happen in this case in .set_affinity operation.
Still, if you have any better solution for the original problem (broken Exynos4210 local timers, due to failing irq_set_affinity()), then I'd appreciate it, as I don't like the one from this patch too much either.
Best regards,
Tomasz
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
Quoted 28 lines · Tomasz Figa
Quoted 16 lines · On Tuesday 20 of August 2013 22:14:42 Russell King - ARM Linux wrote:
Quoted 4 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU, for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible CPU if no online CPU can be found in requested CPU mask.
Err, this is a bad idea. If a CPU is offline, then it must not respond to interrupts. If you bind an interrupt to an offline CPU, and that device asserts its interrupt, what happens? It doesn't get serviced until that CPU comes back online, which may be a very long time.
If, for example, that is your network device, it would mean your network stops operating. Worse, the network layer will time out and reset the ethernet device, trying to get things working (which it won't.)
Quoted 6 lines
I think how I used to handle this case prior to genirq is that I fell back to any online CPU if the interrupt ended up only routed to offline CPUs, but when an offline CPU comes back, it could then be re-routed back to that CPU. In other words, the mask change was non-destructive.
I think with genirq, such mask changes are destructive.
Yes, that's correct. Although if you _explicitly_ request the interrupt to be routed to an offline CPU (i.e. only offline CPUs have bits set in passed cpumask), is it an error?
Quoted 8 lines
There is at least one irqchip that does not check received cpumask for this (metag) and I don't see any documentation saying what should happen in this case in .set_affinity operation.
Still, if you have any better solution for the original problem (broken Exynos4210 local timers, due to failing
irq_set_affinity()), then I'd appreciate it, as I don't like the one from this patch too much either.
One "solution" might be to change the irq affinity after the CPU is marked online via the hotplug notifier chain. For a short period of time the timer interrupt will go to a different CPU but I don't see how that is a problem.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
PATCH 1/1 irqchip: gic: Allow setting affinity to offline CPUs
[Copying Daniel]
Quoted 38 lines · On Tuesday 20 of August 2013 15:39:17 Stephen Boyd wrote:
Quoted 37 lines · Tomasz Figa
Quoted 18 lines · On Tuesday 20 of August 2013 22:14:42 Russell King - ARM Linux wrote:
Quoted 5 lines · Tomasz Figa
Sometimes it is necessary to fix interrupt affinity to an offline CPU,
for example in initialization of local timers. This patch modifies .set_affinity() operation of irq-gic driver to fall back to any possible CPU if no online CPU can be found in requested CPU mask.Err, this is a bad idea. If a CPU is offline, then it must not respond
to interrupts. If you bind an interrupt to an offline CPU, and that device asserts its interrupt, what happens? It doesn't get serviced until that CPU comes back online, which may be a very long time.If, for example, that is your network device, it would mean your network stops operating. Worse, the network layer will time out and reset the ethernet device, trying to get things working (which it won't.)
Quoted 8 lines
I think how I used to handle this case prior to genirq is that I fell back to any online CPU if the interrupt ended up only routed to offline
CPUs, but when an offline CPU comes back, it could then be re-routed back to that CPU. In other words, the mask change was non-destructive.I think with genirq, such mask changes are destructive.
Yes, that's correct. Although if you _explicitly_ request the interrupt to be routed to an offline CPU (i.e. only offline CPUs have bits set in passed cpumask), is it an error?
There is at least one irqchip that does not check received cpumask for this (metag) and I don't see any documentation saying what should happen
in this case in .set_affinity operation.
Quoted 8 lines
Quoted 3 lines
Still, if you have any better solution for the original problem (broken Exynos4210 local timers, due to failing
irq_set_affinity()), then I'd appreciate it, as I don't like the one from this patch too much either.One "solution" might be to change the irq affinity after the CPU is marked online via the hotplug notifier chain. For a short period of time the timer interrupt will go to a different CPU but I don't see how that is a problem.
After initial testing, this seems to work, but but it still seems a little hackish.
I'd like to make sure that nothing bad happens if the irq somehow fires before setting the affinity. An opinion of someone that is more into kernel timekeeping than me would be nice.
Best regards,
Tomasz