From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753497AbdJNAgY (ORCPT ); Fri, 13 Oct 2017 20:36:24 -0400 Received: from cloudserver094114.home.net.pl ([79.96.170.134]:60573 "EHLO cloudserver094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753350AbdJNAgX (ORCPT ); Fri, 13 Oct 2017 20:36:23 -0400 From: "Rafael J. Wysocki" To: Aubrey Li Cc: tglx@linutronix.de, peterz@infradead.org, len.brown@intel.com, ak@linux.intel.com, tim.c.chen@linux.intel.com, x86@kernel.org, linux-kernel@vger.kernel.org, Aubrey Li Subject: Re: [RFC PATCH v2 1/8] cpuidle: menu: extract prediction functionality Date: Sat, 14 Oct 2017 02:26:50 +0200 Message-ID: <1629755.KbDSmDPDTX@aspire.rjw.lan> In-Reply-To: <1506756034-6340-2-git-send-email-aubrey.li@intel.com> References: <1506756034-6340-1-git-send-email-aubrey.li@intel.com> <1506756034-6340-2-git-send-email-aubrey.li@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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; >