On Tue, Oct 24, 2017 at 07:45:07AM -0700, Joel Fernandes wrote:
Hi Leo,
Nice to see this patch and I like this approach better than the alternate approach proposed since its cleaner.
Thanks!
Just for fyi, I am experimenting with patches to use mainline's find_idlest_group for Android: https://android-review.googlesource.com/#/q/status:open+project:kernel/commo... My thought is to discontinue using our out-of-tree find_best_target as much as possible and switch to using mainline slow/fast paths. It was resulted in a decent code reduction as well.
I was wondering if we can discontinue modifying find_best_target more, and continue working on that? Although the patches in the link above only replace (Case A - prefer-idle latency sensitive tasks) to use mainline - so this particular patch of yours is still fine. Some more
I agree it's a good direction for Case A to use mainline paths.
inline comments below:
On Mon, Oct 23, 2017 at 11:21 PM, Leo Yan leo.yan@linaro.org wrote:
The idle candidate CPUs utilization may have quite different values, as result this may introduce high CPU OPP selection if place the waken task on the candidate CPU which has high utilization value.
This commit introduces the variable 'best_idle_max_spare_cap' to select the maximum spare capacity within the idle CPUs with the same idle state. This results in the candidate idle CPU has lowest utilization, and later it compares energy with the candidate active CPU which also has lowest utilization from all active CPUs
This commit is highly inspired by Joonwoo Park's patch "sched/fair: take CPU energy cost into consideration for placement"; Joonwoo's patch uses cpu_util(), which may has stale utilization value for waken up task, and cpu_util() value is hard to take account into RT/DL pressure. This commit uses variable 'new_util' to calculate the maximum spare capacity for idle CPUs, which has fixed up the waken task util and can more smoothly to considered RT/DL pressure by sequential optimization.
Signed-off-by: Leo Yan leo.yan@linaro.org
kernel/sched/fair.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 062a3b7..4b17a08 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6643,6 +6643,7 @@ static inline int find_best_target(struct task_struct *p, int *backup_cpu, unsigned long target_capacity = ULONG_MAX; unsigned long min_wake_util = ULONG_MAX; unsigned long target_max_spare_cap = 0;
unsigned long best_idle_max_spare_cap = 0; unsigned long target_util = ULONG_MAX; unsigned long best_active_util = ULONG_MAX; int best_idle_cstate = INT_MAX;
@@ -6897,15 +6898,27 @@ static inline int find_best_target(struct task_struct *p, int *backup_cpu, * if they are also less energy efficient. * IOW, prefer a deep IDLE LITTLE CPU vs a * shallow idle big CPU.
*
* Within same idle state CPUs, select the
* maximum spare capacity CPU. */
if (sysctl_sched_cstate_aware &&
best_idle_cstate <= idle_idx)
continue;
if (sysctl_sched_cstate_aware) {
if (best_idle_cstate < idle_idx)
continue;
if (best_idle_cstate == idle_idx &&
(capacity_orig - new_util) < best_idle_max_spare_cap)
continue;
} else {
if ((capacity_orig - new_util) < best_idle_max_spare_cap)
continue;
This is quite similar to the patch I am testing more that adds capacity_of to mainline's capacity_spare_wake. Things look good with my synthetic and Rohit's tests. I posted a thread about it here to eas-dev: https://lists.linaro.org/pipermail/eas-dev/2017-October/001005.html
I will look into more deeply for your and Rohit's patches.
If there's a way we can switch to using that for this case, that would be great as well - although this path also does energy diff.
I think you are suggesting the modification like below patch; please note below patch has two issues, first one issue I have replied in another email that capacity_of() cannot update RT pressure for every call, another issue is for WALT signal the RT pressure has been counted twice (one is from capacity_of(), another is from cpu_util() which natually includes RT + CFS util).
---8<---
From eb5544d3f8827b196453485b65a4ef72b6748bdc Mon Sep 17 00:00:00 2001
From: Leo Yan leo.yan@linaro.org Date: Tue, 17 Oct 2017 09:26:55 +0800 Subject: [PATCH 3/7] sched/fair: Replace capacity_orig_of() with capacity_of() in find_best_target()
Inspired by Vincent, the CFS scheduling class uses capacity_of() to present RT/DL pressure and capacity_orig_of() presents the CPU original capacity; Furthermore, capacity_of() reflects the capacity capping from thermal throttling as well.
This patch is to use capacity_of() to replace capacity_orig_of() for below two cases in find_best_target():
- The first case is to check CPU is overutilized, in this case we use capacity_of() rather than capacity_orig_of(); as result this can let find_best_target() has consistent behaviour with function cpu_overutilized();
- The second case is calculation the maximum spare capacity; for this case we need to consider RT/DL pressure, so can use capacity_of() value to reflect it. This results in we can select CPU with lowest sum utilization from RT, DT and CFS classes.
Change-Id: I4b0bb994f9c28f90f55dd6572c90f11bc0608f70 Signed-off-by: Leo Yan leo.yan@linaro.org --- kernel/sched/fair.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 9aaf8ee..ae85fa7 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6691,7 +6691,7 @@ static inline int find_best_target(struct task_struct *p, int *backup_cpu, sg = sd->groups; do { for_each_cpu_and(i, tsk_cpus_allowed(p), sched_group_cpus(sg)) { - unsigned long capacity_curr = capacity_curr_of(i); + unsigned long capacity = capacity_of(i); unsigned long capacity_orig = capacity_orig_of(i); unsigned long wake_util, new_util;
@@ -6813,9 +6813,9 @@ static inline int find_best_target(struct task_struct *p, int *backup_cpu, * Case A.2: Target ACTIVE CPU * Favor CPUs with max spare capacity. */ - if ((capacity_curr > new_util) && - (capacity_orig - new_util > target_max_spare_cap)) { - target_max_spare_cap = capacity_orig - new_util; + if ((capacity > new_util) && + (capacity - new_util > target_max_spare_cap)) { + target_max_spare_cap = capacity - new_util; target_cpu = i; continue; } @@ -6863,7 +6863,7 @@ static inline int find_best_target(struct task_struct *p, int *backup_cpu, * possible at least for !prefer_idle tasks. */ if ((new_util * capacity_margin) > - (capacity_orig * SCHED_CAPACITY_SCALE)) + (capacity * SCHED_CAPACITY_SCALE)) continue;
/* @@ -6939,10 +6939,10 @@ static inline int find_best_target(struct task_struct *p, int *backup_cpu, continue;
/* Favor CPUs with maximum spare capacity */ - if ((capacity_orig - new_util) < target_max_spare_cap) + if ((capacity - new_util) < target_max_spare_cap) continue;
- target_max_spare_cap = capacity_orig - new_util; + target_max_spare_cap = capacity - new_util; target_capacity = capacity_orig; target_util = new_util; target_cpu = i; -- 1.9.1
thanks,
- Joel
<snip>