Hi Quentin, On Fri, Sep 20, 2019 at 11:41:15AM +0200, Quentin Perret wrote: > Hi Pavan, > > On Friday 20 Sep 2019 at 08:32:15 (+0530), Pavan Kondeti wrote: > > Earlier, we are not checking the spare capacity for the prev_cpu. Now that > > the > > continue statement is removed, prev_cpu could also be the max_spare_cap_cpu. > > Actually that makes sense. Because there is no reason why we want to select > > another CPU which has less spare capacity than previous CPU. > > > > Is this behavior intentional? > > The intent was indeed to not compute the energy for another CPU in > prev_cpu's perf domain if prev_cpu is the one with max spare cap -- it > is useless to do so since this other CPU cannot 'beat' prev_cpu and > will never be chosen in the end.
Yes. Selecting the prev_cpu is the correct decision. > > But I did miss that we'd end up computing the energy for prev_cpu > twice ... Harmless but useless. So yeah, let's optimize that case too :) > > > When prev_cpu == max_spare_cap_cpu, we are evaluating the energy again for > > the > > same CPU below. That could have been skipped by returning prev_cpu when > > prev_cpu == max_spare_cap_cpu. > > Right, something like the patch below ? My test results are still > looking good with it applied. > > Thanks for the careful review, > Quentin > --- > From 7b8258287f180a2c383ebe397e8129f5f898ffbe Mon Sep 17 00:00:00 2001 > From: Quentin Perret <[email protected]> > Date: Fri, 20 Sep 2019 09:07:20 +0100 > Subject: [PATCH] sched/fair: Avoid redundant EAS calculation > > The EAS wake-up path computes the system energy for several CPU > candidates: the CPU with maximum spare capacity in each performance > domain, and the prev_cpu. However, if prev_cpu also happens to be the > CPU with maximum spare capacity in its performance domain, the energy > calculation is still done twice, unnecessarily. > > Add a condition to filter out this corner case before doing the energy > calculation. > > Reported-by: Pavan Kondeti <[email protected]> > Signed-off-by: Quentin Perret <[email protected]> > --- > kernel/sched/fair.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index d4bbf68c3161..7399382bc291 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -6412,7 +6412,7 @@ static int find_energy_efficient_cpu(struct task_struct > *p, int prev_cpu) > } > > /* Evaluate the energy impact of using this CPU. */ > - if (max_spare_cap_cpu >= 0) { > + if (max_spare_cap_cpu >= 0 && max_spare_cap_cpu != prev_cpu) { > cur_delta = compute_energy(p, max_spare_cap_cpu, pd); > cur_delta -= base_energy_pd; > if (cur_delta < best_delta) { > -- > 2.22.1 > +1. Looks good to me. -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
