Re: [PATCH v9 03/32] tick-sched: Warn when next tick seems to be in the past

Frederic Weisbecker
2023-12-20T13:27:46Z

Le Fri, Dec 01, 2023 at 10:26:25AM +0100, Anna-Maria Behnsen a écrit :
> When the next tick is in the past, the delta between basemono and the next
> tick gets negativ. But the next tick should never be in the past. The
> negative effect of a wrong next tick might be a stop of the tick and timers
> might expire late.
>
> To prevent expensive debugging when changing underlying code, add a
> WARN_ON_ONCE into this code path. To prevent complete misbehaviour, also
> reset next_tick to basemono in this case.
>
> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
> ---
> v9: Add reset of next_tick to basemono
> ---
> kernel/time/tick-sched.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
> index 89517cfb6510..b1b591de781e 100644
> --- a/kernel/time/tick-sched.c
> +++ b/kernel/time/tick-sched.c
> @@ -839,6 +839,10 @@ static ktime_t tick_nohz_next_event(struct tick_sched *ts, int cpu)
> ts->next_timer = next_tick;
> }
>
> + /* Make sure next_tick is never before basemono! */
> + if (WARN_ON_ONCE(basemono > next_tick))
> + next_tick = basemono;
> +

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

And some food for thoughts:

1) Is it possible for hrtimer_get_next_event() to return values in the past?
2) Is hrtimer_get_next_event() unconditionally locking &cpu_base->lock even
high resolution is active? Can we avoid that?

Thanks.

> /*
> * If the tick is due in the next period, keep it ticking or
> * force prod the timer.
> --
> 2.39.2
>