On 18-01-18, 10:11, Quentin Perret wrote:
On Thursday 18 Jan 2018 at 12:00:54 (+0530), Viresh Kumar wrote:
The comment inside cpu_util_wake() clearly says that the task_util() isn't subtracted from cpu_util() as WALT doesn't decay idle tasks like PELT does. That probably works fine in most of the cases, but there is at least one case (find_best_target()) where we will account for task_util() twice.
So, I don't have a great knowledge about WALT either but I think what is meant in this comment really is that, since WALT only accounts for runnable tasks, if the task is waking, it is not accounted in cpu_util() so there is no need to remove it.
Yeah, I was talking about this with Vincent few minutes back and noticed that this patch surely isn't correct.
I would say that our goal is to improve the PCMark scores with PELT, not make them worst with WALT ... ;)
We need to make them equal, strategy is all yours :)
kernel/sched/fair.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 2f5925cc541f..06246e02ea09 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6812,6 +6812,14 @@ 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);
+#ifdef CONFIG_SCHED_WALT
if (!walt_disabled && sysctl_sched_use_walt_cpu_util &&i == task_cpu(p)) {wake_util -= task_util(p);Since task_util(p) is not accounted in cpu_util_wake(), you shouldn't do this.
So aren't there two cases:
- The task's utilization is already part of cpu_util(), if the task was running recently.
- The task's utilization isn't part of cpu_util() as it hasn't run for a long time, maybe last window or whatever ?
If the above is true, then we should still need the above diff, but maybe with few more 'if' conditionals, i.e. a way to check if task's util was already part of cpu_util() ?
-- viresh