[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKfTPtCaxGhpGk348Q8jGcEA13Xv4VHS+nX0aW4C+ba5df_4Lw@mail.gmail.com>
Date: Wed, 22 Dec 2021 11:47:34 +0100
From: Vincent Guittot <vincent.guittot@...aro.org>
To: Yicong Yang <yangyicong@...ilicon.com>
Cc: peterz@...radead.org, mingo@...hat.com, juri.lelli@...hat.com,
mgorman@...hsingularity.net, linux-kernel@...r.kernel.org,
dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
bristot@...hat.com, prime.zeng@...wei.com, linuxarm@...wei.com,
21cnbao@...il.com
Subject: Re: [PATCH] sched/fair: Track target domain's avg_scan_cost in select_idle_cpu
On Sat, 11 Dec 2021 at 11:43, Yicong Yang <yangyicong@...ilicon.com> wrote:
>
> We regulate the LLC domain scan in select_idle_cpu() by comparing
> the average scan cost of this_sd against the average idle time of
> this_rq. This is correct when the domain to scan is the LLC domain
> of this cpu. But when the domain to scan is different from this
> LLC domain, we'll have an inaccurate estimation of the scan cost
> on the target domain as this_sd->avg_scan_cost contains contributions
> of scanning other domains besides the target domain.
>
> Track the avg_scan_cost of the target domain to make the estimation
> more accurate.
>
> Signed-off-by: Yicong Yang <yangyicong@...ilicon.com>
> ---
> kernel/sched/fair.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 6e476f6d9435..6301740d98cb 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6267,7 +6267,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
> }
>
> avg_idle = this_rq->wake_avg_idle;
> - avg_cost = this_sd->avg_scan_cost + 1;
> + avg_cost = sd->avg_scan_cost + 1;
>
> span_avg = sd->span_weight * avg_idle;
> if (span_avg > 4*avg_cost)
> @@ -6305,7 +6305,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
> */
> this_rq->wake_avg_idle -= min(this_rq->wake_avg_idle, time);
>
> - update_avg(&this_sd->avg_scan_cost, time);
> + update_avg(&sd->avg_scan_cost, time);
But then you can have several cpus updating the same value simultaneously
> }
>
> return idle_cpu;
> --
> 2.33.0
>
Powered by blists - more mailing lists