Messages in this thread | | | Subject | Re: [PATCH V5] powercap/drivers/idle_injection: Add an idle injection framework | From | Daniel Lezcano <> | Date | Thu, 7 Jun 2018 16:11:02 +0200 |
| |
On 06/06/2018 12:45, Viresh Kumar wrote: > On 06-06-18, 12:22, Daniel Lezcano wrote: >> (mb() are done in the atomic operations AFAICT). > > AFAIU, it is required to make sure the operations are seen in a particular order > on another CPU and the compiler doesn't reorganize code to optimize it. > > For example, in our case what if the compiler reorganizes the atomic-set > operation after wakeup-process ? But maybe that wouldn't happen across function > calls and we should be safe then. > >> What about: >> >> get_online_cpus(); >> >> nr_tasks = cpumask_weight( >> cpumask_and(ii_dev->cpumask, cpu_online_mask); >> >> atomic_set(&ii_dev->count, nr_tasks); >> >> 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); >> } >> >> put_online_cpus(); >> ? > > Looks good this time. > >> I'm wondering if we can have a CPU hotplugged right after the >> 'put_online_cpus', resulting in the 'should park' flag set and then the >> thread goes in the kthread_parkme instead of jumping back the idle >> injection function and decrease the count, leading up to the timer not >> being set again. > > True. That looks like a valid problem to me as well. > > What about starting the hrtimer right from this routine itself, after taking > into account running-time, idle-time, delay, etc ? That would get rid of the > count stuff, this get_online_cpus(), etc. > > Even if we restart the next play-idle cycle a bit early or with some delay, sky > wouldn't fall :)
We won't be able to call completion() in this case.
-- <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
| |