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

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, May 13 2021 at 09:47, Peter Zijlstra wrote:
> On Fri, Apr 30, 2021 at 09:12:15AM +0200, Thomas Gleixner wrote:
>> +	/*
>> +	 * If the remote CPU is currently handling an hrtimer interrupt, it
>> +	 * will reevaluate the first expiring timer of all clock bases
>> +	 * before reprogramming. Nothing to do here.
>> +	 */
>> +	if (cpu_base->in_hrtirq)
>> +		return false;
>
> This one gives me a head-ache though; if we get here, that means
> hrtimer_interrupt()'s hrtimer_update_base() happened before the change.
> It also means that CPU is in __run_hrtimer() running a fn(), since we
> own cpu_base->lock.
>
> That in turn means it is in __hrtimer_run_queues(), possible on the last
> base.
>
> Now, if I understand it right, the thing that saves us, is that
> hrtimer_update_next_event() -- right after returning from
> __hrtimer_run_queues() -- will re-evaluate all bases (with the
> hrtimer_update_base() we just did visible to it) and we'll eventually
> goto retry if time moved such that we now have timers that should've ran
> but were missed due to this concurrent shift in time.

Correct.

> However, since that retries thing is limited to 3; could we not trigger
> that by generating a stream of these updates, causing the timer to keep
> having to be reset? I suppose updating time is a root only thing, and
> root can shoot its own foot off any time it damn well likes, so who
> cares.

It's root only. Sou you could argue that a borked NTPd can cause this to
happen, but then you surely have other problems aside of hitting the
retries limit.

Thanks,

        tglx