[PATCH v1 0/5] cpuidle: Take possible negative "sleep length" values into account

[PATCH v1 0/5] cpuidle: Take possible negative "sleep length" values into account

[Rafael J. Wysocki]

Hi All,

As follows from the discussion triggered by the patch at

https://lore.kernel.org/lkml/20210311123708.23501-2-frederic@kernel.org/

the cpuidle governors using tick_nohz_get_sleep_length() assume it to always
return positive values which is not correct in general.

To address this issues, first document the fact that negative values can
be returned by tick_nohz_get_sleep_length() (patch [1/5]).  Then, in
preparation for more substantial changes, change the data type of two
fields in struct cpuidle_state to s64 so they can be used in computations
involving negative numbers safely (patch [2/5]).

Next, adjust the teo governor a bit so that negative "sleep length" values
are counted like zero by it (patch [3/5]) and modify it so as to avoid
mishandling negative "sleep length" values (patch [4/5]).

Finally, make the menu governor take negative "sleep length" values into
account properly (patch [5/5]).

Please see the changelogs of the patches for details.

Thanks!

[PATCH v1 1/5] tick/nohz: Improve tick_nohz_get_next_hrtimer() kerneldoc

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Make the tick_nohz_get_next_hrtimer() kerneldoc comment state clearly
that the function may return negative numbers.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 kernel/time/tick-sched.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux-pm/kernel/time/tick-sched.c
===================================================================
--- linux-pm.orig/kernel/time/tick-sched.c
+++ linux-pm/kernel/time/tick-sched.c
@@ -1124,7 +1124,11 @@ ktime_t tick_nohz_get_next_hrtimer(void)
  * tick_nohz_get_sleep_length - return the expected length of the current sleep
  * @delta_next: duration until the next event if the tick cannot be stopped
  *
- * Called from power state control code with interrupts disabled
+ * Called from power state control code with interrupts disabled.
+ *
+ * The return value of this function and/or the value returned by it through the
+ * @delta_next pointer can be negative which must be taken into account by its
+ * callers.
  */
 ktime_t tick_nohz_get_sleep_length(ktime_t *delta_next)
 {

[PATCH v1 2/5] cpuidle: Use s64 as exit_latency_ns and target_residency_ns data type

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Subsequent changes will cause the exit_latency_ns and target_residency_ns
fields in struct cpuidle_state to be used in computations in which data
type conversions to u64 may turn a negative number close to zero into
a verly large positive number leading to incorrect results.

In preparation for that, change the data type of the fields mentioned
above to s64, but ensure that they will not be negative themselves.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/driver.c |    4 ++++
 include/linux/cpuidle.h  |    4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

Index: linux-pm/include/linux/cpuidle.h
===================================================================
--- linux-pm.orig/include/linux/cpuidle.h
+++ linux-pm/include/linux/cpuidle.h
@@ -49,8 +49,8 @@ struct cpuidle_state {
 	char		name[CPUIDLE_NAME_LEN];
 	char		desc[CPUIDLE_DESC_LEN];
 
-	u64		exit_latency_ns;
-	u64		target_residency_ns;
+	s64		exit_latency_ns;
+	s64		target_residency_ns;
 	unsigned int	flags;
 	unsigned int	exit_latency; /* in US */
 	int		power_usage; /* in mW */
Index: linux-pm/drivers/cpuidle/driver.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/driver.c
+++ linux-pm/drivers/cpuidle/driver.c
@@ -181,9 +181,13 @@ static void __cpuidle_driver_init(struct
 		 */
 		if (s->target_residency > 0)
 			s->target_residency_ns = s->target_residency * NSEC_PER_USEC;
+		else if (s->target_residency_ns < 0)
+			s->target_residency_ns = 0;
 
 		if (s->exit_latency > 0)
 			s->exit_latency_ns = s->exit_latency * NSEC_PER_USEC;
+		else if (s->exit_latency_ns < 0)
+			s->exit_latency_ns =  0;
 	}
 }
 

[PATCH v1 3/5] cpuidle: teo: Adjust handling of very short idle times

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

If the time till the next timer event is shorter than the target
residency of the first idle state (state 0), the TEO governor does
not update its metrics for any idle states, but arguably it should
record a "hit" for idle state 0 in that case, so modify it to do
that.

Accordingly, also make it record an "early hit" for idle state 0 if
the measured idle duration is less than its target residency, which
allows one branch more to be dropped from teo_update().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/governors/teo.c |   32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

Index: linux-pm/drivers/cpuidle/governors/teo.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/teo.c
+++ linux-pm/drivers/cpuidle/governors/teo.c
@@ -117,7 +117,8 @@ static DEFINE_PER_CPU(struct teo_cpu, te
 static void teo_update(struct cpuidle_driver *drv, struct cpuidle_device *dev)
 {
 	struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
-	int i, idx_hit = -1, idx_timer = -1;
+	int i, idx_hit = 0, idx_timer = 0;
+	unsigned int hits, misses;
 	u64 measured_ns;
 
 	if (cpu_data->time_span_ns >= cpu_data->sleep_length_ns) {
@@ -174,25 +175,22 @@ static void teo_update(struct cpuidle_dr
 	 * also increase the "early hits" metric for the state that actually
 	 * matches the measured idle duration.
 	 */
-	if (idx_timer >= 0) {
-		unsigned int hits = cpu_data->states[idx_timer].hits;
-		unsigned int misses = cpu_data->states[idx_timer].misses;
-
-		hits -= hits >> DECAY_SHIFT;
-		misses -= misses >> DECAY_SHIFT;
-
-		if (idx_timer > idx_hit) {
-			misses += PULSE;
-			if (idx_hit >= 0)
-				cpu_data->states[idx_hit].early_hits += PULSE;
-		} else {
-			hits += PULSE;
-		}
+	hits = cpu_data->states[idx_timer].hits;
+	hits -= hits >> DECAY_SHIFT;
+
+	misses = cpu_data->states[idx_timer].misses;
+	misses -= misses >> DECAY_SHIFT;
 
-		cpu_data->states[idx_timer].misses = misses;
-		cpu_data->states[idx_timer].hits = hits;
+	if (idx_timer == idx_hit) {
+		hits += PULSE;
+	} else {
+		misses += PULSE;
+		cpu_data->states[idx_hit].early_hits += PULSE;
 	}
 
+	cpu_data->states[idx_timer].misses = misses;
+	cpu_data->states[idx_timer].hits = hits;
+
 	/*
 	 * Save idle duration values corresponding to non-timer wakeups for
 	 * pattern detection.

[PATCH v1 4/5] cpuidle: teo: Take negative "sleep length" values into account

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Modify the TEO governor to take possible negative return values of
tick_nohz_get_next_hrtimer() into account by changing the data type
of some variables used by it to s64 which allows it to carry out
computations without potentially problematic data type conversions
into u64.

Also change the computations in teo_select() so that the negative
values themselves are handled in a natural way to avoid adding extra
negative value checks to that function.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/governors/teo.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

Index: linux-pm/drivers/cpuidle/governors/teo.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/teo.c
+++ linux-pm/drivers/cpuidle/governors/teo.c
@@ -100,8 +100,8 @@ struct teo_idle_state {
  * @intervals: Saved idle duration values.
  */
 struct teo_cpu {
-	u64 time_span_ns;
-	u64 sleep_length_ns;
+	s64 time_span_ns;
+	s64 sleep_length_ns;
 	struct teo_idle_state states[CPUIDLE_STATE_MAX];
 	int interval_idx;
 	u64 intervals[INTERVALS];
@@ -214,7 +214,7 @@ static bool teo_time_ok(u64 interval_ns)
  */
 static int teo_find_shallower_state(struct cpuidle_driver *drv,
 				    struct cpuidle_device *dev, int state_idx,
-				    u64 duration_ns)
+				    s64 duration_ns)
 {
 	int i;
 
@@ -240,10 +240,10 @@ static int teo_select(struct cpuidle_dri
 {
 	struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
 	s64 latency_req = cpuidle_governor_latency_req(dev->cpu);
-	u64 duration_ns;
+	int max_early_idx, prev_max_early_idx, constraint_idx, idx0, idx, i;
 	unsigned int hits, misses, early_hits;
-	int max_early_idx, prev_max_early_idx, constraint_idx, idx, i;
 	ktime_t delta_tick;
+	s64 duration_ns;
 
 	if (dev->last_state_idx >= 0) {
 		teo_update(drv, dev);
@@ -262,6 +262,7 @@ static int teo_select(struct cpuidle_dri
 	prev_max_early_idx = -1;
 	constraint_idx = drv->state_count;
 	idx = -1;
+	idx0 = idx;
 
 	for (i = 0; i < drv->state_count; i++) {
 		struct cpuidle_state *s = &drv->states[i];
@@ -322,6 +323,7 @@ static int teo_select(struct cpuidle_dri
 			idx = i; /* first enabled state */
 			hits = cpu_data->states[i].hits;
 			misses = cpu_data->states[i].misses;
+			idx0 = i;
 		}
 
 		if (s->target_residency_ns > duration_ns)
@@ -374,11 +376,16 @@ static int teo_select(struct cpuidle_dri
 
 	if (idx < 0) {
 		idx = 0; /* No states enabled. Must use 0. */
-	} else if (idx > 0) {
+	} else if (idx > idx0) {
 		unsigned int count = 0;
 		u64 sum = 0;
 
 		/*
+		 * The target residencies of at least two different enabled idle
+		 * states are less than or equal to the current expected idle
+		 * duration.  Try to refine the selection using the most recent
+		 * measured idle duration values.
+		 *
 		 * Count and sum the most recent idle duration values less than
 		 * the current expected idle duration value.
 		 */
@@ -426,7 +433,8 @@ static int teo_select(struct cpuidle_dri
 		 * till the closest timer including the tick, try to correct
 		 * that.
 		 */
-		if (idx > 0 && drv->states[idx].target_residency_ns > delta_tick)
+		if (idx > idx0 &&
+		    drv->states[idx].target_residency_ns > delta_tick)
 			idx = teo_find_shallower_state(drv, dev, idx, delta_tick);
 	}
 

[PATCH v1 5/5] cpuidle: menu: Take negative "sleep length" values into account

[Rafael J. Wysocki]

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Subject: [PATCH] cpuidle: menu: Take negative "sleep length" values into account

Make the menu governor check the tick_nohz_get_next_hrtimer()
return value so as to avoid dealing with negative "sleep length"
values and make it use that value directly when the tick is stopped.

While at it, rename local variable delta_next in menu_select() to
delta_tick which better reflects its purpose.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/governors/menu.c |   17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/cpuidle/governors/menu.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/menu.c
+++ linux-pm/drivers/cpuidle/governors/menu.c
@@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_dr
 	u64 predicted_ns;
 	u64 interactivity_req;
 	unsigned long nr_iowaiters;
-	ktime_t delta_next;
+	ktime_t delta, delta_tick;
 	int i, idx;
 
 	if (data->needs_update) {
@@ -280,7 +280,12 @@ static int menu_select(struct cpuidle_dr
 	}
 
 	/* determine the expected residency time, round up */
-	data->next_timer_ns = tick_nohz_get_sleep_length(&delta_next);
+	delta = tick_nohz_get_sleep_length(&delta_tick);
+	if (unlikely(delta < 0)) {
+		delta = 0;
+		delta_tick = 0;
+	}
+	data->next_timer_ns = delta;
 
 	nr_iowaiters = nr_iowait_cpu(dev->cpu);
 	data->bucket = which_bucket(data->next_timer_ns, nr_iowaiters);
@@ -318,7 +323,7 @@ static int menu_select(struct cpuidle_dr
 		 * state selection.
 		 */
 		if (predicted_ns < TICK_NSEC)
-			predicted_ns = delta_next;
+			predicted_ns = data->next_timer_ns;
 	} else {
 		/*
 		 * Use the performance multiplier and the user-configurable
@@ -377,7 +382,7 @@ static int menu_select(struct cpuidle_dr
 			 * stuck in the shallow one for too long.
 			 */
 			if (drv->states[idx].target_residency_ns < TICK_NSEC &&
-			    s->target_residency_ns <= delta_next)
+			    s->target_residency_ns <= delta_tick)
 				idx = i;
 
 			return idx;
@@ -399,7 +404,7 @@ static int menu_select(struct cpuidle_dr
 	     predicted_ns < TICK_NSEC) && !tick_nohz_tick_stopped()) {
 		*stop_tick = false;
 
-		if (idx > 0 && drv->states[idx].target_residency_ns > delta_next) {
+		if (idx > 0 && drv->states[idx].target_residency_ns > delta_tick) {
 			/*
 			 * The tick is not going to be stopped and the target
 			 * residency of the state to be returned is not within
@@ -411,7 +416,7 @@ static int menu_select(struct cpuidle_dr
 					continue;
 
 				idx = i;
-				if (drv->states[i].target_residency_ns <= delta_next)
+				if (drv->states[i].target_residency_ns <= delta_tick)
 					break;
 			}
 		}

Re: [PATCH v1 5/5] cpuidle: menu: Take negative "sleep length" values into account

[Zhou Ti (x2019cwm)]

On Mon 2021-03-29 14:37 Rafael J. Wysocki wrote:
> Make the menu governor check the tick_nohz_get_next_hrtimer()
> return value so as to avoid dealing with negative "sleep length"
> values and make it use that value directly when the tick is stopped.
> 
> While at it, rename local variable delta_next in menu_select() to
> delta_tick which better reflects its purpose.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpuidle/governors/menu.c |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> Index: linux-pm/drivers/cpuidle/governors/menu.c
> ===================================================================
> --- linux-pm.orig/drivers/cpuidle/governors/menu.c
> +++ linux-pm/drivers/cpuidle/governors/menu.c
> @@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_dr
>          u64 predicted_ns;
>          u64 interactivity_req;
>          unsigned long nr_iowaiters;
> -       ktime_t delta_next;
> +       ktime_t delta, delta_tick;
>          int i, idx;
>  
>          if (data->needs_update) {
> @@ -280,7 +280,12 @@ static int menu_select(struct cpuidle_dr
>          }
>  
>          /* determine the expected residency time, round up */
> -       data->next_timer_ns = tick_nohz_get_sleep_length(&delta_next);
> +       delta = tick_nohz_get_sleep_length(&delta_tick);
> +       if (unlikely(delta < 0)) {
> +               delta = 0;
> +               delta_tick = 0;
> +       }
> +       data->next_timer_ns = delta;
>  
>          nr_iowaiters = nr_iowait_cpu(dev->cpu);
>          data->bucket = which_bucket(data->next_timer_ns, nr_iowaiters);
> @@ -318,7 +323,7 @@ static int menu_select(struct cpuidle_dr
>                   * state selection.
>                   */
>                  if (predicted_ns < TICK_NSEC)
> -                       predicted_ns = delta_next;
> +                       predicted_ns = data->next_timer_ns;
>          } else {
>                  /*
>                   * Use the performance multiplier and the user-configurable
> @@ -377,7 +382,7 @@ static int menu_select(struct cpuidle_dr
>                           * stuck in the shallow one for too long.
>                           */
>                          if (drv->states[idx].target_residency_ns < TICK_NSEC &&
> -                           s->target_residency_ns <= delta_next)
> +                           s->target_residency_ns <= delta_tick)
>                                  idx = i;
>  
>                          return idx;
> @@ -399,7 +404,7 @@ static int menu_select(struct cpuidle_dr
>               predicted_ns < TICK_NSEC) && !tick_nohz_tick_stopped()) {
>                  *stop_tick = false;
>  
> -               if (idx > 0 && drv->states[idx].target_residency_ns > delta_next) {
> +               if (idx > 0 && drv->states[idx].target_residency_ns > delta_tick) {
>                          /*
>                           * The tick is not going to be stopped and the target
>                           * residency of the state to be returned is not within
> @@ -411,7 +416,7 @@ static int menu_select(struct cpuidle_dr
>                                          continue;
>  
>                                  idx = i;
> -                               if (drv->states[i].target_residency_ns <= delta_next)
> +                               if (drv->states[i].target_residency_ns <= delta_tick)
>                                          break;
>                          }
>                  }

How about this.
I think it's possible to avoid the new variable delta.

---

--- linux-pm/drivers/cpuidle/governors/menu.c.orig	2021-03-29 22:44:02.316971970 -0300
+++ linux-pm/drivers/cpuidle/governors/menu.c	2021-03-29 22:51:15.804377168 -0300
@@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_dr
 	u64 predicted_ns;
 	u64 interactivity_req;
 	unsigned long nr_iowaiters;
-	ktime_t delta_next;
+	ktime_t delta_tick;
 	int i, idx;
 
 	if (data->needs_update) {
@@ -280,7 +280,12 @@ static int menu_select(struct cpuidle_dr
 	}
 
 	/* determine the expected residency time, round up */
-	data->next_timer_ns = tick_nohz_get_sleep_length(&delta_next);
+	data->next_timer_ns = tick_nohz_get_sleep_length(&delta_tick);
+
+	if (unlikely(data->next_timer_ns >> 63)) {
+		data->next_timer_ns = 0;
+		delta_tick = 0;
+	}
 
 	nr_iowaiters = nr_iowait_cpu(dev->cpu);
 	data->bucket = which_bucket(data->next_timer_ns, nr_iowaiters);
@@ -318,7 +323,7 @@ static int menu_select(struct cpuidle_dr
 		 * state selection.
 		 */
 		if (predicted_ns < TICK_NSEC)
-			predicted_ns = delta_next;
+			predicted_ns = data->next_timer_ns;
 	} else {
 		/*
 		 * Use the performance multiplier and the user-configurable
@@ -377,7 +382,7 @@ static int menu_select(struct cpuidle_dr
 			 * stuck in the shallow one for too long.
 			 */
 			if (drv->states[idx].target_residency_ns < TICK_NSEC &&
-			    s->target_residency_ns <= delta_next)
+			    s->target_residency_ns <= delta_tick)
 				idx = i;
 
 			return idx;
@@ -399,7 +404,7 @@ static int menu_select(struct cpuidle_dr
 	     predicted_ns < TICK_NSEC) && !tick_nohz_tick_stopped()) {
 		*stop_tick = false;
 
-		if (idx > 0 && drv->states[idx].target_residency_ns > delta_next) {
+		if (idx > 0 && drv->states[idx].target_residency_ns > delta_tick) {
 			/*
 			 * The tick is not going to be stopped and the target
 			 * residency of the state to be returned is not within
@@ -411,7 +416,7 @@ static int menu_select(struct cpuidle_dr
 					continue;
 
 				idx = i;
-				if (drv->states[i].target_residency_ns <= delta_next)
+				if (drv->states[i].target_residency_ns <= delta_tick)
 					break;
 			}
 		}

Re: [PATCH v1 5/5] cpuidle: menu: Take negative "sleep length" values into account

[Rafael J. Wysocki]

On Tue, Mar 30, 2021 at 4:00 AM Zhou Ti (x2019cwm) <x2019cwm@stfx.ca> wrote:
>
> On Mon 2021-03-29 14:37 Rafael J. Wysocki wrote:
> > Make the menu governor check the tick_nohz_get_next_hrtimer()
> > return value so as to avoid dealing with negative "sleep length"
> > values and make it use that value directly when the tick is stopped.
> >
> > While at it, rename local variable delta_next in menu_select() to
> > delta_tick which better reflects its purpose.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/cpuidle/governors/menu.c |   17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > Index: linux-pm/drivers/cpuidle/governors/menu.c
> > ===================================================================
> > --- linux-pm.orig/drivers/cpuidle/governors/menu.c
> > +++ linux-pm/drivers/cpuidle/governors/menu.c
> > @@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_dr
> >          u64 predicted_ns;
> >          u64 interactivity_req;
> >          unsigned long nr_iowaiters;
> > -       ktime_t delta_next;
> > +       ktime_t delta, delta_tick;
> >          int i, idx;
> >
> >          if (data->needs_update) {
> > @@ -280,7 +280,12 @@ static int menu_select(struct cpuidle_dr
> >          }
> >
> >          /* determine the expected residency time, round up */
> > -       data->next_timer_ns = tick_nohz_get_sleep_length(&delta_next);
> > +       delta = tick_nohz_get_sleep_length(&delta_tick);
> > +       if (unlikely(delta < 0)) {
> > +               delta = 0;
> > +               delta_tick = 0;
> > +       }
> > +       data->next_timer_ns = delta;
> >
> >          nr_iowaiters = nr_iowait_cpu(dev->cpu);
> >          data->bucket = which_bucket(data->next_timer_ns, nr_iowaiters);
> > @@ -318,7 +323,7 @@ static int menu_select(struct cpuidle_dr
> >                   * state selection.
> >                   */
> >                  if (predicted_ns < TICK_NSEC)
> > -                       predicted_ns = delta_next;
> > +                       predicted_ns = data->next_timer_ns;
> >          } else {
> >                  /*
> >                   * Use the performance multiplier and the user-configurable
> > @@ -377,7 +382,7 @@ static int menu_select(struct cpuidle_dr
> >                           * stuck in the shallow one for too long.
> >                           */
> >                          if (drv->states[idx].target_residency_ns < TICK_NSEC &&
> > -                           s->target_residency_ns <= delta_next)
> > +                           s->target_residency_ns <= delta_tick)
> >                                  idx = i;
> >
> >                          return idx;
> > @@ -399,7 +404,7 @@ static int menu_select(struct cpuidle_dr
> >               predicted_ns < TICK_NSEC) && !tick_nohz_tick_stopped()) {
> >                  *stop_tick = false;
> >
> > -               if (idx > 0 && drv->states[idx].target_residency_ns > delta_next) {
> > +               if (idx > 0 && drv->states[idx].target_residency_ns > delta_tick) {
> >                          /*
> >                           * The tick is not going to be stopped and the target
> >                           * residency of the state to be returned is not within
> > @@ -411,7 +416,7 @@ static int menu_select(struct cpuidle_dr
> >                                          continue;
> >
> >                                  idx = i;
> > -                               if (drv->states[i].target_residency_ns <= delta_next)
> > +                               if (drv->states[i].target_residency_ns <= delta_tick)
> >                                          break;
> >                          }
> >                  }
>
> How about this.
> I think it's possible to avoid the new variable delta.
>
> ---
>
> --- linux-pm/drivers/cpuidle/governors/menu.c.orig      2021-03-29 22:44:02.316971970 -0300
> +++ linux-pm/drivers/cpuidle/governors/menu.c   2021-03-29 22:51:15.804377168 -0300
> @@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_dr
>         u64 predicted_ns;
>         u64 interactivity_req;
>         unsigned long nr_iowaiters;
> -       ktime_t delta_next;
> +       ktime_t delta_tick;
>         int i, idx;
>
>         if (data->needs_update) {
> @@ -280,7 +280,12 @@ static int menu_select(struct cpuidle_dr
>         }
>
>         /* determine the expected residency time, round up */
> -       data->next_timer_ns = tick_nohz_get_sleep_length(&delta_next);
> +       data->next_timer_ns = tick_nohz_get_sleep_length(&delta_tick);
> +
> +       if (unlikely(data->next_timer_ns >> 63)) {
> +               data->next_timer_ns = 0;
> +               delta_tick = 0;
> +       }

Well, not really.  Using a new local var is cleaner IMO.

Re: [PATCH v1 0/5] cpuidle: Take possible negative "sleep length" values into account

[Rafael J. Wysocki]

On Mon, Mar 29, 2021 at 8:38 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Hi All,
>
> As follows from the discussion triggered by the patch at
>
> https://lore.kernel.org/lkml/20210311123708.23501-2-frederic@kernel.org/
>
> the cpuidle governors using tick_nohz_get_sleep_length() assume it to always
> return positive values which is not correct in general.
>
> To address this issues, first document the fact that negative values can
> be returned by tick_nohz_get_sleep_length() (patch [1/5]).  Then, in
> preparation for more substantial changes, change the data type of two
> fields in struct cpuidle_state to s64 so they can be used in computations
> involving negative numbers safely (patch [2/5]).
>
> Next, adjust the teo governor a bit so that negative "sleep length" values
> are counted like zero by it (patch [3/5]) and modify it so as to avoid
> mishandling negative "sleep length" values (patch [4/5]).
>
> Finally, make the menu governor take negative "sleep length" values into
> account properly (patch [5/5]).
>
> Please see the changelogs of the patches for details.

Given no objections or concerns regarding this lot, let me queue it up.

Thanks!