Hi Viresh,
On Wed, Feb 28, 2018 at 12:18:25PM +0530, Viresh Kumar wrote:
find_best_target() tries to find the target CPU where the task should be placed, based on how much would be the utilization of the CPU after the task is placed on it. This is represented by 'new_util' in the routine.
Currently it adds task_util(p) to the wake_util of the CPU to find that out, while it should really be adding the boosted task's util to wake_util, as that's how much the cpu utilization would be.
This is how we used to do it before commit 3bfde3b4f848 ("ANDROID: sched/fair: Change cpu iteration order in find_best_target()"), was merged and the commit doesn't describe the rational behind this change.
This patch reverts to the earlier formula to calculate the new_util.
Fixes: 3bfde3b4f848 ("ANDROID: sched/fair: Change cpu iteration order in find_best_target()") Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
Just wanted to get some review done over the list before posting it to gerrit. Not sure if this commit is doing the right thing, but I couldn't understand why it should be done this way.
This is for the Android 4.9 EAS dev kernel.
kernel/sched/fair.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 88abd5de69ce..1c33a2ddd39c 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6860,14 +6860,13 @@ static inline int find_best_target(struct task_struct *p, int *backup_cpu, * accounting. However, the blocked utilization may be zero. */ wake_util = cpu_util_wake(i, p);
new_util = wake_util + task_util(p); /* * Ensure minimum capacity to grant the required boost. * The target CPU can be already at a capacity level higher * than the one required to boost the task. */
new_util = max(min_util, new_util);
Essentially 'new_util' is one metric for unflated util value, so we can use it to estimate how cpu is 'busy' after place task on it.
But we don't want to place a boosted task on one low capacity CPU, so finally select max value: max(min_util, new_util).
Actually 'new_util' is mixed for two metrics, if we divide it into two metrics, we might get more clearly idea for it: one metric is to estimate the 'pure' utilization so we can select least util CPU with low scheduling latency; another metric is boosted cpu util, we can name as 'new_boosted_util' and use it to select CPU with sufficient capacity with 80% margin.
new_util = wake_util + min_util;
IMHO, this is a bit more confused :)
new_util = pure cpu util + boosted task util; ideally we can have a new metric as boosted_new_util = boosted(cpu) + boosted(task).
This is just my guess, I'd like to hear suggestions from ARM colleagues as well.
/* * Include minimum capacity constraint:
-- 2.15.0.194.g9af6a3dea062
eas-dev mailing list eas-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/eas-dev