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 :) -- viresh