On Tue, Mar 30, 2021 at 4:00 AM Zhou Ti (x2019cwm) <x2019...@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.wyso...@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.