Messages in this thread | | | Date | Mon, 6 Jun 2022 10:41:36 +0100 | From | Vincent Donnefort <> | Subject | Re: [PATCH v9 6/7] sched/fair: Remove task_util from effective utilization in feec() |
| |
[...]
> > + > > +/* > > + * compute_energy(): Use the Energy Model to estimate the energy that @pd would > > + * consume for a given utilization landscape @eenv. If @dst_cpu < 0 the task > > I find this comment a bit confusing because compute_energy() adds the > task contribution if dst_cpu >= 0 but doesn't remove it. The fact that > eenv->pd_busy_time has been previously computed without the > contribution of the task, is outside the scope of this this function > whereas the comment suggest that the remove will happen in > compute_energy()
Arg, leftover from a previous version where this function was adding or removing the contribution. I'll update!
> > > + * contribution is removed from the energy estimation. > > + */ > > +static inline unsigned long > > +compute_energy(struct energy_env *eenv, struct perf_domain *pd, > > + struct cpumask *pd_cpus, struct task_struct *p, int dst_cpu) > > +{ > > + unsigned long max_util = eenv_pd_max_util(eenv, pd_cpus, p, dst_cpu); > > + unsigned long busy_time = eenv->pd_busy_time; > > + > > + if (dst_cpu >= 0) > > + busy_time = min(eenv->pd_cap, busy_time + eenv->task_busy_time); > > + > > + return em_cpu_energy(pd->em_pd, max_util, busy_time, eenv->cpu_cap); > > } > >
[...]
> > @@ -6878,13 +6947,15 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu) > > if (max_spare_cap_cpu < 0 && !compute_prev_delta) > > continue; > > > > + eenv_pd_busy_time(&eenv, cpus, p); > > /* Compute the 'base' energy of the pd, without @p */ > > - base_energy_pd = compute_energy(p, -1, cpus, pd); > > + base_energy_pd = compute_energy(&eenv, pd, cpus, p, -1); > > base_energy += base_energy_pd; > > > > /* Evaluate the energy impact of using prev_cpu. */ > > if (compute_prev_delta) { > > - prev_delta = compute_energy(p, prev_cpu, cpus, pd); > > + prev_delta = compute_energy(&eenv, pd, cpus, p, > > + prev_cpu); > > if (prev_delta < base_energy_pd) > > side question: > -base_energy_pd is the energy for the perf domain without task p > -prev_delta is the energy for the same perf domain if task p is put on dst_cpu > > How can prev_delta be lower than base_energy ?
It can happen if one of the CPU utilization is updated in the middle of feec().
> > if dst_cpu doesn't belong to the perf domain, prev_delta should be > equal to base_energy_pd > if dst_cpu belongs to the perf domain, the compute_energy should be > higher because the busy_time will be higher > > > goto unlock; > > prev_delta -= base_energy_pd; > > @@ -6893,8 +6964,8 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu) > > > > /* Evaluate the energy impact of using max_spare_cap_cpu. */ > > if (max_spare_cap_cpu >= 0) { > > - cur_delta = compute_energy(p, max_spare_cap_cpu, cpus, > > - pd); > > + cur_delta = compute_energy(&eenv, pd, cpus, p, > > + max_spare_cap_cpu); > > if (cur_delta < base_energy_pd) > > same question as above > > > goto unlock; > > cur_delta -= base_energy_pd; > > -- > > 2.36.1.124.g0e6072fb45-goog > >
| |