Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

[Peter Zijlstra]

On Tue, Jun 12, 2018 at 07:02:57PM +0200, Daniel Lezcano wrote:
> Mmh, it is unclear for me if the park() vs wakeup() can happen at the
> same time.
> 
> If the park() function is called, that means the hotplug is allowed.

No, it means we're inside hot-un-plug, but that doesn't stop the hrtimer
from firing.

> If the hotplug is allowed, we can modify the online mask.
> 
> What happens with the online mask when we are processing it in an
> interrupt context ?

RCU-like, if you observe a CPU in the online mask, it will stay
available, but the bit might get cleared.

> > Maybe avoid the issue entire by having a
> > {period,idle} tuple, where your old run := period - idle.
> 
> Can you elaborate ? I don't get it.

Have a period parameter that specifies the interval in which you have
one injected idle, and specify for how long you want to inject idle;
then obviously idle < period.

> >>> Furthermore, should you not be using hrtimer_forward(&timer,
> >>> idle_duration + run_duration) instead? AFAICT the current scheme is
> >>> prone to drifting.
> >>
> >> (I assume you meant setting the timer in the wakeup task function).
> >>
> >> Yes, drifting is not an issue if that happens. This scheme is simpler
> >> and safer than setting the timer ahead before waking up the tasks with
> >> the risk it expires before all the tasks ended their idle cycles.
> > 
> > sloppy though..
> 
> Ok, do you prefer to see the timer set in the wakeup function and thus
> having a periodic tick for the idle injection ?

I think having a HRTIMER_RESTART handler that does hrtimer_forward() is
the most sensible. You will end up having to deal with threads not being
ready, but I think that's not a real problem.