Re: [PATCH 2/4] sched/fair: Move avg_scan_cost calculations under SIS_PROP

From: Vincent Guittot
Date: Tue Dec 08 2020 - 11:04:31 EST


On Tue, 8 Dec 2020 at 16:35, Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxxxxxx>
> ---
> 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;

Just noticed while reviewing the patch that the above related to
this_sd can also go under sched_feat(SIS_PROP)

>
> + 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;
> }
> --
> 2.26.2
>