Messages in this thread Patch in this message | | | From | Vincent Guittot <> | Date | Tue, 8 Dec 2020 14:43:10 +0100 | Subject | Re: [PATCH 1/4] sched/fair: Remove SIS_AVG_CPU |
| |
On Tue, 8 Dec 2020 at 14:36, Mel Gorman <mgorman@techsingularity.net> wrote: > > On Tue, Dec 08, 2020 at 02:24:32PM +0100, Vincent Guittot wrote: > > > > Nitpick: > > > > > > > > Since now avg_cost and avg_idle are only used w/ SIS_PROP, they could go > > > > completely into the SIS_PROP if condition. > > > > > > > > > > Yeah, I can do that. In the initial prototype, that happened in a > > > separate patch that split out SIS_PROP into a helper function and I > > > never merged it back. It's a trivial change. > > > > while doing this, should you also put the update of > > this_sd->avg_scan_cost under the SIS_PROP feature ? > > > > It's outside the scope of the series but why not. This? > > --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> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 19ca0265f8aa..0fee53b1aae4 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -6176,10 +6176,10 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, int t > nr = 4; > } > > - time = cpu_clock(this);
I would move it in the if (sched_feat(SIS_PROP)) above.
> - > cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr); > > + if (sched_feat(SIS_PROP)) > + time = cpu_clock(this); > for_each_cpu_wrap(cpu, cpus, target) { > if (!--nr) > return -1; > @@ -6187,8 +6187,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; > }
| |