On 21/11/14 10:41, Thomas Gleixner wrote:
On Fri, 21 Nov 2014, Daniel Thompson wrote:
On 20/11/14 23:30, Thomas Gleixner wrote:
On Thu, 20 Nov 2014, Daniel Thompson wrote:
+/*
- The PMU IRQ lines of all cores are muxed onto a single interrupt.
- Rotate the interrupt around the cores if the current CPU cannot
- figure out why the interrupt has been triggered.
- */
+static irqreturn_t imx6q_pmu_handler(int irq, void *dev, irq_handler_t handler) +{
- irqreturn_t ret = handler(irq, dev);
- int next;
- if (ret == IRQ_NONE && num_online_cpus() > 1) {
What guarantees that ret == IRQ_HANDLED is a sign for 'this is only for this particular core' interrupt ?
It isn't guaranteed. We rely on re-entering the interrupt handler if more than one PMU is raising the interrupt simultaneously.
next = cpumask_next(smp_processor_id(), cpu_online_mask);if (next > nr_cpu_ids)next = cpumask_next(-1, cpu_online_mask);irq_set_affinity(irq, cpumask_of(next));- }
Aside of the fact, that the hardware designers who came up with such a brainfart should be put on sane drugs, this is just silly.
Rotating that thing around will introduce arbitrary latencies and dependencies on other interrupts to be handled.
To be honest I viewed the only real merits of the rotation workaround to be that it is simple and minimally invasive. I am in total agreement that there are profiling use cases that it will handle badly (although there are a useful set which this workaround is sufficient to support).
So if there is really no way to figure out which of the cores is the actual target of the PMU interrupt
PMU is only accessible via the bus if you are a external debugger (signals external to the cluster control the register windowing). From the kernel we have to use the co-processor interface and can only see our own PMU.
then you should simply broadcast that interrupt to a designated per cpu vector async from the one which handles it in the first place and be done with it. That's the only sane option you have.
As it happens I was planning to do some work on rebroadcasting next anyway regardless of this discussion because I can't call irq_set_affinity() from a FIQ handler...
Options I considered to rebroadcast are either direct use of an (new and ARM specific?) IPI or use of smp_call_function() from a tasklet. I was
inclined to rule out the tasklet because it has the potential for far greater timing jitter than rotating the affinity (doesn't it?).
You cannot schedule a tasklet from FIQ.
Indeed, that was another black mark against it. However I would prefer to be able to convince people to rule out for performance reasons. My FIQ work for perf isn't mainlined at this point so using FIQ to show why I prefer one solution (for mainline) over another isn't entirely reasonable.
The only options you have are:
- Async IPI (direct call into the architecture code). I have no idea whether thats possible on ARM
I think this may be where I have to go eventually (sigh). If I can get to the stage where we can deliver PMU events via FIQ then ideally the workaround would also have to broadcast via FIQ.
Its a real shame that one of the best chips to do FIQ work suffers has such an annoying hardware bug.
- irq_work
I'm currently trying this approach. It makes sense today without FIQ and, fortunately for me, the workaround should mostly still work if it were deployed from a FIQ handler.
BTW thanks very much for the listing options here; I was a little worried I might have overlooked some really obvious way to deploy the workaround.
Daniel.