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

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 12/06/2018 16:06, Peter Zijlstra wrote:
> On Tue, Jun 12, 2018 at 03:02:14PM +0200, Daniel Lezcano wrote:
>> On 12/06/2018 14:52, Peter Zijlstra wrote:
>>> In this case, you can do:
>>
>> That is what we had before but we change the code to set the count
>> before waking up the task, so compute the cpumask_weight of the
>> resulting AND right before this loop.
>>
>>> +       for_each_cpu_and(cpu, &ii_dev->cpumask, cpu_online_mask) {
>>> +               iit = per_cpu_ptr(&idle_injection_thread, cpu);
>>> +               iit->should_run = 1;
>>> +               wake_up_process(iit->tsk);
>>> +       }
> 
> 
> Ah, I see, but since you do:
> 
> 	if (atomic_dec_and_test())
> 	  last_man()
> 
> where that last_man() thing will start a timer, there is no real problem
> with doing atomic_inc() with before wake_up_process().

Viresh was worried about the scenario:

https://lkml.org/lkml/2018/6/5/276


> Yes, it allows doing last_man, too often, but repeated hrtimer_start()
> will DTRT and reprogram the timer.
> 
> Also, last_man() uses @run_duration, but the way I read it, the timer is
> for waking things up again, this means it is in fact the sleep duration,
> no?

No, it is the next idle injection deadline, meanwhile we let the system
continue running.

The sleep duration is managed by another timer in play_idle().

> 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.



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog