[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5547870E.6060308@linaro.org>
Date: Mon, 04 May 2015 16:49:50 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Peter Zijlstra <peterz@...radead.org>
CC: Linux PM list <linux-pm@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] sched / idle: Eliminate the "reflect" check from
cpuidle_idle_call()
On 05/04/2015 03:57 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Since cpuidle_reflect() should only be called if the idle state
> to enter was selected by cpuidle_select(), there is the "reflect"
> variable in cpuidle_idle_call() whose value is used to determine
> whether or not that is the case.
>
> However, if the entire code run between the conditional setting
> "reflect" and the call to cpuidle_reflect() is moved to a separate
> function, it will be possible to call that new function in both
> branches of the conditional, in which case cpuidle_reflect() will
> only need to be called from one of them too and the "reflect"
> variable won't be necessary any more.
>
> This eliminates one check made by cpuidle_idle_call() on the majority
> of its invocations, so change the code as described.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Reviewed-by: Daniel Lezcano <daniel.lezcano@...aro.org>
> ---
> kernel/sched/idle.c | 90 ++++++++++++++++++++++++++--------------------------
> 1 file changed, 46 insertions(+), 44 deletions(-)
>
> Index: linux-pm/kernel/sched/idle.c
> ===================================================================
> --- linux-pm.orig/kernel/sched/idle.c
> +++ linux-pm/kernel/sched/idle.c
> @@ -78,6 +78,46 @@ static void default_idle_call(void) {
> arch_cpu_idle();
> }
>
> +static int call_cpuidle(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> + int next_state)
> +{
> + int entered_state;
> +
> + /* Fall back to the default arch idle method on errors. */
> + if (next_state < 0) {
> + default_idle_call();
> + return next_state;
> + }
> +
> + /*
> + * The idle task must be scheduled, it is pointless to go to idle, just
> + * update no idle residency and return.
> + */
> + if (current_clr_polling_and_test()) {
> + dev->last_residency = 0;
> + local_irq_enable();
> + return -EBUSY;
> + }
> +
> + /* Take note of the planned idle state. */
> + idle_set_state(this_rq(), &drv->states[next_state]);
> +
> + /*
> + * Enter the idle state previously returned by the governor decision.
> + * This function will block until an interrupt occurs and will take
> + * care of re-enabling the local interrupts
> + */
> + entered_state = cpuidle_enter(drv, dev, next_state);
> +
> + /* The cpu is no longer idle or about to enter idle. */
> + idle_set_state(this_rq(), NULL);
> +
> + if (entered_state == -EBUSY)
> + default_idle_call();
> +
> + return entered_state;
> +}
> +
> /**
> * cpuidle_idle_call - the main idle function
> *
> @@ -92,7 +132,6 @@ static void cpuidle_idle_call(void)
> struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
> struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
> int next_state, entered_state;
> - bool reflect;
>
> /*
> * Check if the idle task must be rescheduled. If it is the
> @@ -137,56 +176,19 @@ static void cpuidle_idle_call(void)
> goto exit_idle;
> }
>
> - reflect = false;
> next_state = cpuidle_find_deepest_state(drv, dev);
> + call_cpuidle(drv, dev, next_state);
> } else {
> - reflect = true;
> /*
> * Ask the cpuidle framework to choose a convenient idle state.
> */
> next_state = cpuidle_select(drv, dev);
> - }
> - /* Fall back to the default arch idle method on errors. */
> - if (next_state < 0) {
> - default_idle_call();
> - goto exit_idle;
> - }
> -
> - /*
> - * The idle task must be scheduled, it is pointless to
> - * go to idle, just update no idle residency and get
> - * out of this function
> - */
> - if (current_clr_polling_and_test()) {
> - dev->last_residency = 0;
> - entered_state = next_state;
> - local_irq_enable();
> - goto exit_idle;
> - }
> -
> - /* Take note of the planned idle state. */
> - idle_set_state(this_rq(), &drv->states[next_state]);
> -
> - /*
> - * Enter the idle state previously returned by the governor decision.
> - * This function will block until an interrupt occurs and will take
> - * care of re-enabling the local interrupts
> - */
> - entered_state = cpuidle_enter(drv, dev, next_state);
> -
> - /* The cpu is no longer idle or about to enter idle. */
> - idle_set_state(this_rq(), NULL);
> -
> - if (entered_state == -EBUSY) {
> - default_idle_call();
> - goto exit_idle;
> - }
> -
> - /*
> - * Give the governor an opportunity to reflect on the outcome
> - */
> - if (reflect)
> + entered_state = call_cpuidle(drv, dev, next_state);
> + /*
> + * Give the governor an opportunity to reflect on the outcome
> + */
> cpuidle_reflect(dev, entered_state);
> + }
>
> exit_idle:
> __current_set_polling();
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists