mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS
@ 2024-02-10 11:39 alexs
  2024-02-10 11:39 ` [PATCH v5 2/5] sched/fair: Remove unused parameters alexs
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: alexs @ 2024-02-10 11:39 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER
  Cc: Alex Shi, Ricardo Neri

From: Alex Shi <alexs@kernel.org>

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 <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
---
 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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5 2/5] sched/fair: Remove unused parameters
  2024-02-10 11:39 [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS alexs
@ 2024-02-10 11:39 ` alexs
  2024-02-13 16:45   ` Vincent Guittot
  2024-02-28 22:00   ` [tip: sched/core] sched/fair: Remove unused parameter from sched_asym() tip-bot2 for Alex Shi
  2024-02-10 11:39 ` [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer() alexs
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: alexs @ 2024-02-10 11:39 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Ricardo Neri
  Cc: Alex Shi

From: Alex Shi <alexs@kernel.org>

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 <alexs@kernel.org>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
---
 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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()
  2024-02-10 11:39 [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS alexs
  2024-02-10 11:39 ` [PATCH v5 2/5] sched/fair: Remove unused parameters alexs
@ 2024-02-10 11:39 ` alexs
  2024-02-13 16:51   ` Vincent Guittot
  2024-02-28 22:00   ` [tip: sched/core] " tip-bot2 for Alex Shi
  2024-02-10 11:39 ` [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio() alexs
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: alexs @ 2024-02-10 11:39 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER
  Cc: Alex Shi, Ricardo Neri

From: Alex Shi <alexs@kernel.org>

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 <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio()
  2024-02-10 11:39 [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS alexs
  2024-02-10 11:39 ` [PATCH v5 2/5] sched/fair: Remove unused parameters alexs
  2024-02-10 11:39 ` [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer() alexs
@ 2024-02-10 11:39 ` alexs
  2024-02-13 17:02   ` Vincent Guittot
  2024-02-28 22:00   ` [tip: sched/core] " tip-bot2 for Alex Shi
  2024-02-10 11:39 ` [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC alexs
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 17+ messages in thread
From: alexs @ 2024-02-10 11:39 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER
  Cc: Alex Shi, Ricardo Neri, linux-kernel

From: Alex Shi <alexs@kernel.org>

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 <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC
  2024-02-10 11:39 [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS alexs
                   ` (2 preceding siblings ...)
  2024-02-10 11:39 ` [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio() alexs
@ 2024-02-10 11:39 ` alexs
  2024-02-13  7:01   ` Barry Song
  2024-02-28 22:00   ` [tip: sched/core] sched/topology: Rename " tip-bot2 for Alex Shi
  2024-02-13 16:45 ` [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS Vincent Guittot
  2024-02-28 22:00 ` [tip: sched/core] " tip-bot2 for Alex Shi
  5 siblings, 2 replies; 17+ messages in thread
From: alexs @ 2024-02-10 11:39 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy, Aneesh Kumar K.V, Naveen N. Rao,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
  Cc: Alex Shi, Ricardo Neri, linux-kernel, linuxppc-dev, Miaohe Lin,
	Barry Song, Mark Rutland, Frederic Weisbecker, Gautham R. Shenoy,
	Yicong Yang, Josh Poimboeuf, Srikar Dronamraju

From: Alex Shi <alexs@kernel.org>

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 <vschneid@redhat.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Miaohe Lin <linmiaohe@huawei.com>
Cc: Barry Song <song.bao.hua@hisilicon.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Juri Lelli <juri.lelli@redhat.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: "Gautham R. Shenoy" <gautham.shenoy@amd.com>
Cc: Yicong Yang <yangyicong@hisilicon.com>
Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
 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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC
  2024-02-10 11:39 ` [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC alexs
@ 2024-02-13  7:01   ` Barry Song
  2024-02-13  7:21     ` Barry Song
  2024-02-28 22:00   ` [tip: sched/core] sched/topology: Rename " tip-bot2 for Alex Shi
  1 sibling, 1 reply; 17+ messages in thread
From: Barry Song @ 2024-02-13  7:01 UTC (permalink / raw)
  To: alexs
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy, Aneesh Kumar K.V, Naveen N. Rao,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	Ricardo Neri, Miaohe Lin, Barry Song, Mark Rutland,
	Frederic Weisbecker, Gautham R. Shenoy, Yicong Yang,
	Josh Poimboeuf, Srikar Dronamraju

Hi Alex, Valentin,


On Sun, Feb 11, 2024 at 12:37 AM <alexs@kernel.org> wrote:
>
> From: Alex Shi <alexs@kernel.org>
>
> 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 <vschneid@redhat.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Miaohe Lin <linmiaohe@huawei.com>
> Cc: Barry Song <song.bao.hua@hisilicon.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Frederic Weisbecker <frederic@kernel.org>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: Ben Segall <bsegall@google.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> Cc: "Gautham R. Shenoy" <gautham.shenoy@amd.com>
> Cc: Yicong Yang <yangyicong@hisilicon.com>
> Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Valentin Schneider <vschneid@redhat.com>
> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC
  2024-02-13  7:01   ` Barry Song
@ 2024-02-13  7:21     ` Barry Song
  0 siblings, 0 replies; 17+ messages in thread
From: Barry Song @ 2024-02-13  7:21 UTC (permalink / raw)
  To: alexs, Valentin Schneider
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, open list:SCHEDULER,
	Michael Ellerman, Nicholas Piggin, Christophe Leroy,
	Aneesh Kumar K.V, Naveen N. Rao,
	open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
	Ricardo Neri, Miaohe Lin, Barry Song, Mark Rutland,
	Frederic Weisbecker, Gautham R. Shenoy, Yicong Yang,
	Josh Poimboeuf, Srikar Dronamraju

On Tue, Feb 13, 2024 at 8:01 PM Barry Song <21cnbao@gmail.com> wrote:
>
> Hi Alex, Valentin,
>
>
> On Sun, Feb 11, 2024 at 12:37 AM <alexs@kernel.org> wrote:
> >
> > From: Alex Shi <alexs@kernel.org>
> >
> > 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 <baohua@kernel.org>

>
> >
> > Suggested-by: Valentin Schneider <vschneid@redhat.com>
> > Signed-off-by: Alex Shi <alexs@kernel.org>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: Miaohe Lin <linmiaohe@huawei.com>
> > Cc: Barry Song <song.bao.hua@hisilicon.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Frederic Weisbecker <frederic@kernel.org>
> > Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> > Cc: Ben Segall <bsegall@google.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> > Cc: Juri Lelli <juri.lelli@redhat.com>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "Naveen N. Rao" <naveen.n.rao@linux.ibm.com>
> > Cc: "Aneesh Kumar K.V" <aneesh.kumar@kernel.org>
> > Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
> > Cc: "Gautham R. Shenoy" <gautham.shenoy@amd.com>
> > Cc: Yicong Yang <yangyicong@hisilicon.com>
> > Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> > Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> > Cc: Valentin Schneider <vschneid@redhat.com>
> > Cc: Nicholas Piggin <npiggin@gmail.com>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Reviewed-by: Valentin Schneider <vschneid@redhat.com>
> > Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS
  2024-02-10 11:39 [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS alexs
                   ` (3 preceding siblings ...)
  2024-02-10 11:39 ` [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC alexs
@ 2024-02-13 16:45 ` Vincent Guittot
  2024-02-28 22:00 ` [tip: sched/core] " tip-bot2 for Alex Shi
  5 siblings, 0 replies; 17+ messages in thread
From: Vincent Guittot @ 2024-02-13 16:45 UTC (permalink / raw)
  To: alexs
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Ricardo Neri

On Sat, 10 Feb 2024 at 12:36, <alexs@kernel.org> wrote:
>
> From: Alex Shi <alexs@kernel.org>
>
> 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 <ricardo.neri-calderon@linux.intel.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> Reviewed-by: Valentin Schneider <vschneid@redhat.com>

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>

> ---
>  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
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 2/5] sched/fair: Remove unused parameters
  2024-02-10 11:39 ` [PATCH v5 2/5] sched/fair: Remove unused parameters alexs
@ 2024-02-13 16:45   ` Vincent Guittot
  2024-02-28 22:00   ` [tip: sched/core] sched/fair: Remove unused parameter from sched_asym() tip-bot2 for Alex Shi
  1 sibling, 0 replies; 17+ messages in thread
From: Vincent Guittot @ 2024-02-13 16:45 UTC (permalink / raw)
  To: alexs
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Ricardo Neri

On Sat, 10 Feb 2024 at 12:36, <alexs@kernel.org> wrote:
>
> From: Alex Shi <alexs@kernel.org>
>
> 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 <alexs@kernel.org>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> Reviewed-by: Valentin Schneider <vschneid@redhat.com>

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>

> ---
>  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
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()
  2024-02-10 11:39 ` [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer() alexs
@ 2024-02-13 16:51   ` Vincent Guittot
  2024-02-23  3:52     ` kuiliang Shi
  2024-02-28 22:00   ` [tip: sched/core] " tip-bot2 for Alex Shi
  1 sibling, 1 reply; 17+ messages in thread
From: Vincent Guittot @ 2024-02-13 16:51 UTC (permalink / raw)
  To: alexs
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Ricardo Neri

On Sat, 10 Feb 2024 at 12:36, <alexs@kernel.org> wrote:
>
> From: Alex Shi <alexs@kernel.org>
>
> 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 <ricardo.neri-calderon@linux.intel.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>

> ---
>  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
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio()
  2024-02-10 11:39 ` [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio() alexs
@ 2024-02-13 17:02   ` Vincent Guittot
  2024-02-28 22:00   ` [tip: sched/core] " tip-bot2 for Alex Shi
  1 sibling, 0 replies; 17+ messages in thread
From: Vincent Guittot @ 2024-02-13 17:02 UTC (permalink / raw)
  To: alexs
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Ricardo Neri

On Sat, 10 Feb 2024 at 12:36, <alexs@kernel.org> wrote:
>
> From: Alex Shi <alexs@kernel.org>
>
> 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 <ricardo.neri-calderon@linux.intel.com>
> Signed-off-by: Alex Shi <alexs@kernel.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> Cc: Ben Segall <bsegall@google.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>

> ---
>  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
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()
  2024-02-13 16:51   ` Vincent Guittot
@ 2024-02-23  3:52     ` kuiliang Shi
  0 siblings, 0 replies; 17+ messages in thread
From: kuiliang Shi @ 2024-02-23  3:52 UTC (permalink / raw)
  To: Vincent Guittot, alexs
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Mel Gorman,
	Daniel Bristot de Oliveira, Valentin Schneider,
	open list:SCHEDULER, Ricardo Neri



On 2/14/24 12:51 AM, Vincent Guittot wrote:
> On Sat, 10 Feb 2024 at 12:36, <alexs@kernel.org> wrote:
>>
>> From: Alex Shi <alexs@kernel.org>
>>
>> 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 <ricardo.neri-calderon@linux.intel.com>
>> Signed-off-by: Alex Shi <alexs@kernel.org>
>> Cc: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
>> Cc: Valentin Schneider <vschneid@redhat.com>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> 
> Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> 
>> ---
>>  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
>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [tip: sched/core] sched/topology: Rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC
  2024-02-10 11:39 ` [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC alexs
  2024-02-13  7:01   ` Barry Song
@ 2024-02-28 22:00   ` tip-bot2 for Alex Shi
  1 sibling, 0 replies; 17+ messages in thread
From: tip-bot2 for Alex Shi @ 2024-02-28 22:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Valentin Schneider, Alex Shi, Ingo Molnar, Ricardo Neri,
	Barry Song, x86, linux-kernel

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 <alexs@kernel.org>
AuthorDate:    Sat, 10 Feb 2024 19:39:23 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
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 <vschneid@redhat.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Barry Song <baohua@kernel.org>
Link: https://lore.kernel.org/r/20240210113924.1130448-5-alexs@kernel.org
---
 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;
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [tip: sched/core] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio()
  2024-02-10 11:39 ` [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio() alexs
  2024-02-13 17:02   ` Vincent Guittot
@ 2024-02-28 22:00   ` tip-bot2 for Alex Shi
  1 sibling, 0 replies; 17+ messages in thread
From: tip-bot2 for Alex Shi @ 2024-02-28 22:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Alex Shi, Ingo Molnar, Ricardo Neri, Vincent Guittot, x86, linux-kernel

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 <alexs@kernel.org>
AuthorDate:    Sat, 10 Feb 2024 19:39:22 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
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 <alexs@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Tested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20240210113924.1130448-4-alexs@kernel.org
---
 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));
 }

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [tip: sched/core] sched/fair: Remove unused parameter from sched_asym()
  2024-02-10 11:39 ` [PATCH v5 2/5] sched/fair: Remove unused parameters alexs
  2024-02-13 16:45   ` Vincent Guittot
@ 2024-02-28 22:00   ` tip-bot2 for Alex Shi
  1 sibling, 0 replies; 17+ messages in thread
From: tip-bot2 for Alex Shi @ 2024-02-28 22:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Alex Shi, Ingo Molnar, Ricardo Neri, Valentin Schneider,
	Vincent Guittot, x86, linux-kernel

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 <alexs@kernel.org>
AuthorDate:    Sat, 10 Feb 2024 19:39:20 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
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 <alexs@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20240210113924.1130448-2-alexs@kernel.org
---
 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;
 	}
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [tip: sched/core] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer()
  2024-02-10 11:39 ` [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer() alexs
  2024-02-13 16:51   ` Vincent Guittot
@ 2024-02-28 22:00   ` tip-bot2 for Alex Shi
  1 sibling, 0 replies; 17+ messages in thread
From: tip-bot2 for Alex Shi @ 2024-02-28 22:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Alex Shi, Ingo Molnar, Ricardo Neri, Vincent Guittot, x86, linux-kernel

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 <alexs@kernel.org>
AuthorDate:    Sat, 10 Feb 2024 19:39:21 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
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 <alexs@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Tested-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20240210113924.1130448-3-alexs@kernel.org
---
 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;
 			}

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [tip: sched/core] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS
  2024-02-10 11:39 [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS alexs
                   ` (4 preceding siblings ...)
  2024-02-13 16:45 ` [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS Vincent Guittot
@ 2024-02-28 22:00 ` tip-bot2 for Alex Shi
  5 siblings, 0 replies; 17+ messages in thread
From: tip-bot2 for Alex Shi @ 2024-02-28 22:00 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Ricardo Neri, Alex Shi, Ingo Molnar, Valentin Schneider,
	Vincent Guittot, x86, linux-kernel

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 <alexs@kernel.org>
AuthorDate:    Sat, 10 Feb 2024 19:39:19 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
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 <ricardo.neri-calderon@linux.intel.com>
Signed-off-by: Alex Shi <alexs@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
Reviewed-by: Valentin Schneider <vschneid@redhat.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20240210113924.1130448-1-alexs@kernel.org
---
 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:

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2024-02-28 22:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-10 11:39 [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS alexs
2024-02-10 11:39 ` [PATCH v5 2/5] sched/fair: Remove unused parameters alexs
2024-02-13 16:45   ` Vincent Guittot
2024-02-28 22:00   ` [tip: sched/core] sched/fair: Remove unused parameter from sched_asym() tip-bot2 for Alex Shi
2024-02-10 11:39 ` [PATCH v5 3/5] sched/fair: Rework sched_use_asym_prio() and sched_asym_prefer() alexs
2024-02-13 16:51   ` Vincent Guittot
2024-02-23  3:52     ` kuiliang Shi
2024-02-28 22:00   ` [tip: sched/core] " tip-bot2 for Alex Shi
2024-02-10 11:39 ` [PATCH v5 4/5] sched/fair: Check the SD_ASYM_PACKING flag in sched_use_asym_prio() alexs
2024-02-13 17:02   ` Vincent Guittot
2024-02-28 22:00   ` [tip: sched/core] " tip-bot2 for Alex Shi
2024-02-10 11:39 ` [PATCH v5 5/5] sched: rename SD_SHARE_PKG_RESOURCES to SD_SHARE_LLC alexs
2024-02-13  7:01   ` Barry Song
2024-02-13  7:21     ` Barry Song
2024-02-28 22:00   ` [tip: sched/core] sched/topology: Rename " tip-bot2 for Alex Shi
2024-02-13 16:45 ` [PATCH v5 1/5] sched/topology: Remove duplicate descriptions from TOPOLOGY_SD_FLAGS Vincent Guittot
2024-02-28 22:00 ` [tip: sched/core] " tip-bot2 for Alex Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®