Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch 7/8] hrtimer: Avoid unnecessary SMP function calls in clock_was_set() | Date | Fri, 14 May 2021 20:52:33 +0200 |
| |
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.
Aside of that any CPU which has an affected timer will get notified even on NOHZ_FULL.
>> + goto out_timerfd; >> + } >> + >> + /* Avoid interrupting CPUs if possible */ >> + cpus_read_lock(); >> + for_each_online_cpu(cpu) { >> + struct hrtimer_cpu_base *cpu_base = &per_cpu(hrtimer_bases, cpu); >> + unsigned long flags; >> + >> + raw_spin_lock_irqsave(&cpu_base->lock, flags); >> + /* >> + * Only send the IPI when there are timers queued in one of >> + * the affected clock bases. Otherwise update the base >> + * remote to ensure that the next enqueue of a timer on >> + * such a clock base will see the correct offsets. >> + */ >> + if (cpu_base->active_bases & bases) >> + cpumask_set_cpu(cpu, mask); >> + else >> + hrtimer_update_base(cpu_base); >> + raw_spin_unlock_irqrestore(&cpu_base->lock, flags); >> + } >> + >> + 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.
Thanks,
tglx
| |