Re: [patch 7/8] hrtimer: Avoid unnecessary SMP function calls in clock_was_set()

Thomas Gleixner
2021-05-14T18:52:40Z

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