Re: [RFC PATCH v2 1/8] cpuidle: menu: extract prediction functionality

Rafael J. Wysocki
2017-10-14T00:36:24Z

On Saturday, September 30, 2017 9:20:27 AM CEST Aubrey Li wrote:
> There are several factors in the menu governor to predict the next
> idle interval:
> - the next timer
> - the recent idle interval history
> - the corrected idle interval pattern
> These factors are common enough to be extracted to be one function.
>
> Signed-off-by: Aubrey Li <aubrey.li@linux.intel.com>

This patch alone would break things AFAICS, because it removes code from
menu_select() without a replacement (and menu_predict() is never called
just yet).

Please always do your best to ensure that things will work after *every*
patch in a series.

> ---
> drivers/cpuidle/governors/menu.c | 64 +++++++++++++++++++++++++---------------
> 1 file changed, 40 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
> index 61b64c2..6bed197 100644
> --- a/drivers/cpuidle/governors/menu.c
> +++ b/drivers/cpuidle/governors/menu.c
> @@ -276,6 +276,45 @@ static unsigned int get_typical_interval(struct menu_device *data)
> }
>
> /**
> + * menu_predict - predict the coming idle interval
> + *
> + * Return the predicted coming idle interval in micro-second
> + */
> +static unsigned int menu_predict(void)
> +{
> + struct menu_device *data = this_cpu_ptr(&menu_devices);
> + unsigned int expected_interval;
> + int cpu = smp_processor_id();
> +
> + if (!data)
> + return UINT_MAX;
> +
> + /* determine the expected residency time, round up */
> + data->next_timer_us = ktime_to_us(tick_nohz_get_sleep_length());
> +
> + data->bucket = which_bucket(data->next_timer_us, nr_iowait_cpu(cpu));
> +
> + /*
> + * Force the result of multiplication to be 64 bits even if both
> + * operands are 32 bits.
> + * Make sure to round up for half microseconds.
> + */
> + data->predicted_us = DIV_ROUND_CLOSEST_ULL((uint64_t)
> + data->next_timer_us * data->correction_factor[data->bucket],
> + RESOLUTION * DECAY);
> +
> + expected_interval = get_typical_interval(data);
> + expected_interval = min(expected_interval, data->next_timer_us);
> +
> + /*
> + * Use the lowest expected idle interval to pick the idle state.
> + */
> + data->predicted_us = min(data->predicted_us, expected_interval);
> +
> + return data->predicted_us;
> +}
> +
> +/**
> * menu_select - selects the next idle state to enter
> * @drv: cpuidle driver containing state data
> * @dev: the CPU
> @@ -289,7 +328,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> int first_idx;
> int idx;
> unsigned int interactivity_req;
> - unsigned int expected_interval;
> unsigned long nr_iowaiters, cpu_load;
> int resume_latency = dev_pm_qos_raw_read_value(device);
>
> @@ -306,24 +344,6 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> if (unlikely(latency_req == 0))
> return 0;
>
> - /* determine the expected residency time, round up */
> - data->next_timer_us = ktime_to_us(tick_nohz_get_sleep_length());
> -
> - get_iowait_load(&nr_iowaiters, &cpu_load);
> - data->bucket = which_bucket(data->next_timer_us, nr_iowaiters);
> -
> - /*
> - * Force the result of multiplication to be 64 bits even if both
> - * operands are 32 bits.
> - * Make sure to round up for half microseconds.
> - */
> - data->predicted_us = DIV_ROUND_CLOSEST_ULL((uint64_t)data->next_timer_us *
> - data->correction_factor[data->bucket],
> - RESOLUTION * DECAY);
> -
> - expected_interval = get_typical_interval(data);
> - expected_interval = min(expected_interval, data->next_timer_us);
> -
> if (CPUIDLE_DRIVER_STATE_START > 0) {
> struct cpuidle_state *s = &drv->states[CPUIDLE_DRIVER_STATE_START];
> unsigned int polling_threshold;
> @@ -345,14 +365,10 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev)
> }
>
> /*
> - * Use the lowest expected idle interval to pick the idle state.
> - */
> - data->predicted_us = min(data->predicted_us, expected_interval);
> -
> - /*
> * Use the performance multiplier and the user-configurable
> * latency_req to determine the maximum exit latency.
> */
> + get_iowait_load(&nr_iowaiters, &cpu_load);
> interactivity_req = data->predicted_us / performance_multiplier(nr_iowaiters, cpu_load);
> if (latency_req > interactivity_req)
> latency_req = interactivity_req;
>