* [RESEND PATCH v5 0/2] sched/fair: Scan cluster before scanning LLC in wake-up path
@ 2022-07-20 8:11 Yicong Yang
2022-07-20 8:11 ` [RESEND PATCH v5 1/2] sched: Add per_cpu cluster domain info and cpus_share_lowest_cache API Yicong Yang
2022-07-20 8:11 ` [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
0 siblings, 2 replies; 9+ messages in thread
From: Yicong Yang @ 2022-07-20 8:11 UTC (permalink / raw)
To: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen,
gautham.shenoy, linux-kernel, linux-arm-kernel
Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
yangyicong, jonathan.cameron, ego, srikar, linuxarm, 21cnbao,
guodong.xu, hesham.almatary, john.garry, shenyang39,
kprateek.nayak, yu.c.chen, wuyun.abel
This is the follow-up work to support cluster scheduler. Previously
we have added cluster level in the scheduler for both ARM64[1] and
X86[2] to support load balance between clusters to bring more memory
bandwidth and decrease cache contention. This patchset, on the other
hand, takes care of wake-up path by giving CPUs within the same cluster
a try before scanning the whole LLC to benefit those tasks communicating
with each other.
[1] 778c558f49a2 ("sched: Add cluster scheduler level in core and related Kconfig for ARM64")
[2] 66558b730f25 ("sched: Add cluster scheduler level for x86")
Change since v4:
- rename cpus_share_resources to cpus_share_lowest_cache to be more informative, per Tim
- return -1 when nr==0 in scan_cluster(), per Abel
Thanks!
Link: https://lore.kernel.org/lkml/20220609120622.47724-1-yangyicong@hisilicon.com/
Change since v3:
- fix compile error when !CONFIG_SCHED_CLUSTER, reported by lkp test.
Link: https://lore.kernel.org/lkml/20220608095758.60504-1-yangyicong@hisilicon.com/
Change since v2:
- leverage SIS_PROP to suspend redundant scanning when LLC is overloaded
- remove the ping-pong suppression
- address the comment from Tim, thanks.
Link: https://lore.kernel.org/lkml/20220126080947.4529-1-yangyicong@hisilicon.com/
Change since v1:
- regain the performance data based on v5.17-rc1
- rename cpus_share_cluster to cpus_share_resources per Vincent and Gautham, thanks!
Link: https://lore.kernel.org/lkml/20211215041149.73171-1-yangyicong@hisilicon.com/
Barry Song (2):
sched: Add per_cpu cluster domain info and cpus_share_lowest_cache API
sched/fair: Scan cluster before scanning LLC in wake-up path
include/linux/sched/sd_flags.h | 7 ++++++
include/linux/sched/topology.h | 8 ++++++-
kernel/sched/core.c | 12 ++++++++++
kernel/sched/fair.c | 44 +++++++++++++++++++++++++++++++---
kernel/sched/sched.h | 2 ++
kernel/sched/topology.c | 15 ++++++++++++
6 files changed, 84 insertions(+), 4 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RESEND PATCH v5 1/2] sched: Add per_cpu cluster domain info and cpus_share_lowest_cache API
2022-07-20 8:11 [RESEND PATCH v5 0/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
@ 2022-07-20 8:11 ` Yicong Yang
2022-07-20 13:56 ` Vincent Guittot
2022-07-20 8:11 ` [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
1 sibling, 1 reply; 9+ messages in thread
From: Yicong Yang @ 2022-07-20 8:11 UTC (permalink / raw)
To: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen,
gautham.shenoy, linux-kernel, linux-arm-kernel
Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
yangyicong, jonathan.cameron, ego, srikar, linuxarm, 21cnbao,
guodong.xu, hesham.almatary, john.garry, shenyang39,
kprateek.nayak, yu.c.chen, wuyun.abel
From: Barry Song <song.bao.hua@hisilicon.com>
Add per-cpu cluster domain info and cpus_share_lowest_cache() API.
This is the preparation for the optimization of select_idle_cpu()
on platforms with cluster scheduler level.
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>
---
include/linux/sched/sd_flags.h | 7 +++++++
include/linux/sched/topology.h | 8 +++++++-
kernel/sched/core.c | 12 ++++++++++++
kernel/sched/sched.h | 2 ++
kernel/sched/topology.c | 15 +++++++++++++++
5 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
index 57bde66d95f7..42ed454e8b18 100644
--- a/include/linux/sched/sd_flags.h
+++ b/include/linux/sched/sd_flags.h
@@ -109,6 +109,13 @@ SD_FLAG(SD_ASYM_CPUCAPACITY_FULL, SDF_SHARED_PARENT | SDF_NEEDS_GROUPS)
*/
SD_FLAG(SD_SHARE_CPUCAPACITY, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
+/*
+ * Domain members share CPU cluster (LLC tags or L2 cache)
+ *
+ * NEEDS_GROUPS: Clusters are shared between groups.
+ */
+SD_FLAG(SD_CLUSTER, SDF_NEEDS_GROUPS)
+
/*
* Domain members share CPU package resources (i.e. caches)
*
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index 816df6cc444e..c0d21667ddf3 100644
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -45,7 +45,7 @@ static inline int cpu_smt_flags(void)
#ifdef CONFIG_SCHED_CLUSTER
static inline int cpu_cluster_flags(void)
{
- return SD_SHARE_PKG_RESOURCES;
+ return SD_CLUSTER | SD_SHARE_PKG_RESOURCES;
}
#endif
@@ -179,6 +179,7 @@ cpumask_var_t *alloc_sched_domains(unsigned int ndoms);
void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms);
bool cpus_share_cache(int this_cpu, int that_cpu);
+bool cpus_share_lowest_cache(int this_cpu, int that_cpu);
typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
typedef int (*sched_domain_flags_f)(void);
@@ -232,6 +233,11 @@ static inline bool cpus_share_cache(int this_cpu, int that_cpu)
return true;
}
+static inline bool cpus_share_lowest_cache(int this_cpu, int that_cpu)
+{
+ return true;
+}
+
#endif /* !CONFIG_SMP */
#if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_CPU_FREQ_GOV_SCHEDUTIL)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a463dbc92fcd..96109ad82694 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3802,6 +3802,18 @@ bool cpus_share_cache(int this_cpu, int that_cpu)
return per_cpu(sd_llc_id, this_cpu) == per_cpu(sd_llc_id, that_cpu);
}
+/*
+ * Whether CPUs are share lowest cache, which means LLC on non-cluster
+ * machines and LLC tag or L2 on machines with clusters.
+ */
+bool cpus_share_lowest_cache(int this_cpu, int that_cpu)
+{
+ if (this_cpu == that_cpu)
+ return true;
+
+ return per_cpu(sd_lowest_cache_id, this_cpu) == per_cpu(sd_lowest_cache_id, that_cpu);
+}
+
static inline bool ttwu_queue_cond(int cpu)
{
/*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 73ae32898f25..845cd029d572 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1802,7 +1802,9 @@ static inline struct sched_domain *lowest_flag_domain(int cpu, int flag)
DECLARE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DECLARE_PER_CPU(int, sd_llc_size);
DECLARE_PER_CPU(int, sd_llc_id);
+DECLARE_PER_CPU(int, sd_lowest_cache_id);
DECLARE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
+DECLARE_PER_CPU(struct sched_domain __rcu *, sd_cluster);
DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 8739c2a5a54e..8ab27c0d6d1f 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -664,6 +664,8 @@ static void destroy_sched_domains(struct sched_domain *sd)
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
DEFINE_PER_CPU(int, sd_llc_size);
DEFINE_PER_CPU(int, sd_llc_id);
+DEFINE_PER_CPU(int, sd_lowest_cache_id);
+DEFINE_PER_CPU(struct sched_domain __rcu *, sd_cluster);
DEFINE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
@@ -689,6 +691,18 @@ static void update_top_cache_domain(int cpu)
per_cpu(sd_llc_id, cpu) = id;
rcu_assign_pointer(per_cpu(sd_llc_shared, cpu), sds);
+ sd = lowest_flag_domain(cpu, SD_CLUSTER);
+ if (sd)
+ id = cpumask_first(sched_domain_span(sd));
+ rcu_assign_pointer(per_cpu(sd_cluster, cpu), sd);
+
+ /*
+ * This assignment should be placed after the sd_llc_id as
+ * we want this id equals to cluster id on cluster machines
+ * but equals to LLC id on non-Cluster machines.
+ */
+ per_cpu(sd_lowest_cache_id, cpu) = id;
+
sd = lowest_flag_domain(cpu, SD_NUMA);
rcu_assign_pointer(per_cpu(sd_numa, cpu), sd);
@@ -1532,6 +1546,7 @@ static struct cpumask ***sched_domains_numa_masks;
*/
#define TOPOLOGY_SD_FLAGS \
(SD_SHARE_CPUCAPACITY | \
+ SD_CLUSTER | \
SD_SHARE_PKG_RESOURCES | \
SD_NUMA | \
SD_ASYM_PACKING)
--
2.24.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
2022-07-20 8:11 [RESEND PATCH v5 0/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
2022-07-20 8:11 ` [RESEND PATCH v5 1/2] sched: Add per_cpu cluster domain info and cpus_share_lowest_cache API Yicong Yang
@ 2022-07-20 8:11 ` Yicong Yang
2022-07-20 11:14 ` Peter Zijlstra
1 sibling, 1 reply; 9+ messages in thread
From: Yicong Yang @ 2022-07-20 8:11 UTC (permalink / raw)
To: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen,
gautham.shenoy, linux-kernel, linux-arm-kernel
Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
yangyicong, jonathan.cameron, ego, srikar, linuxarm, 21cnbao,
guodong.xu, hesham.almatary, john.garry, shenyang39,
kprateek.nayak, yu.c.chen, wuyun.abel
From: Barry Song <song.bao.hua@hisilicon.com>
For platforms having clusters like Kunpeng920, CPUs within the same cluster
have lower latency when synchronizing and accessing shared resources like
cache. Thus, this patch tries to find an idle cpu within the cluster of the
target CPU before scanning the whole LLC to gain lower latency.
Note neither Kunpeng920 nor x86 Jacobsville supports SMT, so this patch
doesn't consider SMT for this moment.
Testing has been done on Kunpeng920 by pinning tasks to one numa and two
numa. On Kunpeng920, Each numa has 8 clusters and each cluster has 4 CPUs.
With this patch, We noticed enhancement on tbench within one numa or cross
two numa.
On numa 0:
tip/core patched
Hmean 1 345.89 ( 0.00%) 393.96 * 13.90%*
Hmean 2 697.77 ( 0.00%) 786.04 * 12.65%*
Hmean 4 1392.51 ( 0.00%) 1570.26 * 12.76%*
Hmean 8 2800.61 ( 0.00%) 3083.98 * 10.12%*
Hmean 16 5514.27 ( 0.00%) 6116.00 * 10.91%*
Hmean 32 10869.81 ( 0.00%) 10782.98 * -0.80%*
Hmean 64 8315.22 ( 0.00%) 8519.84 * 2.46%*
Hmean 128 6324.47 ( 0.00%) 7159.35 * 13.20%*
On numa 0-1:
tip/core patched
Hmean 1 348.68 ( 0.00%) 387.91 * 11.25%*
Hmean 2 693.57 ( 0.00%) 774.91 * 11.73%*
Hmean 4 1369.26 ( 0.00%) 1475.48 * 7.76%*
Hmean 8 2772.99 ( 0.00%) 2984.61 * 7.63%*
Hmean 16 4825.83 ( 0.00%) 5873.13 * 21.70%*
Hmean 32 10250.32 ( 0.00%) 11688.06 * 14.03%*
Hmean 64 16309.51 ( 0.00%) 19889.48 * 21.95%*
Hmean 128 13022.32 ( 0.00%) 16005.64 * 22.91%*
Hmean 256 11335.79 ( 0.00%) 13821.74 * 21.93%*
Tested-by: Yicong Yang <yangyicong@hisilicon.com>
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>
---
kernel/sched/fair.c | 44 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 41 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 914096c5b1ae..25d2900ae221 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6377,6 +6377,40 @@ static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd
#endif /* CONFIG_SCHED_SMT */
+#ifdef CONFIG_SCHED_CLUSTER
+/*
+ * Scan the cluster domain for idle CPUs and clear cluster cpumask after scanning
+ */
+static inline int scan_cluster(struct task_struct *p, struct cpumask *cpus,
+ int target, int *nr)
+{
+ struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
+ int cpu, idle_cpu;
+
+ /* TODO: Support SMT system with cluster topology */
+ if (!sched_smt_active() && sd) {
+ for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
+ if (!--*nr)
+ return -1;
+
+ idle_cpu = __select_idle_cpu(cpu, p);
+ if ((unsigned int)idle_cpu < nr_cpumask_bits)
+ return idle_cpu;
+ }
+
+ cpumask_andnot(cpus, cpus, sched_domain_span(sd));
+ }
+
+ return -1;
+}
+#else
+static inline int scan_cluster(struct task_struct *p, struct cpumask *cpus,
+ int target, int *nr)
+{
+ return -1;
+}
+#endif
+
/*
* Scan the LLC domain for idle CPUs; this is dynamically regulated by
* comparing the average scan cost (tracked in sd->avg_scan_cost) against the
@@ -6437,6 +6471,10 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
}
}
+ idle_cpu = scan_cluster(p, cpus, target, &nr);
+ if ((unsigned int)idle_cpu < nr_cpumask_bits)
+ return idle_cpu;
+
for_each_cpu_wrap(cpu, cpus, target + 1) {
if (has_idle_core) {
i = select_idle_core(p, cpu, cpus, &idle_cpu);
@@ -6444,7 +6482,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
return i;
} else {
- if (!--nr)
+ if (--nr <= 0)
return -1;
idle_cpu = __select_idle_cpu(cpu, p);
if ((unsigned int)idle_cpu < nr_cpumask_bits)
@@ -6543,7 +6581,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
/*
* If the previous CPU is cache affine and idle, don't be stupid:
*/
- if (prev != target && cpus_share_cache(prev, target) &&
+ if (prev != target && cpus_share_lowest_cache(prev, target) &&
(available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
asym_fits_capacity(task_util, prev))
return prev;
@@ -6569,7 +6607,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
p->recent_used_cpu = prev;
if (recent_used_cpu != prev &&
recent_used_cpu != target &&
- cpus_share_cache(recent_used_cpu, target) &&
+ cpus_share_lowest_cache(recent_used_cpu, target) &&
(available_idle_cpu(recent_used_cpu) || sched_idle_cpu(recent_used_cpu)) &&
cpumask_test_cpu(p->recent_used_cpu, p->cpus_ptr) &&
asym_fits_capacity(task_util, recent_used_cpu)) {
--
2.24.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
2022-07-20 8:11 ` [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
@ 2022-07-20 11:14 ` Peter Zijlstra
2022-07-20 11:33 ` Barry Song
2022-07-21 9:38 ` Barry Song
0 siblings, 2 replies; 9+ messages in thread
From: Peter Zijlstra @ 2022-07-20 11:14 UTC (permalink / raw)
To: Yicong Yang
Cc: mingo, juri.lelli, vincent.guittot, tim.c.chen, gautham.shenoy,
linux-kernel, linux-arm-kernel, dietmar.eggemann, rostedt,
bsegall, bristot, prime.zeng, jonathan.cameron, ego, srikar,
linuxarm, 21cnbao, guodong.xu, hesham.almatary, john.garry,
shenyang39, kprateek.nayak, yu.c.chen, wuyun.abel
On Wed, Jul 20, 2022 at 04:11:50PM +0800, Yicong Yang wrote:
> + /* TODO: Support SMT system with cluster topology */
> + if (!sched_smt_active() && sd) {
> + for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
So that's no SMT and no wrap iteration..
Does something like this work?
---
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6437,6 +6437,30 @@ static int select_idle_cpu(struct task_s
}
}
+ if (IS_ENABLED(CONFIG_SCHED_CLUSTER) &&
+ static_branch_unlikely(&sched_cluster_active)) {
+ struct sched_domain *sdc = rcu_dereference(per_cpu(sd_cluster, target));
+ if (sdc) {
+ for_each_cpu_wrap(cpu, sched_domain_span(sdc), target + 1) {
+ if (!cpumask_test_cpu(cpu, cpus))
+ continue;
+
+ if (has_idle_core) {
+ i = select_idle_core(p, cpu, cpus, &idle_cpu);
+ if ((unsigned int)i < nr_cpumask_bits)
+ return i;
+ } else {
+ if (--nr <= 0)
+ return -1;
+ idle_cpu = __select_idle_cpu(cpu, p);
+ if ((unsigned int)idle_cpu < nr_cpumask_bits)
+ break;
+ }
+ }
+ cpumask_andnot(cpus, cpus, sched_domain_span(sdc));
+ }
+ }
+
for_each_cpu_wrap(cpu, cpus, target + 1) {
if (has_idle_core) {
i = select_idle_core(p, cpu, cpus, &idle_cpu);
@@ -6444,7 +6468,7 @@ static int select_idle_cpu(struct task_s
return i;
} else {
- if (!--nr)
+ if (--nr <= 0)
return -1;
idle_cpu = __select_idle_cpu(cpu, p);
if ((unsigned int)idle_cpu < nr_cpumask_bits)
@@ -6543,7 +6567,7 @@ static int select_idle_sibling(struct ta
/*
* If the previous CPU is cache affine and idle, don't be stupid:
*/
- if (prev != target && cpus_share_cache(prev, target) &&
+ if (prev != target && cpus_share_lowest_cache(prev, target) &&
(available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
asym_fits_capacity(task_util, prev))
return prev;
@@ -6569,7 +6593,7 @@ static int select_idle_sibling(struct ta
p->recent_used_cpu = prev;
if (recent_used_cpu != prev &&
recent_used_cpu != target &&
- cpus_share_cache(recent_used_cpu, target) &&
+ cpus_share_lowest_cache(recent_used_cpu, target) &&
(available_idle_cpu(recent_used_cpu) || sched_idle_cpu(recent_used_cpu)) &&
cpumask_test_cpu(p->recent_used_cpu, p->cpus_ptr) &&
asym_fits_capacity(task_util, recent_used_cpu)) {
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1813,7 +1813,9 @@ DECLARE_PER_CPU(struct sched_domain __rc
DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
+
extern struct static_key_false sched_asym_cpucapacity;
+extern struct static_key_false sched_cluster_active;
struct sched_group_capacity {
atomic_t ref;
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -670,7 +670,9 @@ DEFINE_PER_CPU(struct sched_domain_share
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
+
DEFINE_STATIC_KEY_FALSE(sched_asym_cpucapacity);
+DEFINE_STATIC_KEY_FALSE(sched_cluster_active);
static void update_top_cache_domain(int cpu)
{
@@ -2268,6 +2270,7 @@ build_sched_domains(const struct cpumask
struct rq *rq = NULL;
int i, ret = -ENOMEM;
bool has_asym = false;
+ bool has_cluster = false;
if (WARN_ON(cpumask_empty(cpu_map)))
goto error;
@@ -2289,6 +2292,7 @@ build_sched_domains(const struct cpumask
sd = build_sched_domain(tl, cpu_map, attr, sd, i);
has_asym |= sd->flags & SD_ASYM_CPUCAPACITY;
+ has_cluster |= sd->flags & SD_CLUSTER;
if (tl == sched_domain_topology)
*per_cpu_ptr(d.sd, i) = sd;
@@ -2399,6 +2403,9 @@ build_sched_domains(const struct cpumask
if (has_asym)
static_branch_inc_cpuslocked(&sched_asym_cpucapacity);
+ if (has_cluster)
+ static_branch_inc_cpuslocked(&sched_cluster_active);
+
if (rq && sched_debug_verbose) {
pr_info("root domain span: %*pbl (max cpu_capacity = %lu)\n",
cpumask_pr_args(cpu_map), rq->rd->max_cpu_capacity);
@@ -2498,6 +2505,9 @@ static void detach_destroy_domains(const
if (rcu_access_pointer(per_cpu(sd_asym_cpucapacity, cpu)))
static_branch_dec_cpuslocked(&sched_asym_cpucapacity);
+ if (rcu_access_pointer(per_cpu(sd_cluster, cpu)))
+ static_branch_dec_cpuslocked(&sched_cluster_active);
+
rcu_read_lock();
for_each_cpu(i, cpu_map)
cpu_attach_domain(NULL, &def_root_domain, i);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
2022-07-20 11:14 ` Peter Zijlstra
@ 2022-07-20 11:33 ` Barry Song
2022-07-21 9:38 ` Barry Song
1 sibling, 0 replies; 9+ messages in thread
From: Barry Song @ 2022-07-20 11:33 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Yicong Yang, Ingo Molnar, Juri Lelli, Vincent Guittot, Tim Chen,
Gautham R. Shenoy, LKML, LAK, Dietmar Eggemann, Steven Rostedt,
Ben Segall, Daniel Bristot de Oliveira, prime.zeng,
Jonathan Cameron, ego, Srikar Dronamraju, Linuxarm, Guodong Xu,
hesham.almatary, john.garry, Yang Shen, kprateek.nayak, Chen Yu,
wuyun.abel
On Wed, Jul 20, 2022 at 11:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Jul 20, 2022 at 04:11:50PM +0800, Yicong Yang wrote:
> > + /* TODO: Support SMT system with cluster topology */
> > + if (!sched_smt_active() && sd) {
> > + for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
>
> So that's no SMT and no wrap iteration..
>
> Does something like this work?
I guess it is ok. The main reason for me to disable smt from the first beginning
is that we don't really have a machine with both cluster and smt to
reach the code
for smt, aka has_idle_core=true. so I thought I was going to bring up
that branch
when I really have a machine. so, it is marked as TODO.
till now, neither kunpeng920 nor Jacobsville is smt.
But I don't mind if it is enabled now.
>
> ---
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6437,6 +6437,30 @@ static int select_idle_cpu(struct task_s
> }
> }
>
> + if (IS_ENABLED(CONFIG_SCHED_CLUSTER) &&
> + static_branch_unlikely(&sched_cluster_active)) {
> + struct sched_domain *sdc = rcu_dereference(per_cpu(sd_cluster, target));
> + if (sdc) {
> + for_each_cpu_wrap(cpu, sched_domain_span(sdc), target + 1) {
> + if (!cpumask_test_cpu(cpu, cpus))
> + continue;
> +
> + if (has_idle_core) {
> + i = select_idle_core(p, cpu, cpus, &idle_cpu);
> + if ((unsigned int)i < nr_cpumask_bits)
> + return i;
> + } else {
> + if (--nr <= 0)
> + return -1;
> + idle_cpu = __select_idle_cpu(cpu, p);
> + if ((unsigned int)idle_cpu < nr_cpumask_bits)
> + break;
> + }
> + }
> + cpumask_andnot(cpus, cpus, sched_domain_span(sdc));
> + }
> + }
> +
> for_each_cpu_wrap(cpu, cpus, target + 1) {
> if (has_idle_core) {
> i = select_idle_core(p, cpu, cpus, &idle_cpu);
> @@ -6444,7 +6468,7 @@ static int select_idle_cpu(struct task_s
> return i;
>
> } else {
> - if (!--nr)
> + if (--nr <= 0)
> return -1;
> idle_cpu = __select_idle_cpu(cpu, p);
> if ((unsigned int)idle_cpu < nr_cpumask_bits)
> @@ -6543,7 +6567,7 @@ static int select_idle_sibling(struct ta
> /*
> * If the previous CPU is cache affine and idle, don't be stupid:
> */
> - if (prev != target && cpus_share_cache(prev, target) &&
> + if (prev != target && cpus_share_lowest_cache(prev, target) &&
> (available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
> asym_fits_capacity(task_util, prev))
> return prev;
> @@ -6569,7 +6593,7 @@ static int select_idle_sibling(struct ta
> p->recent_used_cpu = prev;
> if (recent_used_cpu != prev &&
> recent_used_cpu != target &&
> - cpus_share_cache(recent_used_cpu, target) &&
> + cpus_share_lowest_cache(recent_used_cpu, target) &&
> (available_idle_cpu(recent_used_cpu) || sched_idle_cpu(recent_used_cpu)) &&
> cpumask_test_cpu(p->recent_used_cpu, p->cpus_ptr) &&
> asym_fits_capacity(task_util, recent_used_cpu)) {
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1813,7 +1813,9 @@ DECLARE_PER_CPU(struct sched_domain __rc
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
> +
> extern struct static_key_false sched_asym_cpucapacity;
> +extern struct static_key_false sched_cluster_active;
>
> struct sched_group_capacity {
> atomic_t ref;
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -670,7 +670,9 @@ DEFINE_PER_CPU(struct sched_domain_share
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
> +
> DEFINE_STATIC_KEY_FALSE(sched_asym_cpucapacity);
> +DEFINE_STATIC_KEY_FALSE(sched_cluster_active);
>
> static void update_top_cache_domain(int cpu)
> {
> @@ -2268,6 +2270,7 @@ build_sched_domains(const struct cpumask
> struct rq *rq = NULL;
> int i, ret = -ENOMEM;
> bool has_asym = false;
> + bool has_cluster = false;
>
> if (WARN_ON(cpumask_empty(cpu_map)))
> goto error;
> @@ -2289,6 +2292,7 @@ build_sched_domains(const struct cpumask
> sd = build_sched_domain(tl, cpu_map, attr, sd, i);
>
> has_asym |= sd->flags & SD_ASYM_CPUCAPACITY;
> + has_cluster |= sd->flags & SD_CLUSTER;
>
> if (tl == sched_domain_topology)
> *per_cpu_ptr(d.sd, i) = sd;
> @@ -2399,6 +2403,9 @@ build_sched_domains(const struct cpumask
> if (has_asym)
> static_branch_inc_cpuslocked(&sched_asym_cpucapacity);
>
> + if (has_cluster)
> + static_branch_inc_cpuslocked(&sched_cluster_active);
> +
> if (rq && sched_debug_verbose) {
> pr_info("root domain span: %*pbl (max cpu_capacity = %lu)\n",
> cpumask_pr_args(cpu_map), rq->rd->max_cpu_capacity);
> @@ -2498,6 +2505,9 @@ static void detach_destroy_domains(const
> if (rcu_access_pointer(per_cpu(sd_asym_cpucapacity, cpu)))
> static_branch_dec_cpuslocked(&sched_asym_cpucapacity);
>
> + if (rcu_access_pointer(per_cpu(sd_cluster, cpu)))
> + static_branch_dec_cpuslocked(&sched_cluster_active);
> +
> rcu_read_lock();
> for_each_cpu(i, cpu_map)
> cpu_attach_domain(NULL, &def_root_domain, i);
Thanks
Barry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v5 1/2] sched: Add per_cpu cluster domain info and cpus_share_lowest_cache API
2022-07-20 8:11 ` [RESEND PATCH v5 1/2] sched: Add per_cpu cluster domain info and cpus_share_lowest_cache API Yicong Yang
@ 2022-07-20 13:56 ` Vincent Guittot
0 siblings, 0 replies; 9+ messages in thread
From: Vincent Guittot @ 2022-07-20 13:56 UTC (permalink / raw)
To: Yicong Yang
Cc: peterz, mingo, juri.lelli, tim.c.chen, gautham.shenoy,
linux-kernel, linux-arm-kernel, dietmar.eggemann, rostedt,
bsegall, bristot, prime.zeng, jonathan.cameron, ego, srikar,
linuxarm, 21cnbao, guodong.xu, hesham.almatary, john.garry,
shenyang39, kprateek.nayak, yu.c.chen, wuyun.abel
On Wed, 20 Jul 2022 at 10:14, Yicong Yang <yangyicong@hisilicon.com> wrote:
>
> From: Barry Song <song.bao.hua@hisilicon.com>
>
> Add per-cpu cluster domain info and cpus_share_lowest_cache() API.
> This is the preparation for the optimization of select_idle_cpu()
> on platforms with cluster scheduler level.
Don't know why but your patchset ended up in my spam. Peterz
resurected it by replying to patch 2
>
> Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> include/linux/sched/sd_flags.h | 7 +++++++
> include/linux/sched/topology.h | 8 +++++++-
> kernel/sched/core.c | 12 ++++++++++++
> kernel/sched/sched.h | 2 ++
> kernel/sched/topology.c | 15 +++++++++++++++
> 5 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/sched/sd_flags.h b/include/linux/sched/sd_flags.h
> index 57bde66d95f7..42ed454e8b18 100644
> --- a/include/linux/sched/sd_flags.h
> +++ b/include/linux/sched/sd_flags.h
> @@ -109,6 +109,13 @@ SD_FLAG(SD_ASYM_CPUCAPACITY_FULL, SDF_SHARED_PARENT | SDF_NEEDS_GROUPS)
> */
> SD_FLAG(SD_SHARE_CPUCAPACITY, SDF_SHARED_CHILD | SDF_NEEDS_GROUPS)
>
> +/*
> + * Domain members share CPU cluster (LLC tags or L2 cache)
> + *
> + * NEEDS_GROUPS: Clusters are shared between groups.
> + */
> +SD_FLAG(SD_CLUSTER, SDF_NEEDS_GROUPS)
> +
> /*
> * Domain members share CPU package resources (i.e. caches)
> *
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index 816df6cc444e..c0d21667ddf3 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -45,7 +45,7 @@ static inline int cpu_smt_flags(void)
> #ifdef CONFIG_SCHED_CLUSTER
> static inline int cpu_cluster_flags(void)
> {
> - return SD_SHARE_PKG_RESOURCES;
> + return SD_CLUSTER | SD_SHARE_PKG_RESOURCES;
> }
> #endif
>
> @@ -179,6 +179,7 @@ cpumask_var_t *alloc_sched_domains(unsigned int ndoms);
> void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms);
>
> bool cpus_share_cache(int this_cpu, int that_cpu);
> +bool cpus_share_lowest_cache(int this_cpu, int that_cpu);
>
> typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
> typedef int (*sched_domain_flags_f)(void);
> @@ -232,6 +233,11 @@ static inline bool cpus_share_cache(int this_cpu, int that_cpu)
> return true;
> }
>
> +static inline bool cpus_share_lowest_cache(int this_cpu, int that_cpu)
> +{
> + return true;
> +}
> +
> #endif /* !CONFIG_SMP */
>
> #if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_CPU_FREQ_GOV_SCHEDUTIL)
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a463dbc92fcd..96109ad82694 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3802,6 +3802,18 @@ bool cpus_share_cache(int this_cpu, int that_cpu)
> return per_cpu(sd_llc_id, this_cpu) == per_cpu(sd_llc_id, that_cpu);
> }
>
> +/*
> + * Whether CPUs are share lowest cache, which means LLC on non-cluster
> + * machines and LLC tag or L2 on machines with clusters.
> + */
> +bool cpus_share_lowest_cache(int this_cpu, int that_cpu)
> +{
> + if (this_cpu == that_cpu)
> + return true;
> +
> + return per_cpu(sd_lowest_cache_id, this_cpu) == per_cpu(sd_lowest_cache_id, that_cpu);
> +}
> +
> static inline bool ttwu_queue_cond(int cpu)
> {
> /*
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 73ae32898f25..845cd029d572 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1802,7 +1802,9 @@ static inline struct sched_domain *lowest_flag_domain(int cpu, int flag)
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_llc);
> DECLARE_PER_CPU(int, sd_llc_size);
> DECLARE_PER_CPU(int, sd_llc_id);
> +DECLARE_PER_CPU(int, sd_lowest_cache_id);
> DECLARE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
> +DECLARE_PER_CPU(struct sched_domain __rcu *, sd_cluster);
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 8739c2a5a54e..8ab27c0d6d1f 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -664,6 +664,8 @@ static void destroy_sched_domains(struct sched_domain *sd)
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_llc);
> DEFINE_PER_CPU(int, sd_llc_size);
> DEFINE_PER_CPU(int, sd_llc_id);
> +DEFINE_PER_CPU(int, sd_lowest_cache_id);
> +DEFINE_PER_CPU(struct sched_domain __rcu *, sd_cluster);
> DEFINE_PER_CPU(struct sched_domain_shared __rcu *, sd_llc_shared);
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
> @@ -689,6 +691,18 @@ static void update_top_cache_domain(int cpu)
> per_cpu(sd_llc_id, cpu) = id;
> rcu_assign_pointer(per_cpu(sd_llc_shared, cpu), sds);
>
> + sd = lowest_flag_domain(cpu, SD_CLUSTER);
> + if (sd)
> + id = cpumask_first(sched_domain_span(sd));
> + rcu_assign_pointer(per_cpu(sd_cluster, cpu), sd);
> +
> + /*
> + * This assignment should be placed after the sd_llc_id as
> + * we want this id equals to cluster id on cluster machines
> + * but equals to LLC id on non-Cluster machines.
> + */
> + per_cpu(sd_lowest_cache_id, cpu) = id;
> +
> sd = lowest_flag_domain(cpu, SD_NUMA);
> rcu_assign_pointer(per_cpu(sd_numa, cpu), sd);
>
> @@ -1532,6 +1546,7 @@ static struct cpumask ***sched_domains_numa_masks;
> */
> #define TOPOLOGY_SD_FLAGS \
> (SD_SHARE_CPUCAPACITY | \
> + SD_CLUSTER | \
> SD_SHARE_PKG_RESOURCES | \
> SD_NUMA | \
> SD_ASYM_PACKING)
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
2022-07-20 11:14 ` Peter Zijlstra
2022-07-20 11:33 ` Barry Song
@ 2022-07-21 9:38 ` Barry Song
2022-07-21 10:33 ` Peter Zijlstra
1 sibling, 1 reply; 9+ messages in thread
From: Barry Song @ 2022-07-21 9:38 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Yicong Yang, Ingo Molnar, Juri Lelli, Vincent Guittot, Tim Chen,
Gautham R. Shenoy, LKML, LAK, Dietmar Eggemann, Steven Rostedt,
Ben Segall, Daniel Bristot de Oliveira, prime.zeng,
Jonathan Cameron, ego, Srikar Dronamraju, Linuxarm, Guodong Xu,
hesham.almatary, john.garry, Yang Shen, kprateek.nayak, Chen Yu,
wuyun.abel
On Wed, Jul 20, 2022 at 11:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Jul 20, 2022 at 04:11:50PM +0800, Yicong Yang wrote:
> > + /* TODO: Support SMT system with cluster topology */
> > + if (!sched_smt_active() && sd) {
> > + for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
>
> So that's no SMT and no wrap iteration..
>
> Does something like this work?
>
> ---
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6437,6 +6437,30 @@ static int select_idle_cpu(struct task_s
> }
> }
>
> + if (IS_ENABLED(CONFIG_SCHED_CLUSTER) &&
> + static_branch_unlikely(&sched_cluster_active)) {
> + struct sched_domain *sdc = rcu_dereference(per_cpu(sd_cluster, target));
> + if (sdc) {
> + for_each_cpu_wrap(cpu, sched_domain_span(sdc), target + 1) {
> + if (!cpumask_test_cpu(cpu, cpus))
> + continue;
> +
> + if (has_idle_core) {
> + i = select_idle_core(p, cpu, cpus, &idle_cpu);
> + if ((unsigned int)i < nr_cpumask_bits)
> + return i;
> + } else {
> + if (--nr <= 0)
> + return -1;
> + idle_cpu = __select_idle_cpu(cpu, p);
> + if ((unsigned int)idle_cpu < nr_cpumask_bits)
> + break;
Guess here it should be "return idle_cpu", but not "break". as "break"
will make us scan more
other cpus outside the cluster if we have found idle_cpu within the cluster.
Yicong,
Please test Peter's code with the above change.
> + }
> + }
> + cpumask_andnot(cpus, cpus, sched_domain_span(sdc));
> + }
> + }
> +
> for_each_cpu_wrap(cpu, cpus, target + 1) {
> if (has_idle_core) {
> i = select_idle_core(p, cpu, cpus, &idle_cpu);
> @@ -6444,7 +6468,7 @@ static int select_idle_cpu(struct task_s
> return i;
>
> } else {
> - if (!--nr)
> + if (--nr <= 0)
> return -1;
> idle_cpu = __select_idle_cpu(cpu, p);
> if ((unsigned int)idle_cpu < nr_cpumask_bits)
> @@ -6543,7 +6567,7 @@ static int select_idle_sibling(struct ta
> /*
> * If the previous CPU is cache affine and idle, don't be stupid:
> */
> - if (prev != target && cpus_share_cache(prev, target) &&
> + if (prev != target && cpus_share_lowest_cache(prev, target) &&
> (available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
> asym_fits_capacity(task_util, prev))
> return prev;
> @@ -6569,7 +6593,7 @@ static int select_idle_sibling(struct ta
> p->recent_used_cpu = prev;
> if (recent_used_cpu != prev &&
> recent_used_cpu != target &&
> - cpus_share_cache(recent_used_cpu, target) &&
> + cpus_share_lowest_cache(recent_used_cpu, target) &&
> (available_idle_cpu(recent_used_cpu) || sched_idle_cpu(recent_used_cpu)) &&
> cpumask_test_cpu(p->recent_used_cpu, p->cpus_ptr) &&
> asym_fits_capacity(task_util, recent_used_cpu)) {
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1813,7 +1813,9 @@ DECLARE_PER_CPU(struct sched_domain __rc
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_numa);
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
> DECLARE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
> +
> extern struct static_key_false sched_asym_cpucapacity;
> +extern struct static_key_false sched_cluster_active;
>
> struct sched_group_capacity {
> atomic_t ref;
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -670,7 +670,9 @@ DEFINE_PER_CPU(struct sched_domain_share
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_numa);
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_packing);
> DEFINE_PER_CPU(struct sched_domain __rcu *, sd_asym_cpucapacity);
> +
> DEFINE_STATIC_KEY_FALSE(sched_asym_cpucapacity);
> +DEFINE_STATIC_KEY_FALSE(sched_cluster_active);
>
> static void update_top_cache_domain(int cpu)
> {
> @@ -2268,6 +2270,7 @@ build_sched_domains(const struct cpumask
> struct rq *rq = NULL;
> int i, ret = -ENOMEM;
> bool has_asym = false;
> + bool has_cluster = false;
>
> if (WARN_ON(cpumask_empty(cpu_map)))
> goto error;
> @@ -2289,6 +2292,7 @@ build_sched_domains(const struct cpumask
> sd = build_sched_domain(tl, cpu_map, attr, sd, i);
>
> has_asym |= sd->flags & SD_ASYM_CPUCAPACITY;
> + has_cluster |= sd->flags & SD_CLUSTER;
>
> if (tl == sched_domain_topology)
> *per_cpu_ptr(d.sd, i) = sd;
> @@ -2399,6 +2403,9 @@ build_sched_domains(const struct cpumask
> if (has_asym)
> static_branch_inc_cpuslocked(&sched_asym_cpucapacity);
>
> + if (has_cluster)
> + static_branch_inc_cpuslocked(&sched_cluster_active);
> +
> if (rq && sched_debug_verbose) {
> pr_info("root domain span: %*pbl (max cpu_capacity = %lu)\n",
> cpumask_pr_args(cpu_map), rq->rd->max_cpu_capacity);
> @@ -2498,6 +2505,9 @@ static void detach_destroy_domains(const
> if (rcu_access_pointer(per_cpu(sd_asym_cpucapacity, cpu)))
> static_branch_dec_cpuslocked(&sched_asym_cpucapacity);
>
> + if (rcu_access_pointer(per_cpu(sd_cluster, cpu)))
> + static_branch_dec_cpuslocked(&sched_cluster_active);
> +
> rcu_read_lock();
> for_each_cpu(i, cpu_map)
> cpu_attach_domain(NULL, &def_root_domain, i);
Thanks
Barry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
2022-07-21 9:38 ` Barry Song
@ 2022-07-21 10:33 ` Peter Zijlstra
2022-07-21 12:42 ` Yicong Yang
0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2022-07-21 10:33 UTC (permalink / raw)
To: Barry Song
Cc: Yicong Yang, Ingo Molnar, Juri Lelli, Vincent Guittot, Tim Chen,
Gautham R. Shenoy, LKML, LAK, Dietmar Eggemann, Steven Rostedt,
Ben Segall, Daniel Bristot de Oliveira, prime.zeng,
Jonathan Cameron, ego, Srikar Dronamraju, Linuxarm, Guodong Xu,
hesham.almatary, john.garry, Yang Shen, kprateek.nayak, Chen Yu,
wuyun.abel
On Thu, Jul 21, 2022 at 09:38:04PM +1200, Barry Song wrote:
> On Wed, Jul 20, 2022 at 11:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Jul 20, 2022 at 04:11:50PM +0800, Yicong Yang wrote:
> > > + /* TODO: Support SMT system with cluster topology */
> > > + if (!sched_smt_active() && sd) {
> > > + for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> >
> > So that's no SMT and no wrap iteration..
> >
> > Does something like this work?
> >
> > ---
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -6437,6 +6437,30 @@ static int select_idle_cpu(struct task_s
> > }
> > }
> >
> > + if (IS_ENABLED(CONFIG_SCHED_CLUSTER) &&
> > + static_branch_unlikely(&sched_cluster_active)) {
> > + struct sched_domain *sdc = rcu_dereference(per_cpu(sd_cluster, target));
> > + if (sdc) {
> > + for_each_cpu_wrap(cpu, sched_domain_span(sdc), target + 1) {
> > + if (!cpumask_test_cpu(cpu, cpus))
> > + continue;
> > +
> > + if (has_idle_core) {
> > + i = select_idle_core(p, cpu, cpus, &idle_cpu);
> > + if ((unsigned int)i < nr_cpumask_bits)
> > + return i;
> > + } else {
> > + if (--nr <= 0)
> > + return -1;
> > + idle_cpu = __select_idle_cpu(cpu, p);
> > + if ((unsigned int)idle_cpu < nr_cpumask_bits)
> > + break;
>
> Guess here it should be "return idle_cpu", but not "break". as "break"
> will make us scan more
> other cpus outside the cluster if we have found idle_cpu within the cluster.
>
> Yicong,
> Please test Peter's code with the above change.
Indeed. Sorry for that.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
2022-07-21 10:33 ` Peter Zijlstra
@ 2022-07-21 12:42 ` Yicong Yang
0 siblings, 0 replies; 9+ messages in thread
From: Yicong Yang @ 2022-07-21 12:42 UTC (permalink / raw)
To: Peter Zijlstra, Barry Song
Cc: yangyicong, Ingo Molnar, Juri Lelli, Vincent Guittot, Tim Chen,
Gautham R. Shenoy, LKML, LAK, Dietmar Eggemann, Steven Rostedt,
Ben Segall, Daniel Bristot de Oliveira, prime.zeng,
Jonathan Cameron, ego, Srikar Dronamraju, Linuxarm, Guodong Xu,
hesham.almatary, john.garry, Yang Shen, kprateek.nayak, Chen Yu,
wuyun.abel
On 2022/7/21 18:33, Peter Zijlstra wrote:
> On Thu, Jul 21, 2022 at 09:38:04PM +1200, Barry Song wrote:
>> On Wed, Jul 20, 2022 at 11:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
>>>
>>> On Wed, Jul 20, 2022 at 04:11:50PM +0800, Yicong Yang wrote:
>>>> + /* TODO: Support SMT system with cluster topology */
>>>> + if (!sched_smt_active() && sd) {
>>>> + for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
>>>
>>> So that's no SMT and no wrap iteration..
>>>
>>> Does something like this work?
>>>
>>> ---
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -6437,6 +6437,30 @@ static int select_idle_cpu(struct task_s
>>> }
>>> }
>>>
>>> + if (IS_ENABLED(CONFIG_SCHED_CLUSTER) &&
>>> + static_branch_unlikely(&sched_cluster_active)) {
>>> + struct sched_domain *sdc = rcu_dereference(per_cpu(sd_cluster, target));
>>> + if (sdc) {
>>> + for_each_cpu_wrap(cpu, sched_domain_span(sdc), target + 1) {
>>> + if (!cpumask_test_cpu(cpu, cpus))
>>> + continue;
>>> +
>>> + if (has_idle_core) {
>>> + i = select_idle_core(p, cpu, cpus, &idle_cpu);
>>> + if ((unsigned int)i < nr_cpumask_bits)
>>> + return i;
>>> + } else {
>>> + if (--nr <= 0)
>>> + return -1;
>>> + idle_cpu = __select_idle_cpu(cpu, p);
>>> + if ((unsigned int)idle_cpu < nr_cpumask_bits)
>>> + break;
>>
>> Guess here it should be "return idle_cpu", but not "break". as "break"
>> will make us scan more
>> other cpus outside the cluster if we have found idle_cpu within the cluster.
>>
That can explain why the performance regress when underload.
>> Yicong,
>> Please test Peter's code with the above change.
>
> Indeed. Sorry for that.
>
The performance's still positive based on the tip/sched/core used in this patch's commit.
70fb5ccf2ebb ("sched/fair: Introduce SIS_UTIL to search idle CPU based on sum of util_avg").
On numa 0:
tip/core patched
Hmean 1 345.89 ( 0.00%) 398.43 * 15.19%*
Hmean 2 697.77 ( 0.00%) 794.40 * 13.85%*
Hmean 4 1392.51 ( 0.00%) 1577.60 * 13.29%*
Hmean 8 2800.61 ( 0.00%) 3118.38 * 11.35%*
Hmean 16 5514.27 ( 0.00%) 6124.51 * 11.07%*
Hmean 32 10869.81 ( 0.00%) 10690.97 * -1.65%*
Hmean 64 8315.22 ( 0.00%) 8520.73 * 2.47%*
Hmean 128 6324.47 ( 0.00%) 7253.65 * 14.69%*
On numa 0-1:
tip/core patched
Hmean 1 348.68 ( 0.00%) 397.74 * 14.07%*
Hmean 2 693.57 ( 0.00%) 795.54 * 14.70%*
Hmean 4 1369.26 ( 0.00%) 1548.72 * 13.11%*
Hmean 8 2772.99 ( 0.00%) 3055.54 * 10.19%*
Hmean 16 4825.83 ( 0.00%) 5936.64 * 23.02%*
Hmean 32 10250.32 ( 0.00%) 11780.59 * 14.93%*
Hmean 64 16309.51 ( 0.00%) 19864.38 * 21.80%*
Hmean 128 13022.32 ( 0.00%) 16365.43 * 25.67%*
Hmean 256 11335.79 ( 0.00%) 13991.33 * 23.43%*
Hi Peter,
Do you want me to respin a v6 based on your change?
Thanks,
Yicong
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-07-21 12:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-20 8:11 [RESEND PATCH v5 0/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
2022-07-20 8:11 ` [RESEND PATCH v5 1/2] sched: Add per_cpu cluster domain info and cpus_share_lowest_cache API Yicong Yang
2022-07-20 13:56 ` Vincent Guittot
2022-07-20 8:11 ` [RESEND PATCH v5 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
2022-07-20 11:14 ` Peter Zijlstra
2022-07-20 11:33 ` Barry Song
2022-07-21 9:38 ` Barry Song
2022-07-21 10:33 ` Peter Zijlstra
2022-07-21 12:42 ` Yicong Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).