| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZJQONIinvSengWa8@arm.com>
Date: Thu, 22 Jun 2023 10:02:44 +0100
From: Ionela Voinescu <ionela.voinescu@....com>
To: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
Cc: "Peter Zijlstra (Intel)" <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Ricardo Neri <ricardo.neri@...el.com>,
"Ravi V. Shankar" <ravi.v.shankar@...el.com>,
Ben Segall <bsegall@...gle.com>,
Daniel Bristot de Oliveira <bristot@...hat.com>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Len Brown <len.brown@...el.com>, Mel Gorman <mgorman@...e.de>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Steven Rostedt <rostedt@...dmis.org>,
Tim Chen <tim.c.chen@...ux.intel.com>,
Valentin Schneider <vschneid@...hat.com>,
Lukasz Luba <lukasz.luba@....com>,
Zhao Liu <zhao1.liu@...el.com>,
"Yuan, Perry" <Perry.Yuan@....com>, x86@...nel.org,
"Joel Fernandes (Google)" <joel@...lfernandes.org>,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
"Tim C . Chen" <tim.c.chen@...el.com>,
Zhao Liu <zhao1.liu@...ux.intel.com>
Subject: Re: [PATCH v4 07/24] sched/fair: Compute IPC class scores for load
balancing
On Monday 12 Jun 2023 at 21:24:05 (-0700), Ricardo Neri wrote:
> When using IPCC scores to break ties between two scheduling groups, it is
> necessary to consider both the current score and the score that would
> result after load balancing.
>
> Compute the combined IPC class score of a scheduling group and the local
> scheduling group. Compute both the current score and the prospective score.
>
> Collect IPCC statistics only for asym_packing and fully_busy scheduling
> groups. These are the only cases that use IPCC scores.
>
> These IPCC statistics are used during idle load balancing. The candidate
> scheduling group will have one fewer busy CPU after load balancing. This
> observation is important for cores with SMT support.
>
> The IPCC score of scheduling groups composed of SMT siblings needs to
> consider that the siblings share CPU resources. When computing the total
> IPCC score of the scheduling group, divide the score of each sibling by
> the number of busy siblings.
>
> Cc: Ben Segall <bsegall@...gle.com>
> Cc: Daniel Bristot de Oliveira <bristot@...hat.com>
> Cc: Dietmar Eggemann <dietmar.eggemann@....com>
> Cc: Ionela Voinescu <ionela.voinescu@....com>
> Cc: Joel Fernandes (Google) <joel@...lfernandes.org>
> Cc: Len Brown <len.brown@...el.com>
> Cc: Lukasz Luba <lukasz.luba@....com>
> Cc: Mel Gorman <mgorman@...e.de>
> Cc: Perry Yuan <Perry.Yuan@....com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Tim C. Chen <tim.c.chen@...el.com>
> Cc: Valentin Schneider <vschneid@...hat.com>
> Cc: Zhao Liu <zhao1.liu@...ux.intel.com>
> Cc: x86@...nel.org
> Cc: linux-pm@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
> ---
> Changes since v3:
> * None
>
> Changes since v2:
> * Also collect IPCC stats for fully_busy sched groups.
> * Restrict use of IPCC stats to SD_ASYM_PACKING. (Ionela)
> * Handle errors of arch_get_ipcc_score(). (Ionela)
>
> Changes since v1:
> * Implemented cleanups and reworks from PeterZ. I took all his
> suggestions, except the computation of the IPC score before and after
> load balancing. We are computing not the average score, but the *total*.
> * Check for the SD_SHARE_CPUCAPACITY to compute the throughput of the SMT
> siblings of a physical core.
> * Used the new interface names.
> * Reworded commit message for clarity.
> ---
> kernel/sched/fair.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index c0cab5e501b6..a51c65c9335f 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9114,6 +9114,8 @@ struct sg_lb_stats {
> unsigned long min_score; /* Min(score(rq->curr->ipcc)) */
> unsigned short min_ipcc; /* Class of the task with the minimum IPCC score in the rq */
> unsigned long sum_score; /* Sum(score(rq->curr->ipcc)) */
> + long ipcc_score_after; /* Prospective IPCC score after load balancing */
> + unsigned long ipcc_score_before; /* IPCC score before load balancing */
> #endif
> };
>
> @@ -9452,6 +9454,62 @@ static void update_sg_lb_ipcc_stats(int dst_cpu, struct sg_lb_stats *sgs,
> }
> }
>
> +static void update_sg_lb_stats_scores(struct sg_lb_stats *sgs,
> + struct sched_group *sg,
> + struct lb_env *env)
> +{
> + unsigned long score_on_dst_cpu, before;
> + int busy_cpus;
> + long after;
> +
> + if (!sched_ipcc_enabled())
> + return;
> +
> + /*
> + * IPCC scores are only useful during idle load balancing. For now,
> + * only asym_packing uses IPCC scores.
> + */
> + if (!(env->sd->flags & SD_ASYM_PACKING) ||
> + env->idle == CPU_NOT_IDLE)
> + return;
> +
> + /*
> + * IPCC scores are used to break ties only between these types of
> + * groups.
> + */
> + if (sgs->group_type != group_fully_busy &&
> + sgs->group_type != group_asym_packing)
> + return;
> +
> + busy_cpus = sgs->group_weight - sgs->idle_cpus;
> +
> + /* No busy CPUs in the group. No tasks to move. */
> + if (!busy_cpus)
> + return;
> +
> + score_on_dst_cpu = arch_get_ipcc_score(sgs->min_ipcc, env->dst_cpu);
> +
> + /*
> + * Do not use IPC scores. sgs::ipcc_score_{after, before} will be zero
> + * and not used.
> + */
> + if (IS_ERR_VALUE(score_on_dst_cpu))
> + return;
> +
> + before = sgs->sum_score;
> + after = before - sgs->min_score;
I don't believe this can end up being negative as the sum of all
scores should be higher or equal to the min score, right?
I'm just wondering if ipcc_score_after can be made unsigned long as well,
just for consistency.
> +
> + /* SMT siblings share throughput. */
> + if (busy_cpus > 1 && sg->flags & SD_SHARE_CPUCAPACITY) {
> + before /= busy_cpus;
> + /* One sibling will become idle after load balance. */
> + after /= busy_cpus - 1;
> + }
> +
> + sgs->ipcc_score_after = after + score_on_dst_cpu;
> + sgs->ipcc_score_before = before;
Shouldn't the score_on_dst_cpu be added to "after" before being divided
between the SMT siblings?
Thanks,
Ionela.
> +}
> +
> #else /* CONFIG_IPC_CLASSES */
> static void update_sg_lb_ipcc_stats(int dst_cpu, struct sg_lb_stats *sgs,
> struct rq *rq)
> @@ -9461,6 +9519,13 @@ static void update_sg_lb_ipcc_stats(int dst_cpu, struct sg_lb_stats *sgs,
> static void init_rq_ipcc_stats(struct sg_lb_stats *sgs)
> {
> }
> +
> +static void update_sg_lb_stats_scores(struct sg_lb_stats *sgs,
> + struct sched_group *sg,
> + struct lb_env *env)
> +{
> +}
> +
> #endif /* CONFIG_IPC_CLASSES */
>
> /**
> @@ -9620,6 +9685,9 @@ static inline void update_sg_lb_stats(struct lb_env *env,
>
> sgs->group_type = group_classify(env->sd->imbalance_pct, group, sgs);
>
> + if (!local_group)
> + update_sg_lb_stats_scores(sgs, group, env);
> +
> /* Computing avg_load makes sense only when group is overloaded */
> if (sgs->group_type == group_overloaded)
> sgs->avg_load = (sgs->group_load * SCHED_CAPACITY_SCALE) /
> --
> 2.25.1
>
Powered by blists - more mailing lists