Re: [PATCH] fix the issue that the tick_nohz_get_sleep_length() function could return a negative value

[Frederic Weisbecker]

On Wed, Jan 20, 2021 at 11:49:38PM +0000, Zhou Ti (x2019cwm) wrote:
> Fix the issue that the tick_nohz_get_sleep_length() function could return a 
> negative value.
> 
> The variable "dev->next_event" has a small possibility to be smaller than 
> the variable "now" during running, which would result in a negative value 
> of "*delta_next". The variable "next_event" also has a small posibility to 
> be smaller than the variable "now". Both case could lead to a negative 
> return of function tick_nohz_get_sleep_length().
> 
> Signed-off-by: Ti Zhou <x2019cwm@xxxxxxx>
> ---
> --- tip/kernel/time/tick-sched.c.orig	2021-01-20 05:34:25.151325912 -0400
> +++ tip/kernel/time/tick-sched.c	2021-01-20 19:44:28.238538380 -0400
> @@ -1142,6 +1142,9 @@ ktime_t tick_nohz_get_sleep_length(ktime
>  
>  	*delta_next = ktime_sub(dev->next_event, now);
>  
> +	if (unlikely(*delta_next < 0))
> +		*delta_next = 0;
> +
>  	if (!can_stop_idle_tick(cpu, ts))
>  		return *delta_next;
>  
> @@ -1156,6 +1159,9 @@ ktime_t tick_nohz_get_sleep_length(ktime
>  	next_event = min_t(u64, next_event,
>  			   hrtimer_next_event_without(&ts->sched_timer));
>  
> +	if (unlikely(next_event < now))
> +		next_event = now;
> +
>  	return ktime_sub(next_event, now);
>  }
>  

I have reworked the changelog that way and queued. I'll post a
series including it after the merge window.

Thanks!

---
From: "Zhou Ti (x2019cwm)" <x2019cwm@xxxxxxx>
Date: Wed, 20 Jan 2021 23:49:38 +0000
Subject: [PATCH] tick/nohz: Prevent tick_nohz_get_sleep_length() from
 returning negative value

If the hardware clock happens to fire its interrupts late, two possible
issues can happen while calling tick_nohz_get_sleep_length(). Either:

1) The next clockevent device event is due past the last idle entry time.

or:

2) The last timekeeping update happened before the last idle entry time
   and the next timer callback expires past the last idle entry time.

Make sure that both cases are handled to avoid returning a negative
duration to the cpuidle governors.

Signed-off-by: Ti Zhou <x2019cwm@xxxxxxx>
Signed-off-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
---
 kernel/time/tick-sched.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index e10a4af88737..22b6a46818cf 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1142,6 +1142,9 @@ ktime_t tick_nohz_get_sleep_length(ktime_t *delta_next)
 
 	*delta_next = ktime_sub(dev->next_event, now);
 
+	if (unlikely(*delta_next < 0))
+		*delta_next = 0;
+
 	if (!can_stop_idle_tick(cpu, ts))
 		return *delta_next;
 
@@ -1156,6 +1159,9 @@ ktime_t tick_nohz_get_sleep_length(ktime_t *delta_next)
 	next_event = min_t(u64, next_event,
 			   hrtimer_next_event_without(&ts->sched_timer));
 
+	if (unlikely(next_event < now))
+		next_event = now;
+
 	return ktime_sub(next_event, now);
 }
 
-- 
2.25.1