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 <[email protected]>
---
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;
+
/*
* If the tick is due in the next period, keep it ticking or
* force prod the timer.
--
2.39.2
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 <[email protected]>
> ---
> 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 <[email protected]>
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
>
The following commit has been merged into the timers/core branch of tip:
Commit-ID: cbf04a22026100dceeceec67fcbf1973383eb32f
Gitweb: https://git.kernel.org/tip/cbf04a22026100dceeceec67fcbf1973383eb32f
Author: Anna-Maria Behnsen <[email protected]>
AuthorDate: Fri, 01 Dec 2023 10:26:25 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 20 Dec 2023 16:49:38 +01:00
tick-sched: Warn when next tick seems to be in the past
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 <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Frederic Weisbecker <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
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 fce3c6f..a17d260 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.