[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <de1c97e0-9dd7-4e7f-b6f8-ce3f856c96f3@linux.ibm.com>
Date: Fri, 13 Dec 2024 20:32:21 +0530
From: Shrikanth Hegde <sshegde@...ux.ibm.com>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: "H. Peter Anvin" <hpa@...or.com>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Mario Limonciello <mario.limonciello@....com>,
Meng Li <li.meng@....com>, Huang Rui <ray.huang@....com>,
"Gautham R. Shenoy" <gautham.shenoy@....com>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>, x86@...nel.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 8/8] sched/fair: Uncache asym_prefer_cpu and find it
during update_sd_lb_stats()
On 12/12/24 00:25, K Prateek Nayak wrote:
> On AMD processors supporting dynamic preferred core ranking, the
> asym_prefer_cpu cached in sched_group can change dynamically. Since
> asym_prefer_cpu is cached when the sched domain hierarchy is built,
> updating the cached value across the system would require rebuilding
> the sched domain which is prohibitively expensive.
>
> All the asym_prefer_cpu comparisons in the load balancing path are only
> carried out post the sched group stats have been updated after iterating
> all the CPUs in the group. Uncache the asym_prefer_cpu and compute it
> while sched group statistics are being updated as a part of sg_lb_stats.
>
> Fixes: f3a052391822 ("cpufreq: amd-pstate: Enable amd-pstate preferred core support")
> Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>
> ---
> kernel/sched/fair.c | 21 +++++++++++++++++++--
> kernel/sched/sched.h | 1 -
> kernel/sched/topology.c | 15 +--------------
> 3 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 3f36805ecdca..166b8e831064 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9911,6 +9911,8 @@ struct sg_lb_stats {
> unsigned int sum_nr_running; /* Nr of all tasks running in the group */
> unsigned int sum_h_nr_running; /* Nr of CFS tasks running in the group */
> unsigned int idle_cpus; /* Nr of idle CPUs in the group */
> + unsigned int asym_prefer_cpu; /* CPU with highest asym priority */
> + int highest_asym_prio; /* Asym priority of asym_prefer_cpu */
Its better to move this after group_asym_packing field, so all related
fields are together.
> unsigned int group_weight;
> enum group_type group_type;
> unsigned int group_asym_packing; /* Tasks should be moved to preferred CPU */
> @@ -10243,7 +10245,7 @@ sched_group_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group
> (sgs->group_weight - sgs->idle_cpus != 1))
> return false;
>
> - return sched_asym(env->sd, env->dst_cpu, group->asym_prefer_cpu);
> + return sched_asym(env->sd, env->dst_cpu, sgs->asym_prefer_cpu);
> }
>
> /* One group has more than one SMT CPU while the other group does not */
> @@ -10324,6 +10326,17 @@ sched_reduced_capacity(struct rq *rq, struct sched_domain *sd)
> return check_cpu_capacity(rq, sd);
> }
>
> +static inline void
> +update_sg_pick_asym_prefer(struct sg_lb_stats *sgs, int cpu)
> +{
> + int asym_prio = arch_asym_cpu_priority(cpu);
> +
> + if (asym_prio > sgs->highest_asym_prio) {
> + sgs->asym_prefer_cpu = cpu;
> + sgs->highest_asym_prio = asym_prio;
> + }
> +}
> +
> /**
> * update_sg_lb_stats - Update sched_group's statistics for load balancing.
> * @env: The load balancing environment.
> @@ -10345,6 +10358,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> memset(sgs, 0, sizeof(*sgs));
>
> local_group = group == sds->local;
> + sgs->highest_asym_prio = INT_MIN;
>
> for_each_cpu_and(i, sched_group_span(group), env->cpus) {
> struct rq *rq = cpu_rq(i);
> @@ -10358,6 +10372,9 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> nr_running = rq->nr_running;
> sgs->sum_nr_running += nr_running;
>
> + if (sd_flags & SD_ASYM_PACKING)
> + update_sg_pick_asym_prefer(sgs, i);
> +
> if (cpu_overutilized(i))
> *sg_overutilized = 1;
>
> @@ -10479,7 +10496,7 @@ static bool update_sd_pick_busiest(struct lb_env *env,
>
> case group_asym_packing:
> /* Prefer to move from lowest priority CPU's work */
> - return sched_asym_prefer(sds->busiest->asym_prefer_cpu, sg->asym_prefer_cpu);
> + return sched_asym_prefer(busiest->asym_prefer_cpu, sgs->asym_prefer_cpu);
>
> case group_misfit_task:
> /*
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index aef716c41edb..a3f0d326bd11 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2047,7 +2047,6 @@ struct sched_group {
> unsigned int group_weight;
> unsigned int cores;
> struct sched_group_capacity *sgc;
> - int asym_prefer_cpu; /* CPU of highest priority in group */
> int flags;
>
> /*
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 9c405f0e7b26..20aa087710f0 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1302,7 +1302,7 @@ static void init_sched_groups_capacity(int cpu, struct sched_domain *sd)
> WARN_ON(!sg);
>
> do {
> - int cpu, cores = 0, max_cpu = -1;
> + int cpu, cores = 0;
>
> sg->group_weight = cpumask_weight(sched_group_span(sg));
>
> @@ -1314,19 +1314,6 @@ static void init_sched_groups_capacity(int cpu, struct sched_domain *sd)
> #endif
> }
> sg->cores = cores;
> -
> - if (!(sd->flags & SD_ASYM_PACKING))
> - goto next;
> -
> - for_each_cpu(cpu, sched_group_span(sg)) {
> - if (max_cpu < 0)
> - max_cpu = cpu;
> - else if (sched_asym_prefer(cpu, max_cpu))
> - max_cpu = cpu;
> - }
> - sg->asym_prefer_cpu = max_cpu;
> -
> -next:
> sg = sg->next;
> } while (sg != sd->groups);
>
Tried minimal testing of ASYM_PACKING behavior on Power10 Shared VM. It
is working as expected with the patch as well. (functionality wise,
performance isn't tested)
Powered by blists - more mailing lists