Messages in this thread | | | Date | Tue, 12 Jun 2018 16:06:54 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework |
| |
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().
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?
Furthermore, should you not be using hrtimer_forward(&timer, idle_duration + run_duration) instead? AFAICT the current scheme is prone to drifting.
| |