linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Nicolas Pitre <nicolas.pitre@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linaro-kernel@lists.linaro.org
Subject: Re: [PATCH v2 2/2] sched/fair: leverage the idle state info when choosing the "idlest" cpu
Date: Fri, 05 Sep 2014 09:52:53 +0200	[thread overview]
Message-ID: <54096BD5.4030803@linaro.org> (raw)
In-Reply-To: <1409844730-12273-3-git-send-email-nicolas.pitre@linaro.org>

On 09/04/2014 05:32 PM, Nicolas Pitre wrote:
> The code in find_idlest_cpu() looks for the CPU with the smallest load.
> However, if multiple CPUs are idle, the first idle CPU is selected
> irrespective of the depth of its idle state.
>
> Among the idle CPUs we should pick the one with with the shallowest idle
> state, or the latest to have gone idle if all idle CPUs are in the same
> state.  The later applies even when cpuidle is configured out.
>
> This patch doesn't cover the following issues:
>
> - The idle exit latency of a CPU might be larger than the time needed
>    to migrate the waking task to an already running CPU with sufficient
>    capacity, and therefore performance would benefit from task packing
>    in such case (in most cases task packing is about power saving).
>
> - Some idle states have a non negligible and non abortable entry latency
>    which needs to run to completion before the exit latency can start.
>    A concurrent patch series is making this info available to the cpuidle
>    core.  Once available, the entry latency with the idle timestamp could
>    determine when the exit latency may be effective.
>
> Those issues will be handled in due course.  In the mean time, what
> is implemented here should improve things already compared to the current
> state of affairs.
>
> Based on an initial patch from Daniel Lezcano.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>

Sounds good for me.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>   kernel/sched/fair.c | 43 ++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bfa3c86d0d..416329e1a6 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -23,6 +23,7 @@
>   #include <linux/latencytop.h>
>   #include <linux/sched.h>
>   #include <linux/cpumask.h>
> +#include <linux/cpuidle.h>
>   #include <linux/slab.h>
>   #include <linux/profile.h>
>   #include <linux/interrupt.h>
> @@ -4428,20 +4429,48 @@ static int
>   find_idlest_cpu(struct sched_group *group, struct task_struct *p, int this_cpu)
>   {
>   	unsigned long load, min_load = ULONG_MAX;
> -	int idlest = -1;
> +	unsigned int min_exit_latency = UINT_MAX;
> +	u64 latest_idle_timestamp = 0;
> +	int least_loaded_cpu = this_cpu;
> +	int shallowest_idle_cpu = -1;
>   	int i;
>
>   	/* Traverse only the allowed CPUs */
>   	for_each_cpu_and(i, sched_group_cpus(group), tsk_cpus_allowed(p)) {
> -		load = weighted_cpuload(i);
> -
> -		if (load < min_load || (load == min_load && i == this_cpu)) {
> -			min_load = load;
> -			idlest = i;
> +		if (idle_cpu(i)) {
> +			struct rq *rq = cpu_rq(i);
> +			struct cpuidle_state *idle = idle_get_state(rq);
> +			if (idle && idle->exit_latency < min_exit_latency) {
> +				/*
> +				 * We give priority to a CPU whose idle state
> +				 * has the smallest exit latency irrespective
> +				 * of any idle timestamp.
> +				 */
> +				min_exit_latency = idle->exit_latency;
> +				latest_idle_timestamp = rq->idle_stamp;
> +				shallowest_idle_cpu = i;
> +			} else if ((!idle || idle->exit_latency == min_exit_latency) &&
> +				   rq->idle_stamp > latest_idle_timestamp) {
> +				/*
> +				 * If equal or no active idle state, then
> +				 * the most recently idled CPU might have
> +				 * a warmer cache.
> +				 */
> +				latest_idle_timestamp = rq->idle_stamp;
> +				shallowest_idle_cpu = i;
> +			}
> +			cpuidle_put_state(rq);
> +		} else {
> +			load = weighted_cpuload(i);
> +			if (load < min_load ||
> +			    (load == min_load && i == this_cpu)) {
> +				min_load = load;
> +				least_loaded_cpu = i;
> +			}
>   		}
>   	}
>
> -	return idlest;
> +	return shallowest_idle_cpu != -1 ? shallowest_idle_cpu : least_loaded_cpu;
>   }
>
>   /*
>


-- 
  <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


  reply	other threads:[~2014-09-05  7:52 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-04 15:32 [PATCH v2 0/2] sched/idle : find the best idle CPU with cpuidle info Nicolas Pitre
2014-09-04 15:32 ` [PATCH v2 1/2] sched: let the scheduler see CPU idle states Nicolas Pitre
2014-09-18 17:37   ` Paul E. McKenney
2014-09-18 17:39     ` Paul E. McKenney
2014-09-18 23:15       ` Peter Zijlstra
2014-09-18 18:32     ` Nicolas Pitre
2014-09-18 23:17       ` Peter Zijlstra
2014-09-18 23:28         ` Peter Zijlstra
2014-09-19 18:30           ` Nicolas Pitre
2014-09-04 15:32 ` [PATCH v2 2/2] sched/fair: leverage the idle state info when choosing the "idlest" cpu Nicolas Pitre
2014-09-05  7:52   ` Daniel Lezcano [this message]
2014-09-18 23:46   ` Peter Zijlstra
2014-09-19  0:05   ` Peter Zijlstra
2014-09-19  4:49   ` Yao Dongdong
2014-09-30 21:58   ` Rik van Riel
2014-09-30 23:15     ` Nicolas Pitre
2014-10-02 17:15       ` [PATCH RFC] sched,idle: teach select_idle_sibling about idle states Rik van Riel
2014-10-03  6:04         ` Mike Galbraith
2014-10-03  6:23         ` Mike Galbraith
2014-10-03  7:50           ` Peter Zijlstra
2014-10-03 13:05             ` Mike Galbraith
2014-10-03 14:28             ` Rik van Riel
2014-10-03 14:46               ` Peter Zijlstra
2014-10-03 15:37                 ` Rik van Riel
2014-10-09 16:04                   ` Peter Zijlstra
2014-10-03 18:52               ` Nicolas Pitre
2014-09-10 21:35 ` [PATCH v2 0/2] sched/idle : find the best idle CPU with cpuidle info Nicolas Pitre
2014-09-10 22:50   ` Rafael J. Wysocki
2014-09-10 23:25     ` Nicolas Pitre
2014-09-10 23:28       ` Nicolas Pitre
2014-09-10 23:50       ` Rafael J. Wysocki
2014-09-18  0:39   ` Nicolas Pitre
2014-09-18 23:24     ` Peter Zijlstra
2014-09-19 18:22       ` Nicolas Pitre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54096BD5.4030803@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nicolas.pitre@linaro.org \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).