On 12/06/2018 14:52, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 02:44:29PM +0200, Daniel Lezcano wrote: >> On 12/06/2018 14:30, Peter Zijlstra wrote: >>> On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: >>>> +static void __idle_injection_wakeup(struct idle_injection_device *ii_dev) >>>> +{ >>>> + struct idle_injection_thread *iit; >>>> + struct cpumask tmp; >>>> + unsigned int cpu; >>>> + >>>> + cpumask_and(&tmp, ii_dev->cpumask, cpu_online_mask); >>> >>> You should not be having a cpumask on the stack. Those things can be >>> ginormous. >> >> Ok, the kernel code uses of cpumask_t on the stack when dealing with >> cpumask_and. I assume it is also not recommended. > > Yes, that should all get fixed. It's mostly legacy code I suppose. It's > been at least 10 years I think since we merged the whole > CPUMASK_OFFSTACK stuff. > >> What would be the best practice ? Allocate a per cpumask at init time as >> a temporary mask to work with ? > > 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); > + } > -- <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