| 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
| ||
|
Date: Tue, 27 Jun 2023 16:19:14 +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
Hey,
On Sunday 25 Jun 2023 at 13:11:55 (-0700), Ricardo Neri wrote:
> On Thu, Jun 22, 2023 at 10:02:44AM +0100, Ionela Voinescu wrote:
> > 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?
>
> Yes, I agree. `after` cannot be negative.
>
> >
> > I'm just wondering if ipcc_score_after can be made unsigned long as well,
> > just for consistency.
>
> Sure. I can make it of type unsigned long as well.
>
> >
> > > +
> > > + /* 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?
>
> No, because ipcc_score_after represents the joint score of the busiest
> core and the destination core after load balance has taken place. The
> destination core was previously idle and now contributes to the joint
> score.
>
Right! score_on_dst_cpu does not contribute to the per-cpu throughput of
the busiest core, but it reflects the improvement in score gained by the
move to the destination.
Thanks,
Ionela.
> Thanks and BR,
> Ricardo
Powered by blists - more mailing lists