2024-02-10 11:36:30

by alexs

[permalink] [raw]
Subject: [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS

From: Alex Shi <[email protected]>

These flags are already documented in include/linux/sched/sd_flags.h.
Also, add missing SD_CLUSTER and keep the comment on SD_ASYM_PACKING
as it is a special case.

Suggested-by: Ricardo Neri <[email protected]>
Signed-off-by: Alex Shi <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
---
kernel/sched/topology.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 10d1391e7416..0b33f7b05d21 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1551,11 +1551,12 @@ static struct cpumask ***sched_domains_numa_masks;
*
* These flags are purely descriptive of the topology and do not prescribe
* behaviour. Behaviour is artificial and mapped in the below sd_init()
- * function:
+ * function. For details, see include/linux/sched/sd_flags.h.
*
- * SD_SHARE_CPUCAPACITY - describes SMT topologies
- * SD_SHARE_PKG_RESOURCES - describes shared caches
- * SD_NUMA - describes NUMA topologies
+ * SD_SHARE_CPUCAPACITY
+ * SD_SHARE_PKG_RESOURCES
+ * SD_CLUSTER
+ * SD_NUMA
*
* Odd one out, which beside describing the topology has a quirk also
* prescribes the desired behaviour that goes along with it:
--
2.43.0



2024-02-10 11:36:43

by alexs

[permalink] [raw]
Subject: [PATCH v5 2/5] sched/fair: Remove unused parameters

From: Alex Shi <[email protected]>

The argument sds is not used in function sched_asym(). Remove it.

Fixes: c9ca07886aaa ("sched/fair: Do not even the number of busy CPUs via asym_packing")
Signed-off-by: Alex Shi <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
---
kernel/sched/fair.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 533547e3c90a..607dc310b355 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9749,7 +9749,6 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
/**
* sched_asym - Check if the destination CPU can do asym_packing load balance
* @env: The load balancing environment
- * @sds: Load-balancing data with statistics of the local group
* @sgs: Load-balancing statistics of the candidate busiest group
* @group: The candidate busiest group
*
@@ -9768,8 +9767,7 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
* otherwise.
*/
static inline bool
-sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs,
- struct sched_group *group)
+sched_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
{
/* Ensure that the whole local core is idle, if applicable. */
if (!sched_use_asym_prio(env->sd, env->dst_cpu))
@@ -9940,7 +9938,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
/* Check if dst CPU is idle and preferred to this group */
if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
- sched_asym(env, sds, sgs, group)) {
+ sched_asym(env, sgs, group)) {
sgs->group_asym_packing = 1;
}

--
2.43.0


2024-02-10 11:36:56

by alexs

[permalink] [raw]
Subject: [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()

From: Alex Shi <[email protected]>

sched_use_asym_prio() and sched_asym_prefer() are used together in various
places. Consolidate them into a single function sched_asym().

The existing sched_group_asym() is only used when collecting statistics
of a scheduling group. Rename it as sched_group_asym(), and remove the
obsolete function description.

This makes the code easier to read. No functional changes.

Tested-by: Ricardo Neri <[email protected]>
Signed-off-by: Alex Shi <[email protected]>
Cc: Ricardo Neri <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
---
kernel/sched/fair.c | 45 ++++++++++++++++++++-------------------------
1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 607dc310b355..426eda9eda57 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9746,8 +9746,18 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
return sd->flags & SD_SHARE_CPUCAPACITY || is_core_idle(cpu);
}

+static inline bool sched_asym(struct sched_domain *sd, int dst_cpu, int src_cpu)
+{
+ /*
+ * First check if @dst_cpu can do asym_packing load balance. Only do it
+ * if it has higher priority than @src_cpu.
+ */
+ return sched_use_asym_prio(sd, dst_cpu) &&
+ sched_asym_prefer(dst_cpu, src_cpu);
+}
+
/**
- * sched_asym - Check if the destination CPU can do asym_packing load balance
+ * sched_group_asym - Check if the destination CPU can do asym_packing balance
* @env: The load balancing environment
* @sgs: Load-balancing statistics of the candidate busiest group
* @group: The candidate busiest group
@@ -9755,34 +9765,21 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
* @env::dst_cpu can do asym_packing if it has higher priority than the
* preferred CPU of @group.
*
- * SMT is a special case. If we are balancing load between cores, @env::dst_cpu
- * can do asym_packing balance only if all its SMT siblings are idle. Also, it
- * can only do it if @group is an SMT group and has exactly on busy CPU. Larger
- * imbalances in the number of CPUS are dealt with in find_busiest_group().
- *
- * If we are balancing load within an SMT core, or at PKG domain level, always
- * proceed.
- *
* Return: true if @env::dst_cpu can do with asym_packing load balance. False
* otherwise.
*/
static inline bool
-sched_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
+sched_group_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
{
- /* Ensure that the whole local core is idle, if applicable. */
- if (!sched_use_asym_prio(env->sd, env->dst_cpu))
- return false;
-
/*
- * CPU priorities does not make sense for SMT cores with more than one
+ * CPU priorities do not make sense for SMT cores with more than one
* busy sibling.
*/
- if (group->flags & SD_SHARE_CPUCAPACITY) {
- if (sgs->group_weight - sgs->idle_cpus != 1)
- return false;
- }
+ if ((group->flags & SD_SHARE_CPUCAPACITY) &&
+ (sgs->group_weight - sgs->idle_cpus != 1))
+ return false;

- return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
+ return sched_asym(env->sd, env->dst_cpu, group->asym_prefer_cpu);
}

/* One group has more than one SMT CPU while the other group does not */
@@ -9938,7 +9935,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
/* Check if dst CPU is idle and preferred to this group */
if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
- sched_asym(env, sgs, group)) {
+ sched_group_asym(env, sgs, group)) {
sgs->group_asym_packing = 1;
}

@@ -11037,8 +11034,7 @@ static struct rq *find_busiest_queue(struct lb_env *env,
* SMT cores with more than one busy sibling.
*/
if ((env->sd->flags & SD_ASYM_PACKING) &&
- sched_use_asym_prio(env->sd, i) &&
- sched_asym_prefer(i, env->dst_cpu) &&
+ sched_asym(env->sd, i, env->dst_cpu) &&
nr_running == 1)
continue;

@@ -11908,8 +11904,7 @@ static void nohz_balancer_kick(struct rq *rq)
* preferred CPU must be idle.
*/
for_each_cpu_and(i, sched_domain_span(sd), nohz.idle_cpus_mask) {
- if (sched_use_asym_prio(sd, i) &&
- sched_asym_prefer(i, cpu)) {
+ if (sched_asym(sd, i, cpu)) {
flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}
--
2.43.0


2024-02-10 11:37:11

by alexs

[permalink] [raw]
Subject: [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio()

From: Alex Shi <[email protected]>

sched_use_asym_prio() checks whether CPU priorities should be used. It
makes sense to check for the SD_ASYM_PACKING() inside the function.
Since both sched_asym() and sched_group_asym() use sched_use_asym_prio(),
remove the now superfluous checks for the flag in various places.

Tested-by: Ricardo Neri <[email protected]>
Signed-off-by: Alex Shi <[email protected]>
Cc: [email protected]
Cc: Ricardo Neri <[email protected]>
Cc: Ben Segall <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Daniel Bristot de Oliveira <[email protected]>
Cc: Vincent Guittot <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
---
kernel/sched/fair.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 426eda9eda57..cd1ec57c0b7b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9740,6 +9740,9 @@ group_type group_classify(unsigned int imbalance_pct,
*/
static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
{
+ if (!(sd->flags & SD_ASYM_PACKING))
+ return false;
+
if (!sched_smt_active())
return true;

@@ -9933,11 +9936,9 @@ static inline void update_sg_lb_stats(struct lb_env *env,
sgs->group_weight = group->group_weight;

/* Check if dst CPU is idle and preferred to this group */
- if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
- env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
- sched_group_asym(env, sgs, group)) {
+ if (!local_group && env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
+ sched_group_asym(env, sgs, group))
sgs->group_asym_packing = 1;
- }

/* Check for loaded SMT group to be balanced to dst CPU */
if (!local_group && smt_balance(env, sgs, group))
@@ -11033,9 +11034,7 @@ static struct rq *find_busiest_queue(struct lb_env *env,
* If balancing between cores, let lower priority CPUs help
* SMT cores with more than one busy sibling.
*/
- if ((env->sd->flags & SD_ASYM_PACKING) &&
- sched_asym(env->sd, i, env->dst_cpu) &&
- nr_running == 1)
+ if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1)
continue;

switch (env->migration_type) {
@@ -11131,8 +11130,7 @@ asym_active_balance(struct lb_env *env)
* the lower priority @env::dst_cpu help it. Do not follow
* CPU priority.
*/
- return env->idle != CPU_NOT_IDLE && (env->sd->flags & SD_ASYM_PACKING) &&
- sched_use_asym_prio(env->sd, env->dst_cpu) &&
+ return env->idle != CPU_NOT_IDLE && sched_use_asym_prio(env->sd, env->dst_cpu) &&
(sched_asym_prefer(env->dst_cpu, env->src_cpu) ||
!sched_use_asym_prio(env->sd, env->src_cpu));
}
--
2.43.0


2024-02-10 11:37:24

by alexs

[permalink] [raw]
Subject: [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC

From: Alex Shi <[email protected]>

SD_CLUSTER shares the CPU resources like llc tags or l2 cache, that's
easy confuse with SD_SHARE_PKG_RESOURCES. So let's specifical point
what the latter shares: LLC. That would reduce some confusing.

Suggested-by: Valentin Schneider <[email protected]>
Signed-off-by: Alex Shi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Miaohe Lin <[email protected]>
Cc: Barry Song <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Daniel Bristot de Oliveira <[email protected]>
Cc: Ben Segall <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Dietmar Eggemann <[email protected]>
Cc: Juri Lelli <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "Naveen N. Rao" <[email protected]>
Cc: "Aneesh Kumar K.V" <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: "Gautham R. Shenoy" <[email protected]>
Cc: Yicong Yang <[email protected]>
Cc: Ricardo Neri <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Valentin Schneider <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Michael Ellerman <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
---
arch/powerpc/kernel/smp.c | 6 +++---
include/linux/sched/sd_flags.h | 4 ++--
include/linux/sched/topology.h | 6 +++---
kernel/sched/fair.c | 2 +-
kernel/sched/topology.c | 28 ++++++++++++++--------------
5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 693334c20d07..a60e4139214b 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -984,7 +984,7 @@ static bool shared_caches __ro_after_init;
/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
- int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;

if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
@@ -1010,9 +1010,9 @@ static __ro_after_init DEFINE_STATIC_KEY_FALSE(splpar_asym_pack);
static int powerpc_shared_cache_flags(void)
{
if (static_branch_unlikely(&splpar_asym_pack))
- return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
+ return SD_SHARE_LLC | SD_ASYM_PACKING;

- return SD_SHARE_PKG_RESOURCES;
+ return SD_SHARE_LLC;
}

static int powerpc_shared_proc_flags(void)
diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
index a8b28647aafc..b04a5d04dee9 100644
--- a/include/linux/sched/sd_flags.h
+++ b/include/linux/sched/sd_flags.h
@@ -117,13 +117,13 @@ SD_FLAG(SD_SHARE_CPUCAPACITY, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
SD_FLAG(SD_CLUSTER, SDF_NEEDS_GROUPS)

/*
- * Domain members share CPU package resources (i.e. caches)
+ * Domain members share CPU Last Level Caches
*
* SHARED_CHILD: Set from the base domain up until spanned CPUs no longer share
* the same cache(s).
* NEEDS_GROUPS: Caches are shared between groups.
*/
-SD_FLAG(SD_SHARE_PKG_RESOURCES, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
+SD_FLAG(SD_SHARE_LLC, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)

/*
* Only a single load balancing instance
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index a6e04b4a21d7..191b122158fb 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -38,21 +38,21 @@ extern const struct sd_flag_debug sd_flag_debug[];
#ifdef CONFIG_SCHED_SMT
static inline int cpu_smt_flags(void)
{
- return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;
}
#endif

#ifdef CONFIG_SCHED_CLUSTER
static inline int cpu_cluster_flags(void)
{
- return SD_CLUSTER | SD_SHARE_PKG_RESOURCES;
+ return SD_CLUSTER | SD_SHARE_LLC;
}
#endif

#ifdef CONFIG_SCHED_MC
static inline int cpu_core_flags(void)
{
- return SD_SHARE_PKG_RESOURCES;
+ return SD_SHARE_LLC;
}
#endif

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index cd1ec57c0b7b..da6c77d05d07 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10687,7 +10687,7 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s
*/
if (local->group_type == group_has_spare) {
if ((busiest->group_type > group_fully_busy) &&
- !(env->sd->flags & SD_SHARE_PKG_RESOURCES)) {
+ !(env->sd->flags & SD_SHARE_LLC)) {
/*
* If busiest is overloaded, try to fill spare
* capacity. This might end up creating spare capacity
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 0b33f7b05d21..99ea5986038c 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -657,13 +657,13 @@ static void destroy_sched_domains(struct sched_domain *sd)
}

/*
- * Keep a special pointer to the highest sched_domain that has
- * SD_SHARE_PKG_RESOURCE set (Last Level Cache Domain) for this
- * allows us to avoid some pointer chasing select_idle_sibling().
+ * Keep a special pointer to the highest sched_domain that has SD_SHARE_LLC set
+ * (Last Level Cache Domain) for this allows us to avoid some pointer chasing
+ * select_idle_sibling().
*
- * Also keep a unique ID per domain (we use the first CPU number in
- * the cpumask of the domain), this allows us to quickly tell if
- * two CPUs are in the same cache domain, see cpus_share_cache().
+ * Also keep a unique ID per domain (we use the first CPU number in the cpumask
+ * of the domain), this allows us to quickly tell if two CPUs are in the same
+ * cache domain, see cpus_share_cache().
*/
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DEFINE_PER_CPU(int, sd_llc_size);
@@ -684,7 +684,7 @@ static void update_top_cache_domain(int cpu)
int id = cpu;
int size = 1;

- sd = highest_flag_domain(cpu, SD_SHARE_PKG_RESOURCES);
+ sd = highest_flag_domain(cpu, SD_SHARE_LLC);
if (sd) {
id = cpumask_first(sched_domain_span(sd));
size = cpumask_weight(sched_domain_span(sd));
@@ -1554,7 +1554,7 @@ static struct cpumask ***sched_domains_numa_masks;
* function. For details, see include/linux/sched/sd_flags.h.
*
* SD_SHARE_CPUCAPACITY
- * SD_SHARE_PKG_RESOURCES
+ * SD_SHARE_LLC
* SD_CLUSTER
* SD_NUMA
*
@@ -1566,7 +1566,7 @@ static struct cpumask ***sched_domains_numa_masks;
#define TOPOLOGY_SD_FLAGS \
(SD_SHARE_CPUCAPACITY | \
SD_CLUSTER | \
- SD_SHARE_PKG_RESOURCES | \
+ SD_SHARE_LLC | \
SD_NUMA | \
SD_ASYM_PACKING)

@@ -1609,7 +1609,7 @@ sd_init(struct sched_domain_topology_level *tl,
| 0*SD_BALANCE_WAKE
| 1*SD_WAKE_AFFINE
| 0*SD_SHARE_CPUCAPACITY
- | 0*SD_SHARE_PKG_RESOURCES
+ | 0*SD_SHARE_LLC
| 0*SD_SERIALIZE
| 1*SD_PREFER_SIBLING
| 0*SD_NUMA
@@ -1646,7 +1646,7 @@ sd_init(struct sched_domain_topology_level *tl,
if (sd->flags & SD_SHARE_CPUCAPACITY) {
sd->imbalance_pct = 110;

- } else if (sd->flags & SD_SHARE_PKG_RESOURCES) {
+ } else if (sd->flags & SD_SHARE_LLC) {
sd->imbalance_pct = 117;
sd->cache_nice_tries = 1;

@@ -1671,7 +1671,7 @@ sd_init(struct sched_domain_topology_level *tl,
* For all levels sharing cache; connect a sched_domain_shared
* instance.
*/
- if (sd->flags & SD_SHARE_PKG_RESOURCES) {
+ if (sd->flags & SD_SHARE_LLC) {
sd->shared = *per_cpu_ptr(sdd->sds, sd_id);
atomic_inc(&sd->shared->ref);
atomic_set(&sd->shared->nr_busy_cpus, sd_weight);
@@ -2446,8 +2446,8 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
struct sched_domain *child = sd->child;

- if (!(sd->flags & SD_SHARE_PKG_RESOURCES) && child &&
- (child->flags & SD_SHARE_PKG_RESOURCES)) {
+ if (!(sd->flags & SD_SHARE_LLC) && child &&
+ (child->flags & SD_SHARE_LLC)) {
struct sched_domain __rcu *top_p;
unsigned int nr_llcs;

--
2.43.0


2024-02-13 07:01:40

by Barry Song

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC

Hi Alex, Valentin,


On Sun, Feb 11, 2024 at 12:37 AM <[email protected]> wrote:
>
> From: Alex Shi <[email protected]>
>
> SD_CLUSTER shares the CPU resources like llc tags or l2 cache, that's
> easy confuse with SD_SHARE_PKG_RESOURCES. So let's specifical point
> what the latter shares: LLC. That would reduce some confusing.

On neither JACOBSVILLE nor kunpeng920, it seems CLUSTER isn't LLC.
on Jacobsville, cluster is L2-cache while Jacobsville has L3; on kunpeng920,
cluster is L3-tag. On kunpeng920, actually 24 cpus or 32cpus share one LLC,
the whole L3. cluster is kind of like middle-level caches.

So I feel this patch isn't precise.

>
> Suggested-by: Valentin Schneider <[email protected]>
> Signed-off-by: Alex Shi <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Miaohe Lin <[email protected]>
> Cc: Barry Song <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Frederic Weisbecker <[email protected]>
> Cc: Daniel Bristot de Oliveira <[email protected]>
> Cc: Ben Segall <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Dietmar Eggemann <[email protected]>
> Cc: Juri Lelli <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "Naveen N. Rao" <[email protected]>
> Cc: "Aneesh Kumar K.V" <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: "Gautham R. Shenoy" <[email protected]>
> Cc: Yicong Yang <[email protected]>
> Cc: Ricardo Neri <[email protected]>
> Cc: Josh Poimboeuf <[email protected]>
> Cc: Srikar Dronamraju <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Reviewed-by: Valentin Schneider <[email protected]>
> Reviewed-by: Ricardo Neri <[email protected]>
> ---
> arch/powerpc/kernel/smp.c | 6 +++---
> include/linux/sched/sd_flags.h | 4 ++--
> include/linux/sched/topology.h | 6 +++---
> kernel/sched/fair.c | 2 +-
> kernel/sched/topology.c | 28 ++++++++++++++--------------
> 5 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 693334c20d07..a60e4139214b 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -984,7 +984,7 @@ static bool shared_caches __ro_after_init;
> /* cpumask of CPUs with asymmetric SMT dependency */
> static int powerpc_smt_flags(void)
> {
> - int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> + int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;
>
> if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
> @@ -1010,9 +1010,9 @@ static __ro_after_init DEFINE_STATIC_KEY_FALSE(splpar_asym_pack);
> static int powerpc_shared_cache_flags(void)
> {
> if (static_branch_unlikely(&splpar_asym_pack))
> - return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> + return SD_SHARE_LLC | SD_ASYM_PACKING;
>
> - return SD_SHARE_PKG_RESOURCES;
> + return SD_SHARE_LLC;
> }
>
> static int powerpc_shared_proc_flags(void)
> diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
> index a8b28647aafc..b04a5d04dee9 100644
> --- a/include/linux/sched/sd_flags.h
> +++ b/include/linux/sched/sd_flags.h
> @@ -117,13 +117,13 @@ SD_FLAG(SD_SHARE_CPUCAPACITY, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
> SD_FLAG(SD_CLUSTER, SDF_NEEDS_GROUPS)
>
> /*
> - * Domain members share CPU package resources (i.e. caches)
> + * Domain members share CPU Last Level Caches
> *
> * SHARED_CHILD: Set from the base domain up until spanned CPUs no longer share
> * the same cache(s).
> * NEEDS_GROUPS: Caches are shared between groups.
> */
> -SD_FLAG(SD_SHARE_PKG_RESOURCES, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
> +SD_FLAG(SD_SHARE_LLC, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
>
> /*
> * Only a single load balancing instance
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index a6e04b4a21d7..191b122158fb 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -38,21 +38,21 @@ extern const struct sd_flag_debug sd_flag_debug[];
> #ifdef CONFIG_SCHED_SMT
> static inline int cpu_smt_flags(void)
> {
> - return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> + return SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;
> }
> #endif
>
> #ifdef CONFIG_SCHED_CLUSTER
> static inline int cpu_cluster_flags(void)
> {
> - return SD_CLUSTER | SD_SHARE_PKG_RESOURCES;
> + return SD_CLUSTER | SD_SHARE_LLC;
> }
> #endif
>
> #ifdef CONFIG_SCHED_MC
> static inline int cpu_core_flags(void)
> {
> - return SD_SHARE_PKG_RESOURCES;
> + return SD_SHARE_LLC;
> }
> #endif
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index cd1ec57c0b7b..da6c77d05d07 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -10687,7 +10687,7 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s
> */
> if (local->group_type == group_has_spare) {
> if ((busiest->group_type > group_fully_busy) &&
> - !(env->sd->flags & SD_SHARE_PKG_RESOURCES)) {
> + !(env->sd->flags & SD_SHARE_LLC)) {
> /*
> * If busiest is overloaded, try to fill spare
> * capacity. This might end up creating spare capacity
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 0b33f7b05d21..99ea5986038c 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -657,13 +657,13 @@ static void destroy_sched_domains(struct sched_domain *sd)
> }
>
> /*
> - * Keep a special pointer to the highest sched_domain that has
> - * SD_SHARE_PKG_RESOURCE set (Last Level Cache Domain) for this
> - * allows us to avoid some pointer chasing select_idle_sibling().
> + * Keep a special pointer to the highest sched_domain that has SD_SHARE_LLC set
> + * (Last Level Cache Domain) for this allows us to avoid some pointer chasing
> + * select_idle_sibling().
> *
> - * Also keep a unique ID per domain (we use the first CPU number in
> - * the cpumask of the domain), this allows us to quickly tell if
> - * two CPUs are in the same cache domain, see cpus_share_cache().
> + * Also keep a unique ID per domain (we use the first CPU number in the cpumask
> + * of the domain), this allows us to quickly tell if two CPUs are in the same
> + * cache domain, see cpus_share_cache().
> */
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
> DEFINE_PER_CPU(int, sd_llc_size);
> @@ -684,7 +684,7 @@ static void update_top_cache_domain(int cpu)
> int id = cpu;
> int size = 1;
>
> - sd = highest_flag_domain(cpu, SD_SHARE_PKG_RESOURCES);
> + sd = highest_flag_domain(cpu, SD_SHARE_LLC);
> if (sd) {
> id = cpumask_first(sched_domain_span(sd));
> size = cpumask_weight(sched_domain_span(sd));
> @@ -1554,7 +1554,7 @@ static struct cpumask ***sched_domains_numa_masks;
> * function. For details, see include/linux/sched/sd_flags.h.
> *
> * SD_SHARE_CPUCAPACITY
> - * SD_SHARE_PKG_RESOURCES
> + * SD_SHARE_LLC
> * SD_CLUSTER
> * SD_NUMA
> *
> @@ -1566,7 +1566,7 @@ static struct cpumask ***sched_domains_numa_masks;
> #define TOPOLOGY_SD_FLAGS \
> (SD_SHARE_CPUCAPACITY | \
> SD_CLUSTER | \
> - SD_SHARE_PKG_RESOURCES | \
> + SD_SHARE_LLC | \
> SD_NUMA | \
> SD_ASYM_PACKING)
>
> @@ -1609,7 +1609,7 @@ sd_init(struct sched_domain_topology_level *tl,
> | 0*SD_BALANCE_WAKE
> | 1*SD_WAKE_AFFINE
> | 0*SD_SHARE_CPUCAPACITY
> - | 0*SD_SHARE_PKG_RESOURCES
> + | 0*SD_SHARE_LLC
> | 0*SD_SERIALIZE
> | 1*SD_PREFER_SIBLING
> | 0*SD_NUMA
> @@ -1646,7 +1646,7 @@ sd_init(struct sched_domain_topology_level *tl,
> if (sd->flags & SD_SHARE_CPUCAPACITY) {
> sd->imbalance_pct = 110;
>
> - } else if (sd->flags & SD_SHARE_PKG_RESOURCES) {
> + } else if (sd->flags & SD_SHARE_LLC) {
> sd->imbalance_pct = 117;
> sd->cache_nice_tries = 1;
>
> @@ -1671,7 +1671,7 @@ sd_init(struct sched_domain_topology_level *tl,
> * For all levels sharing cache; connect a sched_domain_shared
> * instance.
> */
> - if (sd->flags & SD_SHARE_PKG_RESOURCES) {
> + if (sd->flags & SD_SHARE_LLC) {
> sd->shared = *per_cpu_ptr(sdd->sds, sd_id);
> atomic_inc(&sd->shared->ref);
> atomic_set(&sd->shared->nr_busy_cpus, sd_weight);
> @@ -2446,8 +2446,8 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
> for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
> struct sched_domain *child = sd->child;
>
> - if (!(sd->flags & SD_SHARE_PKG_RESOURCES) && child &&
> - (child->flags & SD_SHARE_PKG_RESOURCES)) {
> + if (!(sd->flags & SD_SHARE_LLC) && child &&
> + (child->flags & SD_SHARE_LLC)) {
> struct sched_domain __rcu *top_p;
> unsigned int nr_llcs;
>
> --
> 2.43.0
>
>

Thanks
Barry

2024-02-13 07:21:50

by Barry Song

[permalink] [raw]
Subject: Re: [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC

On Tue, Feb 13, 2024 at 8:01 PM Barry Song <[email protected]> wrote:
>
> Hi Alex, Valentin,
>
>
> On Sun, Feb 11, 2024 at 12:37 AM <[email protected]> wrote:
> >
> > From: Alex Shi <[email protected]>
> >
> > SD_CLUSTER shares the CPU resources like llc tags or l2 cache, that's
> > easy confuse with SD_SHARE_PKG_RESOURCES. So let's specifical point
> > what the latter shares: LLC. That would reduce some confusing.
>
> On neither JACOBSVILLE nor kunpeng920, it seems CLUSTER isn't LLC.
> on Jacobsville, cluster is L2-cache while Jacobsville has L3; on kunpeng920,
> cluster is L3-tag. On kunpeng920, actually 24 cpus or 32cpus share one LLC,
> the whole L3. cluster is kind of like middle-level caches.
>
> So I feel this patch isn't precise.

sorry for my noise, i thought you were renaming cluster to LLC. but after
second reading, you are renaming the level after cluster, so my comment
was wrong. Please feel free to add:

Reviewed-by: Barry Song <[email protected]>

>
> >
> > Suggested-by: Valentin Schneider <[email protected]>
> > Signed-off-by: Alex Shi <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: Miaohe Lin <[email protected]>
> > Cc: Barry Song <[email protected]>
> > Cc: Mark Rutland <[email protected]>
> > Cc: Frederic Weisbecker <[email protected]>
> > Cc: Daniel Bristot de Oliveira <[email protected]>
> > Cc: Ben Segall <[email protected]>
> > Cc: Steven Rostedt <[email protected]>
> > Cc: Dietmar Eggemann <[email protected]>
> > Cc: Juri Lelli <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: "Naveen N. Rao" <[email protected]>
> > Cc: "Aneesh Kumar K.V" <[email protected]>
> > Cc: Christophe Leroy <[email protected]>
> > Cc: "Gautham R. Shenoy" <[email protected]>
> > Cc: Yicong Yang <[email protected]>
> > Cc: Ricardo Neri <[email protected]>
> > Cc: Josh Poimboeuf <[email protected]>
> > Cc: Srikar Dronamraju <[email protected]>
> > Cc: Valentin Schneider <[email protected]>
> > Cc: Nicholas Piggin <[email protected]>
> > Cc: Michael Ellerman <[email protected]>
> > Reviewed-by: Valentin Schneider <[email protected]>
> > Reviewed-by: Ricardo Neri <[email protected]>
> > ---
> > arch/powerpc/kernel/smp.c | 6 +++---
> > include/linux/sched/sd_flags.h | 4 ++--
> > include/linux/sched/topology.h | 6 +++---
> > kernel/sched/fair.c | 2 +-
> > kernel/sched/topology.c | 28 ++++++++++++++--------------
> > 5 files changed, 23 insertions(+), 23 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> > index 693334c20d07..a60e4139214b 100644
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -984,7 +984,7 @@ static bool shared_caches __ro_after_init;
> > /* cpumask of CPUs with asymmetric SMT dependency */
> > static int powerpc_smt_flags(void)
> > {
> > - int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> > + int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;
> >
> > if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
> > printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
> > @@ -1010,9 +1010,9 @@ static __ro_after_init DEFINE_STATIC_KEY_FALSE(splpar_asym_pack);
> > static int powerpc_shared_cache_flags(void)
> > {
> > if (static_branch_unlikely(&splpar_asym_pack))
> > - return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
> > + return SD_SHARE_LLC | SD_ASYM_PACKING;
> >
> > - return SD_SHARE_PKG_RESOURCES;
> > + return SD_SHARE_LLC;
> > }
> >
> > static int powerpc_shared_proc_flags(void)
> > diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
> > index a8b28647aafc..b04a5d04dee9 100644
> > --- a/include/linux/sched/sd_flags.h
> > +++ b/include/linux/sched/sd_flags.h
> > @@ -117,13 +117,13 @@ SD_FLAG(SD_SHARE_CPUCAPACITY, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
> > SD_FLAG(SD_CLUSTER, SDF_NEEDS_GROUPS)
> >
> > /*
> > - * Domain members share CPU package resources (i.e. caches)
> > + * Domain members share CPU Last Level Caches
> > *
> > * SHARED_CHILD: Set from the base domain up until spanned CPUs no longer share
> > * the same cache(s).
> > * NEEDS_GROUPS: Caches are shared between groups.
> > */
> > -SD_FLAG(SD_SHARE_PKG_RESOURCES, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
> > +SD_FLAG(SD_SHARE_LLC, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
> >
> > /*
> > * Only a single load balancing instance
> > diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> > index a6e04b4a21d7..191b122158fb 100644
> > --- a/include/linux/sched/topology.h
> > +++ b/include/linux/sched/topology.h
> > @@ -38,21 +38,21 @@ extern const struct sd_flag_debug sd_flag_debug[];
> > #ifdef CONFIG_SCHED_SMT
> > static inline int cpu_smt_flags(void)
> > {
> > - return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
> > + return SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;
> > }
> > #endif
> >
> > #ifdef CONFIG_SCHED_CLUSTER
> > static inline int cpu_cluster_flags(void)
> > {
> > - return SD_CLUSTER | SD_SHARE_PKG_RESOURCES;
> > + return SD_CLUSTER | SD_SHARE_LLC;
> > }
> > #endif
> >
> > #ifdef CONFIG_SCHED_MC
> > static inline int cpu_core_flags(void)
> > {
> > - return SD_SHARE_PKG_RESOURCES;
> > + return SD_SHARE_LLC;
> > }
> > #endif
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index cd1ec57c0b7b..da6c77d05d07 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -10687,7 +10687,7 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s
> > */
> > if (local->group_type == group_has_spare) {
> > if ((busiest->group_type > group_fully_busy) &&
> > - !(env->sd->flags & SD_SHARE_PKG_RESOURCES)) {
> > + !(env->sd->flags & SD_SHARE_LLC)) {
> > /*
> > * If busiest is overloaded, try to fill spare
> > * capacity. This might end up creating spare capacity
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 0b33f7b05d21..99ea5986038c 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -657,13 +657,13 @@ static void destroy_sched_domains(struct sched_domain *sd)
> > }
> >
> > /*
> > - * Keep a special pointer to the highest sched_domain that has
> > - * SD_SHARE_PKG_RESOURCE set (Last Level Cache Domain) for this
> > - * allows us to avoid some pointer chasing select_idle_sibling().
> > + * Keep a special pointer to the highest sched_domain that has SD_SHARE_LLC set
> > + * (Last Level Cache Domain) for this allows us to avoid some pointer chasing
> > + * select_idle_sibling().
> > *
> > - * Also keep a unique ID per domain (we use the first CPU number in
> > - * the cpumask of the domain), this allows us to quickly tell if
> > - * two CPUs are in the same cache domain, see cpus_share_cache().
> > + * Also keep a unique ID per domain (we use the first CPU number in the cpumask
> > + * of the domain), this allows us to quickly tell if two CPUs are in the same
> > + * cache domain, see cpus_share_cache().
> > */
> > DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
> > DEFINE_PER_CPU(int, sd_llc_size);
> > @@ -684,7 +684,7 @@ static void update_top_cache_domain(int cpu)
> > int id = cpu;
> > int size = 1;
> >
> > - sd = highest_flag_domain(cpu, SD_SHARE_PKG_RESOURCES);
> > + sd = highest_flag_domain(cpu, SD_SHARE_LLC);
> > if (sd) {
> > id = cpumask_first(sched_domain_span(sd));
> > size = cpumask_weight(sched_domain_span(sd));
> > @@ -1554,7 +1554,7 @@ static struct cpumask ***sched_domains_numa_masks;
> > * function. For details, see include/linux/sched/sd_flags.h.
> > *
> > * SD_SHARE_CPUCAPACITY
> > - * SD_SHARE_PKG_RESOURCES
> > + * SD_SHARE_LLC
> > * SD_CLUSTER
> > * SD_NUMA
> > *
> > @@ -1566,7 +1566,7 @@ static struct cpumask ***sched_domains_numa_masks;
> > #define TOPOLOGY_SD_FLAGS \
> > (SD_SHARE_CPUCAPACITY | \
> > SD_CLUSTER | \
> > - SD_SHARE_PKG_RESOURCES | \
> > + SD_SHARE_LLC | \
> > SD_NUMA | \
> > SD_ASYM_PACKING)
> >
> > @@ -1609,7 +1609,7 @@ sd_init(struct sched_domain_topology_level *tl,
> > | 0*SD_BALANCE_WAKE
> > | 1*SD_WAKE_AFFINE
> > | 0*SD_SHARE_CPUCAPACITY
> > - | 0*SD_SHARE_PKG_RESOURCES
> > + | 0*SD_SHARE_LLC
> > | 0*SD_SERIALIZE
> > | 1*SD_PREFER_SIBLING
> > | 0*SD_NUMA
> > @@ -1646,7 +1646,7 @@ sd_init(struct sched_domain_topology_level *tl,
> > if (sd->flags & SD_SHARE_CPUCAPACITY) {
> > sd->imbalance_pct = 110;
> >
> > - } else if (sd->flags & SD_SHARE_PKG_RESOURCES) {
> > + } else if (sd->flags & SD_SHARE_LLC) {
> > sd->imbalance_pct = 117;
> > sd->cache_nice_tries = 1;
> >
> > @@ -1671,7 +1671,7 @@ sd_init(struct sched_domain_topology_level *tl,
> > * For all levels sharing cache; connect a sched_domain_shared
> > * instance.
> > */
> > - if (sd->flags & SD_SHARE_PKG_RESOURCES) {
> > + if (sd->flags & SD_SHARE_LLC) {
> > sd->shared = *per_cpu_ptr(sdd->sds, sd_id);
> > atomic_inc(&sd->shared->ref);
> > atomic_set(&sd->shared->nr_busy_cpus, sd_weight);
> > @@ -2446,8 +2446,8 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
> > for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
> > struct sched_domain *child = sd->child;
> >
> > - if (!(sd->flags & SD_SHARE_PKG_RESOURCES) && child &&
> > - (child->flags & SD_SHARE_PKG_RESOURCES)) {
> > + if (!(sd->flags & SD_SHARE_LLC) && child &&
> > + (child->flags & SD_SHARE_LLC)) {
> > struct sched_domain __rcu *top_p;
> > unsigned int nr_llcs;
> >
> > --
> > 2.43.0
> >
> >
>
> Thanks
> Barry

2024-02-13 16:46:03

by Vincent Guittot

[permalink] [raw]
Subject: Re: [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS

On Sat, 10 Feb 2024 at 12:36, <[email protected]> wrote:
>
> From: Alex Shi <[email protected]>
>
> These flags are already documented in include/linux/sched/sd_flags.h.
> Also, add missing SD_CLUSTER and keep the comment on SD_ASYM_PACKING
> as it is a special case.
>
> Suggested-by: Ricardo Neri <[email protected]>
> Signed-off-by: Alex Shi <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Vincent Guittot <[email protected]>
> Cc: Juri Lelli <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Reviewed-by: Ricardo Neri <[email protected]>
> Reviewed-by: Valentin Schneider <[email protected]>

Reviewed-by: Vincent Guittot <[email protected]>

> ---
> kernel/sched/topology.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 10d1391e7416..0b33f7b05d21 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1551,11 +1551,12 @@ static struct cpumask ***sched_domains_numa_masks;
> *
> * These flags are purely descriptive of the topology and do not prescribe
> * behaviour. Behaviour is artificial and mapped in the below sd_init()
> - * function:
> + * function. For details, see include/linux/sched/sd_flags.h.
> *
> - * SD_SHARE_CPUCAPACITY - describes SMT topologies
> - * SD_SHARE_PKG_RESOURCES - describes shared caches
> - * SD_NUMA - describes NUMA topologies
> + * SD_SHARE_CPUCAPACITY
> + * SD_SHARE_PKG_RESOURCES
> + * SD_CLUSTER
> + * SD_NUMA
> *
> * Odd one out, which beside describing the topology has a quirk also
> * prescribes the desired behaviour that goes along with it:
> --
> 2.43.0
>

2024-02-13 16:46:36

by Vincent Guittot

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] sched/fair: Remove unused parameters

On Sat, 10 Feb 2024 at 12:36, <[email protected]> wrote:
>
> From: Alex Shi <[email protected]>
>
> The argument sds is not used in function sched_asym(). Remove it.
>
> Fixes: c9ca07886aaa ("sched/fair: Do not even the number of busy CPUs via asym_packing")
> Signed-off-by: Alex Shi <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Vincent Guittot <[email protected]>
> Cc: Juri Lelli <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Reviewed-by: Ricardo Neri <[email protected]>
> Reviewed-by: Valentin Schneider <[email protected]>

Reviewed-by: Vincent Guittot <[email protected]>

> ---
> kernel/sched/fair.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 533547e3c90a..607dc310b355 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9749,7 +9749,6 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
> /**
> * sched_asym - Check if the destination CPU can do asym_packing load balance
> * @env: The load balancing environment
> - * @sds: Load-balancing data with statistics of the local group
> * @sgs: Load-balancing statistics of the candidate busiest group
> * @group: The candidate busiest group
> *
> @@ -9768,8 +9767,7 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
> * otherwise.
> */
> static inline bool
> -sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs,
> - struct sched_group *group)
> +sched_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
> {
> /* Ensure that the whole local core is idle, if applicable. */
> if (!sched_use_asym_prio(env->sd, env->dst_cpu))
> @@ -9940,7 +9938,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> /* Check if dst CPU is idle and preferred to this group */
> if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
> env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
> - sched_asym(env, sds, sgs, group)) {
> + sched_asym(env, sgs, group)) {
> sgs->group_asym_packing = 1;
> }
>
> --
> 2.43.0
>

2024-02-13 16:52:23

by Vincent Guittot

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()

On Sat, 10 Feb 2024 at 12:36, <[email protected]> wrote:
>
> From: Alex Shi <[email protected]>
>
> sched_use_asym_prio() and sched_asym_prefer() are used together in various
> places. Consolidate them into a single function sched_asym().
>
> The existing sched_group_asym() is only used when collecting statistics

nit: The existing sched_asym()

> of a scheduling group. Rename it as sched_group_asym(), and remove the
> obsolete function description.
>
> This makes the code easier to read. No functional changes.
>
> Tested-by: Ricardo Neri <[email protected]>
> Signed-off-by: Alex Shi <[email protected]>
> Cc: Ricardo Neri <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Vincent Guittot <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Reviewed-by: Ricardo Neri <[email protected]>

Reviewed-by: Vincent Guittot <[email protected]>

> ---
> kernel/sched/fair.c | 45 ++++++++++++++++++++-------------------------
> 1 file changed, 20 insertions(+), 25 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 607dc310b355..426eda9eda57 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9746,8 +9746,18 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
> return sd->flags & SD_SHARE_CPUCAPACITY || is_core_idle(cpu);
> }
>
> +static inline bool sched_asym(struct sched_domain *sd, int dst_cpu, int src_cpu)
> +{
> + /*
> + * First check if @dst_cpu can do asym_packing load balance. Only do it
> + * if it has higher priority than @src_cpu.
> + */
> + return sched_use_asym_prio(sd, dst_cpu) &&
> + sched_asym_prefer(dst_cpu, src_cpu);
> +}
> +
> /**
> - * sched_asym - Check if the destination CPU can do asym_packing load balance
> + * sched_group_asym - Check if the destination CPU can do asym_packing balance
> * @env: The load balancing environment
> * @sgs: Load-balancing statistics of the candidate busiest group
> * @group: The candidate busiest group
> @@ -9755,34 +9765,21 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
> * @env::dst_cpu can do asym_packing if it has higher priority than the
> * preferred CPU of @group.
> *
> - * SMT is a special case. If we are balancing load between cores, @env::dst_cpu
> - * can do asym_packing balance only if all its SMT siblings are idle. Also, it
> - * can only do it if @group is an SMT group and has exactly on busy CPU. Larger
> - * imbalances in the number of CPUS are dealt with in find_busiest_group().
> - *
> - * If we are balancing load within an SMT core, or at PKG domain level, always
> - * proceed.
> - *
> * Return: true if @env::dst_cpu can do with asym_packing load balance. False
> * otherwise.
> */
> static inline bool
> -sched_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
> +sched_group_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
> {
> - /* Ensure that the whole local core is idle, if applicable. */
> - if (!sched_use_asym_prio(env->sd, env->dst_cpu))
> - return false;
> -
> /*
> - * CPU priorities does not make sense for SMT cores with more than one
> + * CPU priorities do not make sense for SMT cores with more than one
> * busy sibling.
> */
> - if (group->flags & SD_SHARE_CPUCAPACITY) {
> - if (sgs->group_weight - sgs->idle_cpus != 1)
> - return false;
> - }
> + if ((group->flags & SD_SHARE_CPUCAPACITY) &&
> + (sgs->group_weight - sgs->idle_cpus != 1))
> + return false;
>
> - return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
> + return sched_asym(env->sd, env->dst_cpu, group->asym_prefer_cpu);
> }
>
> /* One group has more than one SMT CPU while the other group does not */
> @@ -9938,7 +9935,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> /* Check if dst CPU is idle and preferred to this group */
> if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
> env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
> - sched_asym(env, sgs, group)) {
> + sched_group_asym(env, sgs, group)) {
> sgs->group_asym_packing = 1;
> }
>
> @@ -11037,8 +11034,7 @@ static struct rq *find_busiest_queue(struct lb_env *env,
> * SMT cores with more than one busy sibling.
> */
> if ((env->sd->flags & SD_ASYM_PACKING) &&
> - sched_use_asym_prio(env->sd, i) &&
> - sched_asym_prefer(i, env->dst_cpu) &&
> + sched_asym(env->sd, i, env->dst_cpu) &&
> nr_running == 1)
> continue;
>
> @@ -11908,8 +11904,7 @@ static void nohz_balancer_kick(struct rq *rq)
> * preferred CPU must be idle.
> */
> for_each_cpu_and(i, sched_domain_span(sd), nohz.idle_cpus_mask) {
> - if (sched_use_asym_prio(sd, i) &&
> - sched_asym_prefer(i, cpu)) {
> + if (sched_asym(sd, i, cpu)) {
> flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
> goto unlock;
> }
> --
> 2.43.0
>

2024-02-13 17:02:23

by Vincent Guittot

[permalink] [raw]
Subject: Re: [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio()

On Sat, 10 Feb 2024 at 12:36, <[email protected]> wrote:
>
> From: Alex Shi <[email protected]>
>
> sched_use_asym_prio() checks whether CPU priorities should be used. It
> makes sense to check for the SD_ASYM_PACKING() inside the function.
> Since both sched_asym() and sched_group_asym() use sched_use_asym_prio(),
> remove the now superfluous checks for the flag in various places.
>
> Tested-by: Ricardo Neri <[email protected]>
> Signed-off-by: Alex Shi <[email protected]>
> Cc: [email protected]
> Cc: Ricardo Neri <[email protected]>
> Cc: Ben Segall <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Dietmar Eggemann <[email protected]>
> Cc: Valentin Schneider <[email protected]>
> Cc: Daniel Bristot de Oliveira <[email protected]>
> Cc: Vincent Guittot <[email protected]>
> Cc: Juri Lelli <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Reviewed-by: Ricardo Neri <[email protected]>

Reviewed-by: Vincent Guittot <[email protected]>

> ---
> kernel/sched/fair.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 426eda9eda57..cd1ec57c0b7b 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9740,6 +9740,9 @@ group_type group_classify(unsigned int imbalance_pct,
> */
> static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
> {
> + if (!(sd->flags & SD_ASYM_PACKING))
> + return false;
> +
> if (!sched_smt_active())
> return true;
>
> @@ -9933,11 +9936,9 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> sgs->group_weight = group->group_weight;
>
> /* Check if dst CPU is idle and preferred to this group */
> - if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
> - env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
> - sched_group_asym(env, sgs, group)) {
> + if (!local_group && env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
> + sched_group_asym(env, sgs, group))
> sgs->group_asym_packing = 1;
> - }
>
> /* Check for loaded SMT group to be balanced to dst CPU */
> if (!local_group && smt_balance(env, sgs, group))
> @@ -11033,9 +11034,7 @@ static struct rq *find_busiest_queue(struct lb_env *env,
> * If balancing between cores, let lower priority CPUs help
> * SMT cores with more than one busy sibling.
> */
> - if ((env->sd->flags & SD_ASYM_PACKING) &&
> - sched_asym(env->sd, i, env->dst_cpu) &&
> - nr_running == 1)
> + if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1)
> continue;
>
> switch (env->migration_type) {
> @@ -11131,8 +11130,7 @@ asym_active_balance(struct lb_env *env)
> * the lower priority @env::dst_cpu help it. Do not follow
> * CPU priority.
> */
> - return env->idle != CPU_NOT_IDLE && (env->sd->flags & SD_ASYM_PACKING) &&
> - sched_use_asym_prio(env->sd, env->dst_cpu) &&
> + return env->idle != CPU_NOT_IDLE && sched_use_asym_prio(env->sd, env->dst_cpu) &&
> (sched_asym_prefer(env->dst_cpu, env->src_cpu) ||
> !sched_use_asym_prio(env->sd, env->src_cpu));
> }
> --
> 2.43.0
>

2024-02-23 04:12:56

by Alex Shi

[permalink] [raw]
Subject: Re: [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()



On 2/14/24 12:51 AM, Vincent Guittot wrote:
> On Sat, 10 Feb 2024 at 12:36, <[email protected]> wrote:
>>
>> From: Alex Shi <[email protected]>
>>
>> sched_use_asym_prio() and sched_asym_prefer() are used together in various
>> places. Consolidate them into a single function sched_asym().
>>
>> The existing sched_group_asym() is only used when collecting statistics
>
> nit: The existing sched_asym()

Hi Vincent,

Thanks for all reviewing. Do I need to update this for next version? or just fix it during merge?

Alex

>
>> of a scheduling group. Rename it as sched_group_asym(), and remove the
>> obsolete function description.
>>
>> This makes the code easier to read. No functional changes.
>>
>> Tested-by: Ricardo Neri <[email protected]>
>> Signed-off-by: Alex Shi <[email protected]>
>> Cc: Ricardo Neri <[email protected]>
>> Cc: Valentin Schneider <[email protected]>
>> Cc: Vincent Guittot <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Reviewed-by: Ricardo Neri <[email protected]>
>
> Reviewed-by: Vincent Guittot <[email protected]>
>
>> ---
>> kernel/sched/fair.c | 45 ++++++++++++++++++++-------------------------
>> 1 file changed, 20 insertions(+), 25 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 607dc310b355..426eda9eda57 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -9746,8 +9746,18 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
>> return sd->flags & SD_SHARE_CPUCAPACITY || is_core_idle(cpu);
>> }
>>
>> +static inline bool sched_asym(struct sched_domain *sd, int dst_cpu, int src_cpu)
>> +{
>> + /*
>> + * First check if @dst_cpu can do asym_packing load balance. Only do it
>> + * if it has higher priority than @src_cpu.
>> + */
>> + return sched_use_asym_prio(sd, dst_cpu) &&
>> + sched_asym_prefer(dst_cpu, src_cpu);
>> +}
>> +
>> /**
>> - * sched_asym - Check if the destination CPU can do asym_packing load balance
>> + * sched_group_asym - Check if the destination CPU can do asym_packing balance
>> * @env: The load balancing environment
>> * @sgs: Load-balancing statistics of the candidate busiest group
>> * @group: The candidate busiest group
>> @@ -9755,34 +9765,21 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
>> * @env::dst_cpu can do asym_packing if it has higher priority than the
>> * preferred CPU of @group.
>> *
>> - * SMT is a special case. If we are balancing load between cores, @env::dst_cpu
>> - * can do asym_packing balance only if all its SMT siblings are idle. Also, it
>> - * can only do it if @group is an SMT group and has exactly on busy CPU. Larger
>> - * imbalances in the number of CPUS are dealt with in find_busiest_group().
>> - *
>> - * If we are balancing load within an SMT core, or at PKG domain level, always
>> - * proceed.
>> - *
>> * Return: true if @env::dst_cpu can do with asym_packing load balance. False
>> * otherwise.
>> */
>> static inline bool
>> -sched_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
>> +sched_group_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
>> {
>> - /* Ensure that the whole local core is idle, if applicable. */
>> - if (!sched_use_asym_prio(env->sd, env->dst_cpu))
>> - return false;
>> -
>> /*
>> - * CPU priorities does not make sense for SMT cores with more than one
>> + * CPU priorities do not make sense for SMT cores with more than one
>> * busy sibling.
>> */
>> - if (group->flags & SD_SHARE_CPUCAPACITY) {
>> - if (sgs->group_weight - sgs->idle_cpus != 1)
>> - return false;
>> - }
>> + if ((group->flags & SD_SHARE_CPUCAPACITY) &&
>> + (sgs->group_weight - sgs->idle_cpus != 1))
>> + return false;
>>
>> - return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
>> + return sched_asym(env->sd, env->dst_cpu, group->asym_prefer_cpu);
>> }
>>
>> /* One group has more than one SMT CPU while the other group does not */
>> @@ -9938,7 +9935,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
>> /* Check if dst CPU is idle and preferred to this group */
>> if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
>> env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
>> - sched_asym(env, sgs, group)) {
>> + sched_group_asym(env, sgs, group)) {
>> sgs->group_asym_packing = 1;
>> }
>>
>> @@ -11037,8 +11034,7 @@ static struct rq *find_busiest_queue(struct lb_env *env,
>> * SMT cores with more than one busy sibling.
>> */
>> if ((env->sd->flags & SD_ASYM_PACKING) &&
>> - sched_use_asym_prio(env->sd, i) &&
>> - sched_asym_prefer(i, env->dst_cpu) &&
>> + sched_asym(env->sd, i, env->dst_cpu) &&
>> nr_running == 1)
>> continue;
>>
>> @@ -11908,8 +11904,7 @@ static void nohz_balancer_kick(struct rq *rq)
>> * preferred CPU must be idle.
>> */
>> for_each_cpu_and(i, sched_domain_span(sd), nohz.idle_cpus_mask) {
>> - if (sched_use_asym_prio(sd, i) &&
>> - sched_asym_prefer(i, cpu)) {
>> + if (sched_asym(sd, i, cpu)) {
>> flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
>> goto unlock;
>> }
>> --
>> 2.43.0
>>

Subject: [tip: sched/core] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio()

The following commit has been merged into the sched/core branch of tip:

Commit-ID: fbc449864e0d2ee2c16f3af2d1e9093b9b8d7ad0
Gitweb: https://git.kernel.org/tip/fbc449864e0d2ee2c16f3af2d1e9093b9b8d7ad0
Author: Alex Shi <[email protected]>
AuthorDate: Sat, 10 Feb 2024 19:39:22 +08:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 28 Feb 2024 15:43:17 +01:00

sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio()

sched_use_asym_prio() checks whether CPU priorities should be used. It
makes sense to check for the SD_ASYM_PACKING() inside the function.
Since both sched_asym() and sched_group_asym() use sched_use_asym_prio(),
remove the now superfluous checks for the flag in various places.

Signed-off-by: Alex Shi <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Tested-by: Ricardo Neri <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 475e2ca..39781a6 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9744,6 +9744,9 @@ group_type group_classify(unsigned int imbalance_pct,
*/
static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
{
+ if (!(sd->flags & SD_ASYM_PACKING))
+ return false;
+
if (!sched_smt_active())
return true;

@@ -9937,11 +9940,9 @@ static inline void update_sg_lb_stats(struct lb_env *env,
sgs->group_weight = group->group_weight;

/* Check if dst CPU is idle and preferred to this group */
- if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
- env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
- sched_group_asym(env, sgs, group)) {
+ if (!local_group && env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
+ sched_group_asym(env, sgs, group))
sgs->group_asym_packing = 1;
- }

/* Check for loaded SMT group to be balanced to dst CPU */
if (!local_group && smt_balance(env, sgs, group))
@@ -11024,9 +11025,7 @@ static struct rq *find_busiest_queue(struct lb_env *env,
* If balancing between cores, let lower priority CPUs help
* SMT cores with more than one busy sibling.
*/
- if ((env->sd->flags & SD_ASYM_PACKING) &&
- sched_asym(env->sd, i, env->dst_cpu) &&
- nr_running == 1)
+ if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1)
continue;

switch (env->migration_type) {
@@ -11122,8 +11121,7 @@ asym_active_balance(struct lb_env *env)
* the lower priority @env::dst_cpu help it. Do not follow
* CPU priority.
*/
- return env->idle != CPU_NOT_IDLE && (env->sd->flags & SD_ASYM_PACKING) &&
- sched_use_asym_prio(env->sd, env->dst_cpu) &&
+ return env->idle != CPU_NOT_IDLE && sched_use_asym_prio(env->sd, env->dst_cpu) &&
(sched_asym_prefer(env->dst_cpu, env->src_cpu) ||
!sched_use_asym_prio(env->sd, env->src_cpu));
}

Subject: [tip: sched/core] sched/topology: Rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC

The following commit has been merged into the sched/core branch of tip:

Commit-ID: 54de442747037485da1fc4eca9636287a61e97e3
Gitweb: https://git.kernel.org/tip/54de442747037485da1fc4eca9636287a61e97e3
Author: Alex Shi <[email protected]>
AuthorDate: Sat, 10 Feb 2024 19:39:23 +08:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 28 Feb 2024 15:43:17 +01:00

sched/topology: Rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC

SD_SHARE_PKG_RESOURCES is a bit of a misnomer: its naming suggests that
it's sharing all 'package resources' - while in reality it's specifically
for sharing the LLC only.

Rename it to SD_SHARE_LLC to reduce confusion.

[ mingo: Rewrote the confusing changelog as well. ]

Suggested-by: Valentin Schneider <[email protected]>
Signed-off-by: Alex Shi <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
Reviewed-by: Barry Song <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/powerpc/kernel/smp.c | 6 +++---
include/linux/sched/sd_flags.h | 4 ++--
include/linux/sched/topology.h | 6 +++---
kernel/sched/fair.c | 2 +-
kernel/sched/topology.c | 28 ++++++++++++++--------------
5 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 693334c..a60e413 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -984,7 +984,7 @@ static bool shared_caches __ro_after_init;
/* cpumask of CPUs with asymmetric SMT dependency */
static int powerpc_smt_flags(void)
{
- int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;

if (cpu_has_feature(CPU_FTR_ASYM_SMT)) {
printk_once(KERN_INFO "Enabling Asymmetric SMT scheduling\n");
@@ -1010,9 +1010,9 @@ static __ro_after_init DEFINE_STATIC_KEY_FALSE(splpar_asym_pack);
static int powerpc_shared_cache_flags(void)
{
if (static_branch_unlikely(&splpar_asym_pack))
- return SD_SHARE_PKG_RESOURCES | SD_ASYM_PACKING;
+ return SD_SHARE_LLC | SD_ASYM_PACKING;

- return SD_SHARE_PKG_RESOURCES;
+ return SD_SHARE_LLC;
}

static int powerpc_shared_proc_flags(void)
diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
index a8b2864..b04a5d0 100644
--- a/include/linux/sched/sd_flags.h
+++ b/include/linux/sched/sd_flags.h
@@ -117,13 +117,13 @@ SD_FLAG(SD_SHARE_CPUCAPACITY, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
SD_FLAG(SD_CLUSTER, SDF_NEEDS_GROUPS)

/*
- * Domain members share CPU package resources (i.e. caches)
+ * Domain members share CPU Last Level Caches
*
* SHARED_CHILD: Set from the base domain up until spanned CPUs no longer share
* the same cache(s).
* NEEDS_GROUPS: Caches are shared between groups.
*/
-SD_FLAG(SD_SHARE_PKG_RESOURCES, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
+SD_FLAG(SD_SHARE_LLC, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)

/*
* Only a single load balancing instance
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index a6e04b4..191b122 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -38,21 +38,21 @@ extern const struct sd_flag_debug sd_flag_debug[];
#ifdef CONFIG_SCHED_SMT
static inline int cpu_smt_flags(void)
{
- return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
+ return SD_SHARE_CPUCAPACITY | SD_SHARE_LLC;
}
#endif

#ifdef CONFIG_SCHED_CLUSTER
static inline int cpu_cluster_flags(void)
{
- return SD_CLUSTER | SD_SHARE_PKG_RESOURCES;
+ return SD_CLUSTER | SD_SHARE_LLC;
}
#endif

#ifdef CONFIG_SCHED_MC
static inline int cpu_core_flags(void)
{
- return SD_SHARE_PKG_RESOURCES;
+ return SD_SHARE_LLC;
}
#endif

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 39781a6..6a16129 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10678,7 +10678,7 @@ static inline void calculate_imbalance(struct lb_env *env, struct sd_lb_stats *s
*/
if (local->group_type == group_has_spare) {
if ((busiest->group_type > group_fully_busy) &&
- !(env->sd->flags & SD_SHARE_PKG_RESOURCES)) {
+ !(env->sd->flags & SD_SHARE_LLC)) {
/*
* If busiest is overloaded, try to fill spare
* capacity. This might end up creating spare capacity
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 0b33f7b..99ea598 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -657,13 +657,13 @@ static void destroy_sched_domains(struct sched_domain *sd)
}

/*
- * Keep a special pointer to the highest sched_domain that has
- * SD_SHARE_PKG_RESOURCE set (Last Level Cache Domain) for this
- * allows us to avoid some pointer chasing select_idle_sibling().
+ * Keep a special pointer to the highest sched_domain that has SD_SHARE_LLC set
+ * (Last Level Cache Domain) for this allows us to avoid some pointer chasing
+ * select_idle_sibling().
*
- * Also keep a unique ID per domain (we use the first CPU number in
- * the cpumask of the domain), this allows us to quickly tell if
- * two CPUs are in the same cache domain, see cpus_share_cache().
+ * Also keep a unique ID per domain (we use the first CPU number in the cpumask
+ * of the domain), this allows us to quickly tell if two CPUs are in the same
+ * cache domain, see cpus_share_cache().
*/
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DEFINE_PER_CPU(int, sd_llc_size);
@@ -684,7 +684,7 @@ static void update_top_cache_domain(int cpu)
int id = cpu;
int size = 1;

- sd = highest_flag_domain(cpu, SD_SHARE_PKG_RESOURCES);
+ sd = highest_flag_domain(cpu, SD_SHARE_LLC);
if (sd) {
id = cpumask_first(sched_domain_span(sd));
size = cpumask_weight(sched_domain_span(sd));
@@ -1554,7 +1554,7 @@ static struct cpumask ***sched_domains_numa_masks;
* function. For details, see include/linux/sched/sd_flags.h.
*
* SD_SHARE_CPUCAPACITY
- * SD_SHARE_PKG_RESOURCES
+ * SD_SHARE_LLC
* SD_CLUSTER
* SD_NUMA
*
@@ -1566,7 +1566,7 @@ static struct cpumask ***sched_domains_numa_masks;
#define TOPOLOGY_SD_FLAGS \
(SD_SHARE_CPUCAPACITY | \
SD_CLUSTER | \
- SD_SHARE_PKG_RESOURCES | \
+ SD_SHARE_LLC | \
SD_NUMA | \
SD_ASYM_PACKING)

@@ -1609,7 +1609,7 @@ sd_init(struct sched_domain_topology_level *tl,
| 0*SD_BALANCE_WAKE
| 1*SD_WAKE_AFFINE
| 0*SD_SHARE_CPUCAPACITY
- | 0*SD_SHARE_PKG_RESOURCES
+ | 0*SD_SHARE_LLC
| 0*SD_SERIALIZE
| 1*SD_PREFER_SIBLING
| 0*SD_NUMA
@@ -1646,7 +1646,7 @@ sd_init(struct sched_domain_topology_level *tl,
if (sd->flags & SD_SHARE_CPUCAPACITY) {
sd->imbalance_pct = 110;

- } else if (sd->flags & SD_SHARE_PKG_RESOURCES) {
+ } else if (sd->flags & SD_SHARE_LLC) {
sd->imbalance_pct = 117;
sd->cache_nice_tries = 1;

@@ -1671,7 +1671,7 @@ sd_init(struct sched_domain_topology_level *tl,
* For all levels sharing cache; connect a sched_domain_shared
* instance.
*/
- if (sd->flags & SD_SHARE_PKG_RESOURCES) {
+ if (sd->flags & SD_SHARE_LLC) {
sd->shared = *per_cpu_ptr(sdd->sds, sd_id);
atomic_inc(&sd->shared->ref);
atomic_set(&sd->shared->nr_busy_cpus, sd_weight);
@@ -2446,8 +2446,8 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
struct sched_domain *child = sd->child;

- if (!(sd->flags & SD_SHARE_PKG_RESOURCES) && child &&
- (child->flags & SD_SHARE_PKG_RESOURCES)) {
+ if (!(sd->flags & SD_SHARE_LLC) && child &&
+ (child->flags & SD_SHARE_LLC)) {
struct sched_domain __rcu *top_p;
unsigned int nr_llcs;


Subject: [tip: sched/core] sched/fair: Remove unused parameter from sched_asym()

The following commit has been merged into the sched/core branch of tip:

Commit-ID: 5a64983731566f3b102b4ed12445b8a1b2f46a46
Gitweb: https://git.kernel.org/tip/5a64983731566f3b102b4ed12445b8a1b2f46a46
Author: Alex Shi <[email protected]>
AuthorDate: Sat, 10 Feb 2024 19:39:20 +08:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 28 Feb 2024 15:43:08 +01:00

sched/fair: Remove unused parameter from sched_asym()

The 'sds' argument is not used in the sched_asym() function anymore, remove it.

Fixes: c9ca07886aaa ("sched/fair: Do not even the number of busy CPUs via asym_packing")
Signed-off-by: Alex Shi <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 51fe17f..300d1bf 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9753,7 +9753,6 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
/**
* sched_asym - Check if the destination CPU can do asym_packing load balance
* @env: The load balancing environment
- * @sds: Load-balancing data with statistics of the local group
* @sgs: Load-balancing statistics of the candidate busiest group
* @group: The candidate busiest group
*
@@ -9772,8 +9771,7 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
* otherwise.
*/
static inline bool
-sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs,
- struct sched_group *group)
+sched_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
{
/* Ensure that the whole local core is idle, if applicable. */
if (!sched_use_asym_prio(env->sd, env->dst_cpu))
@@ -9944,7 +9942,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
/* Check if dst CPU is idle and preferred to this group */
if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
- sched_asym(env, sds, sgs, group)) {
+ sched_asym(env, sgs, group)) {
sgs->group_asym_packing = 1;
}


Subject: [tip: sched/core] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS

The following commit has been merged into the sched/core branch of tip:

Commit-ID: d654c8ddde84b9d1a30a40917e588b5a1e53dada
Gitweb: https://git.kernel.org/tip/d654c8ddde84b9d1a30a40917e588b5a1e53dada
Author: Alex Shi <[email protected]>
AuthorDate: Sat, 10 Feb 2024 19:39:19 +08:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 28 Feb 2024 15:29:21 +01:00

sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS

These flags are already documented in include/linux/sched/sd_flags.h.

Also, add missing SD_CLUSTER and keep the comment on SD_ASYM_PACKING
as it is a special case.

Suggested-by: Ricardo Neri <[email protected]>
Signed-off-by: Alex Shi <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
Reviewed-by: Valentin Schneider <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/topology.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 10d1391..0b33f7b 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1551,11 +1551,12 @@ static struct cpumask ***sched_domains_numa_masks;
*
* These flags are purely descriptive of the topology and do not prescribe
* behaviour. Behaviour is artificial and mapped in the below sd_init()
- * function:
+ * function. For details, see include/linux/sched/sd_flags.h.
*
- * SD_SHARE_CPUCAPACITY - describes SMT topologies
- * SD_SHARE_PKG_RESOURCES - describes shared caches
- * SD_NUMA - describes NUMA topologies
+ * SD_SHARE_CPUCAPACITY
+ * SD_SHARE_PKG_RESOURCES
+ * SD_CLUSTER
+ * SD_NUMA
*
* Odd one out, which beside describing the topology has a quirk also
* prescribes the desired behaviour that goes along with it:

Subject: [tip: sched/core] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()

The following commit has been merged into the sched/core branch of tip:

Commit-ID: 45de20623475049c424bc0b89f42efca54995edd
Gitweb: https://git.kernel.org/tip/45de20623475049c424bc0b89f42efca54995edd
Author: Alex Shi <[email protected]>
AuthorDate: Sat, 10 Feb 2024 19:39:21 +08:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 28 Feb 2024 15:43:17 +01:00

sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()

sched_use_asym_prio() and sched_asym_prefer() are used together in various
places. Consolidate them into a single function sched_asym().

The existing sched_asym() function is only used when collecting statistics
of a scheduling group. Rename it as sched_group_asym(), and remove the
obsolete function description.

This makes the code easier to read. No functional changes.

Signed-off-by: Alex Shi <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Tested-by: Ricardo Neri <[email protected]>
Reviewed-by: Ricardo Neri <[email protected]>
Reviewed-by: Vincent Guittot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 45 +++++++++++++++++++-------------------------
1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 300d1bf..475e2ca 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9750,8 +9750,18 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
return sd->flags & SD_SHARE_CPUCAPACITY || is_core_idle(cpu);
}

+static inline bool sched_asym(struct sched_domain *sd, int dst_cpu, int src_cpu)
+{
+ /*
+ * First check if @dst_cpu can do asym_packing load balance. Only do it
+ * if it has higher priority than @src_cpu.
+ */
+ return sched_use_asym_prio(sd, dst_cpu) &&
+ sched_asym_prefer(dst_cpu, src_cpu);
+}
+
/**
- * sched_asym - Check if the destination CPU can do asym_packing load balance
+ * sched_group_asym - Check if the destination CPU can do asym_packing balance
* @env: The load balancing environment
* @sgs: Load-balancing statistics of the candidate busiest group
* @group: The candidate busiest group
@@ -9759,34 +9769,21 @@ static bool sched_use_asym_prio(struct sched_domain *sd, int cpu)
* @env::dst_cpu can do asym_packing if it has higher priority than the
* preferred CPU of @group.
*
- * SMT is a special case. If we are balancing load between cores, @env::dst_cpu
- * can do asym_packing balance only if all its SMT siblings are idle. Also, it
- * can only do it if @group is an SMT group and has exactly on busy CPU. Larger
- * imbalances in the number of CPUS are dealt with in find_busiest_group().
- *
- * If we are balancing load within an SMT core, or at PKG domain level, always
- * proceed.
- *
* Return: true if @env::dst_cpu can do with asym_packing load balance. False
* otherwise.
*/
static inline bool
-sched_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
+sched_group_asym(struct lb_env *env, struct sg_lb_stats *sgs, struct sched_group *group)
{
- /* Ensure that the whole local core is idle, if applicable. */
- if (!sched_use_asym_prio(env->sd, env->dst_cpu))
- return false;
-
/*
- * CPU priorities does not make sense for SMT cores with more than one
+ * CPU priorities do not make sense for SMT cores with more than one
* busy sibling.
*/
- if (group->flags & SD_SHARE_CPUCAPACITY) {
- if (sgs->group_weight - sgs->idle_cpus != 1)
- return false;
- }
+ if ((group->flags & SD_SHARE_CPUCAPACITY) &&
+ (sgs->group_weight - sgs->idle_cpus != 1))
+ return false;

- return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
+ return sched_asym(env->sd, env->dst_cpu, group->asym_prefer_cpu);
}

/* One group has more than one SMT CPU while the other group does not */
@@ -9942,7 +9939,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
/* Check if dst CPU is idle and preferred to this group */
if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
- sched_asym(env, sgs, group)) {
+ sched_group_asym(env, sgs, group)) {
sgs->group_asym_packing = 1;
}

@@ -11028,8 +11025,7 @@ static struct rq *find_busiest_queue(struct lb_env *env,
* SMT cores with more than one busy sibling.
*/
if ((env->sd->flags & SD_ASYM_PACKING) &&
- sched_use_asym_prio(env->sd, i) &&
- sched_asym_prefer(i, env->dst_cpu) &&
+ sched_asym(env->sd, i, env->dst_cpu) &&
nr_running == 1)
continue;

@@ -11899,8 +11895,7 @@ static void nohz_balancer_kick(struct rq *rq)
* preferred CPU must be idle.
*/
for_each_cpu_and(i, sched_domain_span(sd), nohz.idle_cpus_mask) {
- if (sched_use_asym_prio(sd, i) &&
- sched_asym_prefer(i, cpu)) {
+ if (sched_asym(sd, i, cpu)) {
flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
goto unlock;
}