Re: [PATCH 00/14 v2] idle performance improvements

On Mon, 12 Jun 2017 14:25:24 +1000
Benjamin Herrenschmidt [off-list ref] wrote:

On Sun, 2017-06-11 at 19:30 +1000, Nicholas Piggin wrote:

quoted

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