On 02/28/2018 07:48 AM, 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.
AFAIK, we only want to use util (not the boosted one) when we sum up cpu and task values. This util is then used to compare against the capacity values.
The boosted task util is there to set a certain frequency on a cpu. If this value is not higher then the actual cpu util + task util, then the latter sum will already make sure that this frequency on this cpu will be selected.
I would like to hear Patrick's and Chris' opinion about it since they have been the custodians of find_best_target() in 4.4 and 4.9.
[+cc Patrick and Chris]
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.
You're right, there is no explanation in the patch header why this was changed as well. The only thing I can think of is that the original patch I sent out didn't have this and it was added later and we forgot to add a paragraph to the patch header.
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);
new_util = wake_util + min_util; /* * Include minimum capacity constraint: