Hi Leo,
Nice to see this patch and I like this approach better than the alternate approach proposed since its cleaner.
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 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
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.
thanks,
- Joel
<snip>