Messages in this thread Patch in this message | | | Subject | Re: [RFC PATCH 4/6] sched/fair: Introduce an energy estimation helper function | From | Dietmar Eggemann <> | Date | Wed, 21 Mar 2018 22:15:21 +0100 |
| |
On 03/21/2018 03:02 PM, Quentin Perret wrote: > On Wednesday 21 Mar 2018 at 12:26:21 (+0000), Patrick Bellasi wrote: >> On 21-Mar 10:04, Juri Lelli wrote: >>> Hi, >>> >>> On 20/03/18 09:43, Dietmar Eggemann wrote: >>>> From: Quentin Perret <quentin.perret@arm.com>
[...]
>> Actually I think that this whole function can be written "just" as: >> >> ---8<--- >> unsigned long util = cpu_util_wake(cpu); >> >> if (cpu != dst_cpu) >> return util; >> >> return min(util + task_util(p), capacity_orig_of(cpu)); >> ---8<--- >> > > Yes this should be functionally equivalent. However, with your > suggestion you can potentially remove the task contribution from the > cpu_util in cpu_util_wake() and then add it back right after if > cpu==dst_cpu. This is sub-optimal and that's why I implemented things > slightly differently. But maybe this optimization really is too small to > justify the extra complexity involved ...
What about we merge both functions by adding an additional 'int dst_cpu' parameter to cpu_util_wake() (only lightly tested, w/o util_est):
--->8---
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 65a1bead0773..4d4f104d5b3d 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -5860,11 +5860,11 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p, } static inline unsigned long task_util(struct task_struct *p); -static unsigned long cpu_util_wake(int cpu, struct task_struct *p); +static unsigned long cpu_util_wake(int cpu, int dst_cpu, struct task_struct *p); static unsigned long capacity_spare_wake(int cpu, struct task_struct *p) { - return max_t(long, capacity_of(cpu) - cpu_util_wake(cpu, p), 0); + return max_t(long, capacity_of(cpu) - cpu_util_wake(cpu, -1, p), 0); } /* @@ -6384,16 +6384,22 @@ static inline unsigned long task_util(struct task_struct *p) * cpu_util_wake: Compute CPU utilization with any contributions from * the waking task p removed. */ -static unsigned long cpu_util_wake(int cpu, struct task_struct *p) +static unsigned long cpu_util_wake(int cpu, int dst_cpu, struct task_struct *p) { unsigned long util, capacity; /* Task has no contribution or is new */ - if (cpu != task_cpu(p) || !p->se.avg.last_update_time) + if ((cpu != task_cpu(p) && cpu != dst_cpu) || + dst_cpu == task_cpu(p) || !p->se.avg.last_update_time) return cpu_util(cpu); capacity = capacity_orig_of(cpu); - util = max_t(long, cpu_rq(cpu)->cfs.avg.util_avg - task_util(p), 0); + util = cpu_rq(cpu)->cfs.avg.util_avg; + + if (likely(dst_cpu != cpu)) + util = max_t(long, util - task_util(p), 0); + else + util += task_util(p); return (util >= capacity) ? capacity : util; } @@ -6409,30 +6415,6 @@ static inline int cpu_overutilized(int cpu) } /* - * Returns the util of "cpu" if "p" wakes up on "dst_cpu". - */ -static unsigned long cpu_util_next(int cpu, struct task_struct *p, int dst_cpu) -{ - unsigned long util = cpu_rq(cpu)->cfs.avg.util_avg; - unsigned long capacity = capacity_orig_of(cpu); - - /* - * If p is where it should be, or if it has no impact on cpu, there is - * not much to do. - */ - if ((task_cpu(p) == dst_cpu) || (cpu != task_cpu(p) && cpu != dst_cpu)) - goto clamp_util; - - if (dst_cpu == cpu) - util += task_util(p); - else - util = max_t(long, util - task_util(p), 0); - -clamp_util: - return (util >= capacity) ? capacity : util; -} - -/* * Disable WAKE_AFFINE in the case where task @p doesn't fit in the * capacity of either the waking CPU @cpu or the previous CPU @prev_cpu. * @@ -6488,7 +6470,7 @@ static unsigned long compute_energy(struct task_struct *p, int dst_cpu) for_each_freq_domain(fdom) { fdom_max_util = 0; for_each_cpu_and(cpu, &(fdom->span), cpu_online_mask) { - util = cpu_util_next(cpu, p, dst_cpu); + util = cpu_util_wake(cpu, dst_cpu, p); fdom_max_util = max(util, fdom_max_util); } @@ -6506,7 +6488,7 @@ static unsigned long compute_energy(struct task_struct *p, int dst_cpu) * busy time. */ for_each_cpu_and(cpu, &(fdom->span), cpu_online_mask) { - util = cpu_util_next(cpu, p, dst_cpu); + util = cpu_util_wake(cpu, dst_cpu, p); energy += cs->power * util / cs->cap; } }
| |