Messages in this thread | | | From | Vincent Guittot <> | Date | Tue, 8 Dec 2020 16:19:40 +0100 | Subject | Re: [PATCH 1/4] sched/fair: Remove SIS_AVG_CPU |
| |
On Tue, 8 Dec 2020 at 16:12, Mel Gorman <mgorman@techsingularity.net> wrote: > > On Tue, Dec 08, 2020 at 03:47:40PM +0100, Vincent Guittot wrote: > > > I considered it but made the choice to exclude the cost of cpumask_and() > > > from the avg_scan_cost instead. It's minor but when doing the original > > > > At the cost of a less readable code > > > > Slightly less readable, yes. > > > > prototype, I didn't think it was appropriate to count the cpumask > > > clearing as part of the scan cost as it's not directly related. > > > > hmm... I think it is because the number of loop is directly related to > > the allowed cpus > > > > While that is true, the cost of initialising the map is constant and > what is most important is tracking the scan cost which is variable. > Without SIS_AVG_CPU, the cpumask init can go before SIS_PROP without any > penalty so is this version preferable?
yes looks good to me
> > --8<-- > sched/fair: Move avg_scan_cost calculations under SIS_PROP > > As noted by Vincent Guittot, avg_scan_costs are calculated for SIS_PROP > even if SIS_PROP is disabled. Move the time calculations under a SIS_PROP > check and while we are at it, exclude the cost of initialising the CPU > mask from the average scan cost. > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > --- > kernel/sched/fair.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index ac7b34e7372b..5c41875aec23 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -6153,6 +6153,8 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t > if (!this_sd) > return -1; > > + cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr); > + > if (sched_feat(SIS_PROP)) { > u64 avg_cost, avg_idle, span_avg; > > @@ -6168,11 +6170,9 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t > nr = div_u64(span_avg, avg_cost); > else > nr = 4; > - } > - > - time = cpu_clock(this); > > - cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr); > + time = cpu_clock(this); > + } > > for_each_cpu_wrap(cpu, cpus, target) { > if (!--nr) > @@ -6181,8 +6181,10 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t > break; > } > > - time = cpu_clock(this) - time; > - update_avg(&this_sd->avg_scan_cost, time); > + if (sched_feat(SIS_PROP)) { > + time = cpu_clock(this) - time; > + update_avg(&this_sd->avg_scan_cost, time); > + } > > return cpu; > }
| |