Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH] hrtimer: avoid retrigger_next_event IPI | Date | Fri, 09 Apr 2021 16:15:13 +0200 |
| |
On Wed, Apr 07 2021 at 10:53, Marcelo Tosatti wrote: > Setting the realtime clock triggers an IPI to all CPUs to reprogram > hrtimers. > > However, only base, boottime and tai clocks have their offsets updated
base clock? And why boottime? Boottime is not affected by a clock realtime set. It's clock REALTIME and TAI, nothing else.
> +#define CLOCK_SET_BASES ((1U << HRTIMER_BASE_REALTIME)| \ > + (1U << HRTIMER_BASE_REALTIME_SOFT)| \ > + (1U << HRTIMER_BASE_BOOTTIME)| \ > + (1U << HRTIMER_BASE_BOOTTIME_SOFT)| \ > + (1U << HRTIMER_BASE_TAI)| \ > + (1U << HRTIMER_BASE_TAI_SOFT)) > + > +static bool need_reprogram_timer(struct hrtimer_cpu_base *cpu_base) > +{ > + unsigned int active = 0; > + > + if (!cpu_base->softirq_activated) > + active = cpu_base->active_bases & HRTIMER_ACTIVE_SOFT; > + > + active = active | (cpu_base->active_bases & HRTIMER_ACTIVE_HARD); > + > + if ((active & CLOCK_SET_BASES) == 0) > + return false; > + > + return true; > +}
Errm.
> + /* Avoid interrupting nohz_full CPUs if possible */ > + preempt_disable(); > + for_each_online_cpu(cpu) { > + if (tick_nohz_full_cpu(cpu)) { > + unsigned long flags; > + struct hrtimer_cpu_base *cpu_base = &per_cpu(hrtimer_bases, cpu); > + > + raw_spin_lock_irqsave(&cpu_base->lock, flags); > + if (need_reprogram_timer(cpu_base)) > + cpumask_set_cpu(cpu, mask); > + else > + hrtimer_update_base(cpu_base); > + raw_spin_unlock_irqrestore(&cpu_base->lock, flags); > + } > + }
How is that supposed to be correct?
CPU0 CPU1
clock_was_set() hrtimer_start(CLOCK_REALTIME)
if (!active_mask[CPU1] & XXX) continue; active_mask |= REALTIME;
---> fail because that newly started timer is on the old offset.
Thanks,
tglx
| |