Re: [PATCH] fix the issue that the tick_nohz_get_sleep_length() function could return a negative value
On Wed, Jan 20, 2021 at 11:49:38PM +, 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 > --- > --- 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(>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)" Date: Wed, 20 Jan 2021 23:49:38 + 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 Signed-off-by: Frederic Weisbecker --- 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(>sched_timer)); + if (unlikely(next_event < now)) + next_event = now; + return ktime_sub(next_event, now); } -- 2.25.1
Re: [PATCH] fix the issue that the tick_nohz_get_sleep_length() function could return a negative value
On Wed, Jan 20, 2021 at 11:49:38PM +, 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(). Makes sense, queued, thanks! > > Signed-off-by: Ti Zhou > --- > --- 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(>sched_timer)); > > + if (unlikely(next_event < now)) > + next_event = now; > + > return ktime_sub(next_event, now); > } >
[PATCH] fix the issue that the tick_nohz_get_sleep_length() function could return a negative value
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 --- --- tip/kernel/time/tick-sched.c.orig 2021-01-20 05:34:25.151325912 -0400 +++ tip/kernel/time/tick-sched.c2021-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(>sched_timer)); + if (unlikely(next_event < now)) + next_event = now; + return ktime_sub(next_event, now); }
Re: [PATCH] fix the issue that the tick_nohz_get_sleep_length() function could return a negative value
On 1/20/21 10:58 AM, Zhou Ti (x2019cwm) wrote: > Fix the issue that the tick_nohz_get_sleep_length() function could return > a negative value. > > The variable "next_event" has a small possibility to be smaller than the > variable "now" during running. Since both the menu idle governor and the > teo idle governor use u64 to store the return value of the function, this > may result in an extremely large and invalid value. > > One can easily detect the existence of this issue by using printk to > output a warning. > > Signed-off-by: Ti Zhou > --- > --- tip/kernel/time/tick-sched.c.orig 2021-01-20 05:34:25.151325912 -0400 > +++ tip/kernel/time/tick-sched.c2021-01-20 07:09:15.060980886 -0400 > @@ -1156,6 +1156,9 @@ ktime_t tick_nohz_get_sleep_length(ktime > next_event = min_t(u64, next_event, >hrtimer_next_event_without(>sched_timer)); > > + if (unlikely(next_event < now)) > + next_event = now; > + > return ktime_sub(next_event, now); > } > Hi, You need to send this to some maintainer who can apply/merge it. Just sending it to a mailing list is not sufficient. thanks. -- ~Randy
[PATCH] fix the issue that the tick_nohz_get_sleep_length() function could return a negative value
Fix the issue that the tick_nohz_get_sleep_length() function could return a negative value. The variable "next_event" has a small possibility to be smaller than the variable "now" during running. Since both the menu idle governor and the teo idle governor use u64 to store the return value of the function, this may result in an extremely large and invalid value. One can easily detect the existence of this issue by using printk to output a warning. Signed-off-by: Ti Zhou --- --- tip/kernel/time/tick-sched.c.orig 2021-01-20 05:34:25.151325912 -0400 +++ tip/kernel/time/tick-sched.c2021-01-20 07:09:15.060980886 -0400 @@ -1156,6 +1156,9 @@ ktime_t tick_nohz_get_sleep_length(ktime next_event = min_t(u64, next_event, hrtimer_next_event_without(>sched_timer)); + if (unlikely(next_event < now)) + next_event = now; + return ktime_sub(next_event, now); }
