Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch 7/8] hrtimer: Avoid unnecessary SMP function calls in clock_was_set() | Date | Sat, 15 May 2021 02:24:55 +0200 |
| |
On Sat, May 15 2021 at 01:28, Peter Zijlstra wrote:
> On Fri, May 14, 2021 at 08:52:33PM +0200, Thomas Gleixner wrote: >> On Thu, May 13 2021 at 16:59, Peter Zijlstra wrote: >> > On Tue, Apr 27, 2021 at 10:25:44AM +0200, Thomas Gleixner wrote: >> >> - /* Retrigger the CPU local events everywhere */ >> >> - on_each_cpu(retrigger_next_event, NULL, 1); >> >> + if (!zalloc_cpumask_var(&mask, GFP_KERNEL)) { >> >> + on_each_cpu(retrigger_next_event, NULL, 1); >> > >> > This will violate NOHZ_FULL; >> >> Only if that allocation fails. > > Right, which should be near to never I suppose. > >> Aside of that any CPU which has an affected timer will get notified even >> on NOHZ_FULL. > > Right; but if it's properly NOHZ_FULL -- the kind that wanted a signal > on any entry into the kernel -- when it won't have timers and this IPI > will trigger the signal and kill the program.
That's true today. clock_was_set() IPI's unconditionally. The whole point of this exercise is to avoid that if there are no armed timers on the affected clocks.
>> >> + preempt_disable(); >> >> + smp_call_function_many(mask, retrigger_next_event, NULL, 1); >> > >> > The sane option is: >> > >> > smp_call_function_many_cond(cpu_online_mask, retrigger_next_event, >> > NULL, SCF_WAIT, update_needs_ipi); >> > >> > Which does all of the above, but better. >> >> With the difference that the for_each_cpu() loop runs with preemption >> disabled, while with this approach preemption is only disabled accross >> the function call. > > Yeah, I'd forgotten that... I might put looking at that on the todo list > somewhere :/
That's this append only thingy, right?
Thanks,
tglx
| |