diff mbox series

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

Message ID 20231201092654.34614-4-anna-maria@linutronix.de
State New
Headers
Series timers: Move from a push remote at enqueue to a pull at expiry model |

Commit Message

Anna-Maria Behnsen Dec. 1, 2023, 9:26 a.m. UTC 
  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(+)

Comments

Frederic Weisbecker Dec. 20, 2023, 1:27 p.m. UTC | #1 
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
>
diff mbox series

Patch

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;
+
 	/*
 	 * If the tick is due in the next period, keep it ticking or
 	 * force prod the timer.