On Mon, 12 Jun 2017 14:25:24 +1000 Benjamin Herrenschmidt <b...@au1.ibm.com> wrote:
> On Sun, 2017-06-11 at 19:30 +1000, Nicholas Piggin wrote: > > I rebased this on the powerpc next tree. > > > > A couple of things are changed since last post: > > > > - Patch 1 now properly accounts for the fact the powernv idle > > wakeups do not re-enable interrupts until the cpuidle driver > > enables them. This was not quite right in the previous patch > > (and prep_irq_for_idle() is not quite right for that case so > > a new primitive has to be introduced). > > What do you mean ? We shouldn't be going to sleep with the CPU thinking > it's interrupts are off, otherwise we end up effectively "taking an > interrupt while off" which is not right and it will cause accounting to > think we are off for too long. > > Is this a generic cpuidle problem or a powerpc issue ? cpuidle drivers enter their idle state with local_irq_disable(). powernv cpuidle driver currently does not call trace_hardirqs_on() before going to sleep (e.g., it does not use prep_irq_for_idle()). I did a previous patch that uses prep_irq_for_idle directly, but that assumes when we return from idle that local irqs should be on. The generic cpuidle does not want that, I haven't dug into exactly why not. But it seems to work better just to put the SRR1 wakeup reason into the irq_pending bit and let the lazy irq logic take care of the rest. > I'd rather we don't have to of those "prep_for_idle...". If necessary > sync the other one. Okay one can call the other rather than implementing twice. Thanks, Nick
