linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] sched/fair: Wake task within the cluster when possible
@ 2022-01-26  8:09 Yicong Yang
  2022-01-26  8:09 ` [PATCH v2 1/2] sched: Add per_cpu cluster domain info and cpus_share_resources API Yicong Yang
  2022-01-26  8:09 ` [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
  0 siblings, 2 replies; 27+ messages in thread
From: Yicong Yang @ 2022-01-26  8:09 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,
	song.bao.hua, guodong.xu

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 v1:
- regain the performance data based on v5.17-rc1
- rename cpus_share_cluster to cpus_share_resources per Vincent and Gautham, thanks!

Barry Song (2):
  sched: Add per_cpu cluster domain info and cpus_share_resources 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            | 46 +++++++++++++++++++++++++++++++---
 kernel/sched/sched.h           |  2 ++
 kernel/sched/topology.c        | 15 +++++++++++
 6 files changed, 85 insertions(+), 5 deletions(-)

-- 
2.24.0


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

* [PATCH v2 1/2] sched: Add per_cpu cluster domain info and cpus_share_resources API
  2022-01-26  8:09 [PATCH v2 0/2] sched/fair: Wake task within the cluster when possible Yicong Yang
@ 2022-01-26  8:09 ` Yicong Yang
  2022-01-27 15:26   ` Gautham R. Shenoy
  2022-01-26  8:09 ` [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
  1 sibling, 1 reply; 27+ messages in thread
From: Yicong Yang @ 2022-01-26  8:09 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,
	song.bao.hua, guodong.xu

From: Barry Song <song.bao.hua@hisilicon.com>

Add per-cpu cluster domain info and cpus_share_resources() API.
This is the preparation for the optimization of select_idle_cpu()
on platforms with cluster scheduler level.

Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
Signed-off-by: Yicong Yang <yangyicong@hisilicon.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 8054641c0a7b..2f84fdf00481 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
 
@@ -177,6 +177,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_resources(int this_cpu, int that_cpu);
 
 typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
 typedef int (*sched_domain_flags_f)(void);
@@ -230,6 +231,11 @@ static inline bool cpus_share_cache(int this_cpu, int that_cpu)
 	return true;
 }
 
+static inline bool cpus_share_resources(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 848eaa0efe0e..7b203a6d96b2 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3746,6 +3746,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 cache resources, which means LLC on non-cluster
+ * machines and LLC tag or L2 on machines with clusters.
+ */
+bool cpus_share_resources(int this_cpu, int that_cpu)
+{
+	if (this_cpu == that_cpu)
+		return true;
+
+	return per_cpu(sd_share_id, this_cpu) == per_cpu(sd_share_id, that_cpu);
+}
+
 static inline bool ttwu_queue_cond(int cpu, int wake_flags)
 {
 	/*
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index de53be905739..d04b342cc28d 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1765,7 +1765,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_share_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 d201a7052a29..408fede6e732 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -644,6 +644,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_share_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);
@@ -669,6 +671,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_share_id, cpu) = id;
+
 	sd = lowest_flag_domain(cpu, SD_NUMA);
 	rcu_assign_pointer(per_cpu(sd_numa, cpu), sd);
 
@@ -1514,6 +1528,7 @@ static unsigned long __read_mostly *sched_numa_onlined_nodes;
  */
 #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] 27+ messages in thread

* [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-26  8:09 [PATCH v2 0/2] sched/fair: Wake task within the cluster when possible Yicong Yang
  2022-01-26  8:09 ` [PATCH v2 1/2] sched: Add per_cpu cluster domain info and cpus_share_resources API Yicong Yang
@ 2022-01-26  8:09 ` Yicong Yang
  2022-01-27  1:14   ` Tim Chen
  2022-01-27 15:41   ` Gautham R. Shenoy
  1 sibling, 2 replies; 27+ messages in thread
From: Yicong Yang @ 2022-01-26  8:09 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,
	song.bao.hua, guodong.xu

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:
                            5.17-rc1                patched
Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)

On numa 0-1:
                            5.17-rc1                patched
Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*

This will also help to improve the MySQL. With MySQL server
running on numa 0 and client running on numa 1, both QPS and
latency is imporved on read-write case:
                        5.17-rc1        patched
QPS-16threads        143333.2633    145077.4033(+1.22%)
QPS-24threads        195085.9367    202719.6133(+3.91%)
QPS-32threads        241165.6867      249020.74(+3.26%)
QPS-64threads        244586.8433    253387.7567(+3.60%)
avg-lat-16threads           2.23           2.19(+1.19%)
avg-lat-24threads           2.46           2.36(+3.79%)
avg-lat-36threads           2.66           2.57(+3.26%)
avg-lat-64threads           5.23           5.05(+3.44%)

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>
---
 kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5146163bfabb..2f84a933aedd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6262,12 +6262,46 @@ 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, int prev_cpu, int target)
+{
+	struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
+	struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
+	int cpu, idle_cpu;
+
+	/* TODO: Support SMT case while a machine with both cluster and SMT born */
+	if (!sched_smt_active() && sd) {
+		for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
+			idle_cpu = __select_idle_cpu(cpu, p);
+			if ((unsigned int)idle_cpu < nr_cpumask_bits)
+				return idle_cpu;
+		}
+
+		/* Don't ping-pong tasks in and out cluster frequently */
+		if (cpus_share_resources(target, prev_cpu))
+			return target;
+
+		cpumask_andnot(cpus, cpus, sched_domain_span(sd));
+	}
+
+	return -1;
+}
+#else
+static inline int scan_cluster(struct task_struct *p, int prev_cpu, int target)
+{
+	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
  * average idle time for this rq (as found in rq->avg_idle).
  */
-static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool has_idle_core, int target)
+static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool has_idle_core, int prev_cpu, int target)
 {
 	struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
 	int i, cpu, idle_cpu = -1, nr = INT_MAX;
@@ -6282,6 +6316,10 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
 
 	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
 
+	idle_cpu = scan_cluster(p, prev_cpu, target);
+	if ((unsigned int)idle_cpu < nr_cpumask_bits)
+		return idle_cpu;
+
 	if (sched_feat(SIS_PROP) && !has_idle_core) {
 		u64 avg_cost, avg_idle, span_avg;
 		unsigned long now = jiffies;
@@ -6416,7 +6454,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_resources(prev, target) &&
 	    (available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
 	    asym_fits_capacity(task_util, prev))
 		return prev;
@@ -6442,7 +6480,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_resources(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)) {
@@ -6483,7 +6521,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
 		}
 	}
 
-	i = select_idle_cpu(p, sd, has_idle_core, target);
+	i = select_idle_cpu(p, sd, has_idle_core, prev, target);
 	if ((unsigned)i < nr_cpumask_bits)
 		return i;
 
-- 
2.24.0


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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-26  8:09 ` [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
@ 2022-01-27  1:14   ` Tim Chen
  2022-01-27  2:02     ` Yicong Yang
  2022-01-27 15:41   ` Gautham R. Shenoy
  1 sibling, 1 reply; 27+ messages in thread
From: Tim Chen @ 2022-01-27  1:14 UTC (permalink / raw)
  To: Yicong Yang, peterz, mingo, juri.lelli, vincent.guittot,
	gautham.shenoy, linux-kernel, linux-arm-kernel
  Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
	jonathan.cameron, ego, srikar, linuxarm, 21cnbao, song.bao.hua,
	guodong.xu

On Wed, 2022-01-26 at 16:09 +0800, Yicong Yang wrote:
> 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:
>                             5.17-rc1                patched
> Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> 
> On numa 0-1:
>                             5.17-rc1                patched
> Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> 
> This will also help to improve the MySQL. With MySQL server
> running on numa 0 and client running on numa 1, both QPS and
> latency is imporved on read-write case:
>                         5.17-rc1        patched
> QPS-16threads        143333.2633    145077.4033(+1.22%)
> QPS-24threads        195085.9367    202719.6133(+3.91%)
> QPS-32threads        241165.6867      249020.74(+3.26%)
> QPS-64threads        244586.8433    253387.7567(+3.60%)
> avg-lat-16threads           2.23           2.19(+1.19%)
> avg-lat-24threads           2.46           2.36(+3.79%)
> avg-lat-36threads           2.66           2.57(+3.26%)
> avg-lat-64threads           5.23           5.05(+3.44%)
> 
> 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>
> ---
>  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++
> ----
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 5146163bfabb..2f84a933aedd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6262,12 +6262,46 @@ 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, int prev_cpu,
> int target)
> +{
> +	struct cpumask *cpus =
> this_cpu_cpumask_var_ptr(select_idle_mask);
> +	struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster,
> target));
> +	int cpu, idle_cpu;
> +
> +	/* TODO: Support SMT case while a machine with both cluster and
> SMT born */

This is probably a clearer comment

	/* TODO: Support SMT system with cluster topology */

> +	if (!sched_smt_active() && sd) {
> +		for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> +			idle_cpu = __select_idle_cpu(cpu, p);
>   */
> -static int select_idle_cpu(struct task_struct *p, struct
> sched_domain *sd, bool has_idle_core, int target)
> +static int select_idle_cpu(struct task_struct *p, struct
> sched_domain *sd, bool has_idle_core, int prev_cpu, int target)
>  {
>  	struct cpumask *cpus =
> this_cpu_cpumask_var_ptr(select_idle_mask);
>  	int i, cpu, idle_cpu = -1, nr = INT_MAX;
> @@ -6282,6 +6316,10 @@ static int select_idle_cpu(struct task_struct
> *p, struct sched_domain *sd, bool
>  
>  	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
>  
> +	idle_cpu = scan_cluster(p, prev_cpu, target);

Shouldn't "cpus" from 

cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);

be passed to scan_cluster, to make sure that the cpu returned is 
in the affinity mask of the task? I don't see p->cpus_ptr
being checked in scan_cluster to make sure the cpu found is in the
affinity mask.

Tim


> +	if ((unsigned int)idle_cpu < nr_cpumask_bits)
> +		return idle_cpu;
> +
>  	if (sched_feat(SIS_PROP) && !has_idle_core) {
>  		u64 avg_cost, avg_idle, span_avg;
>  		unsigned long now = jiffies;
> @@ -6416,7 +6454,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_resources(prev, target) &&
>  	    (available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
>  	    asym_fits_capacity(task_util, prev))
>  		return prev;
> @@ -6442,7 +6480,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_resources(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)) {
> @@ -6483,7 +6521,7 @@ static int select_idle_sibling(struct
> task_struct *p, int prev, int target)
>  		}
>  	}
>  
> -	i = select_idle_cpu(p, sd, has_idle_core, target);
> +	i = select_idle_cpu(p, sd, has_idle_core, prev, target);
>  	if ((unsigned)i < nr_cpumask_bits)
>  		return i;
>  


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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-27  1:14   ` Tim Chen
@ 2022-01-27  2:02     ` Yicong Yang
  2022-01-27  2:30       ` Tim Chen
  0 siblings, 1 reply; 27+ messages in thread
From: Yicong Yang @ 2022-01-27  2:02 UTC (permalink / raw)
  To: Tim Chen, Yicong Yang, peterz, mingo, juri.lelli,
	vincent.guittot, gautham.shenoy, linux-kernel, linux-arm-kernel
  Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
	jonathan.cameron, ego, srikar, linuxarm, 21cnbao, song.bao.hua,
	guodong.xu

On 2022/1/27 9:14, Tim Chen wrote:
> On Wed, 2022-01-26 at 16:09 +0800, Yicong Yang wrote:
>> 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:
>>                             5.17-rc1                patched
>> Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
>> Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
>> Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
>> Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
>> Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
>> Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
>> Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
>> Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
>>
>> On numa 0-1:
>>                             5.17-rc1                patched
>> Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
>> Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
>> Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
>> Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
>> Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
>> Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
>> Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
>> Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
>> Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
>> Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
>>
>> This will also help to improve the MySQL. With MySQL server
>> running on numa 0 and client running on numa 1, both QPS and
>> latency is imporved on read-write case:
>>                         5.17-rc1        patched
>> QPS-16threads        143333.2633    145077.4033(+1.22%)
>> QPS-24threads        195085.9367    202719.6133(+3.91%)
>> QPS-32threads        241165.6867      249020.74(+3.26%)
>> QPS-64threads        244586.8433    253387.7567(+3.60%)
>> avg-lat-16threads           2.23           2.19(+1.19%)
>> avg-lat-24threads           2.46           2.36(+3.79%)
>> avg-lat-36threads           2.66           2.57(+3.26%)
>> avg-lat-64threads           5.23           5.05(+3.44%)
>>
>> 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>
>> ---
>>  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++
>> ----
>>  1 file changed, 42 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 5146163bfabb..2f84a933aedd 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -6262,12 +6262,46 @@ 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, int prev_cpu,
>> int target)
>> +{
>> +	struct cpumask *cpus =
>> this_cpu_cpumask_var_ptr(select_idle_mask);
>> +	struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster,
>> target));
>> +	int cpu, idle_cpu;
>> +
>> +	/* TODO: Support SMT case while a machine with both cluster and
>> SMT born */
> 
> This is probably a clearer comment
> 
> 	/* TODO: Support SMT system with cluster topology */
> 
>> +	if (!sched_smt_active() && sd) {
>> +		for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
>> +			idle_cpu = __select_idle_cpu(cpu, p);
>>   */
>> -static int select_idle_cpu(struct task_struct *p, struct
>> sched_domain *sd, bool has_idle_core, int target)
>> +static int select_idle_cpu(struct task_struct *p, struct
>> sched_domain *sd, bool has_idle_core, int prev_cpu, int target)
>>  {
>>  	struct cpumask *cpus =
>> this_cpu_cpumask_var_ptr(select_idle_mask);
>>  	int i, cpu, idle_cpu = -1, nr = INT_MAX;
>> @@ -6282,6 +6316,10 @@ static int select_idle_cpu(struct task_struct
>> *p, struct sched_domain *sd, bool
>>  
>>  	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
>>  
>> +	idle_cpu = scan_cluster(p, prev_cpu, target);
> 
> Shouldn't "cpus" from 
> 
> cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
> 
> be passed to scan_cluster, to make sure that the cpu returned is 
> in the affinity mask of the task? I don't see p->cpus_ptr
> being checked in scan_cluster to make sure the cpu found is in the
> affinity mask.
> 

The cpus scanned in scan_cluster() is the intersection of
select_idle_mask and sched_domain_span(cluster_sd), and
we limited the select_idle_mask in the tasks' affinity mask
before we enter scan_cluster() here.

Thanks.

> Tim
> 
> 
>> +	if ((unsigned int)idle_cpu < nr_cpumask_bits)
>> +		return idle_cpu;
>> +
>>  	if (sched_feat(SIS_PROP) && !has_idle_core) {
>>  		u64 avg_cost, avg_idle, span_avg;
>>  		unsigned long now = jiffies;
>> @@ -6416,7 +6454,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_resources(prev, target) &&
>>  	    (available_idle_cpu(prev) || sched_idle_cpu(prev)) &&
>>  	    asym_fits_capacity(task_util, prev))
>>  		return prev;
>> @@ -6442,7 +6480,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_resources(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)) {
>> @@ -6483,7 +6521,7 @@ static int select_idle_sibling(struct
>> task_struct *p, int prev, int target)
>>  		}
>>  	}
>>  
>> -	i = select_idle_cpu(p, sd, has_idle_core, target);
>> +	i = select_idle_cpu(p, sd, has_idle_core, prev, target);
>>  	if ((unsigned)i < nr_cpumask_bits)
>>  		return i;
>>  
> 
> .
> 

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-27  2:02     ` Yicong Yang
@ 2022-01-27  2:30       ` Tim Chen
  2022-01-27  2:36         ` Tim Chen
  0 siblings, 1 reply; 27+ messages in thread
From: Tim Chen @ 2022-01-27  2:30 UTC (permalink / raw)
  To: Yicong Yang, Yicong Yang, peterz, mingo, juri.lelli,
	vincent.guittot, gautham.shenoy, linux-kernel, linux-arm-kernel
  Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
	jonathan.cameron, ego, srikar, linuxarm, 21cnbao, song.bao.hua,
	guodong.xu

On Thu, 2022-01-27 at 10:02 +0800, Yicong Yang wrote:
> On 2022/1/27 9:14, Tim Chen wrote:
> > On Wed, 2022-01-26 at 16:09 +0800, Yicong Yang wrote:
> > > 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:
> > >                             5.17-rc1                patched
> > > Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> > > Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> > > Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> > > Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> > > Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> > > Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> > > Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> > > Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> > > 
> > > On numa 0-1:
> > >                             5.17-rc1                patched
> > > Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> > > Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> > > Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> > > Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> > > Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> > > Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> > > Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> > > Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> > > Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> > > Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> > > 
> > > This will also help to improve the MySQL. With MySQL server
> > > running on numa 0 and client running on numa 1, both QPS and
> > > latency is imporved on read-write case:
> > >                         5.17-rc1        patched
> > > QPS-16threads        143333.2633    145077.4033(+1.22%)
> > > QPS-24threads        195085.9367    202719.6133(+3.91%)
> > > QPS-32threads        241165.6867      249020.74(+3.26%)
> > > QPS-64threads        244586.8433    253387.7567(+3.60%)
> > > avg-lat-16threads           2.23           2.19(+1.19%)
> > > avg-lat-24threads           2.46           2.36(+3.79%)
> > > avg-lat-36threads           2.66           2.57(+3.26%)
> > > avg-lat-64threads           5.23           5.05(+3.44%)
> > > 
> > > 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>
> > > ---
> > >  kernel/sched/fair.c | 46
> > > +++++++++++++++++++++++++++++++++++++++++
> > > ----
> > >  1 file changed, 42 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > index 5146163bfabb..2f84a933aedd 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -6262,12 +6262,46 @@ 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, int
> > > prev_cpu,
> > > int target)
> > > +{
> > > +	struct cpumask *cpus =
> > > this_cpu_cpumask_var_ptr(select_idle_mask);
> > > +	struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster,
> > > target));
> > > +	int cpu, idle_cpu;
> > > +
> > > +	/* TODO: Support SMT case while a machine with both cluster and
> > > SMT born */
> > 
> > This is probably a clearer comment
> > 
> > 	/* TODO: Support SMT system with cluster topology */
> > 
> > > +	if (!sched_smt_active() && sd) {
> > > +		for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> > > +			idle_cpu = __select_idle_cpu(cpu, p);
> > >   */
> > > -static int select_idle_cpu(struct task_struct *p, struct
> > > sched_domain *sd, bool has_idle_core, int target)
> > > +static int select_idle_cpu(struct task_struct *p, struct
> > > sched_domain *sd, bool has_idle_core, int prev_cpu, int target)
> > >  {
> > >  	struct cpumask *cpus =
> > > this_cpu_cpumask_var_ptr(select_idle_mask);
> > >  	int i, cpu, idle_cpu = -1, nr = INT_MAX;
> > > @@ -6282,6 +6316,10 @@ static int select_idle_cpu(struct
> > > task_struct
> > > *p, struct sched_domain *sd, bool
> > >  
> > >  	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
> > >  
> > > +	idle_cpu = scan_cluster(p, prev_cpu, target);
> > 
> > Shouldn't "cpus" from 
> > 
> > cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
> > 
> > be passed to scan_cluster, to make sure that the cpu returned is 
> > in the affinity mask of the task? I don't see p->cpus_ptr
> > being checked in scan_cluster to make sure the cpu found is in the
> > affinity mask.
> > 
> 
> The cpus scanned in scan_cluster() is the intersection of
> select_idle_mask and sched_domain_span(cluster_sd), and
> we limited the select_idle_mask in the tasks' affinity mask
> before we enter scan_cluster() here.

Ah, I missed the fact that cpus point to the select_idle_mask.

Thanks.

Tim



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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-27  2:30       ` Tim Chen
@ 2022-01-27  2:36         ` Tim Chen
  2022-01-27  3:05           ` Yicong Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Tim Chen @ 2022-01-27  2:36 UTC (permalink / raw)
  To: Yicong Yang, Yicong Yang, peterz, mingo, juri.lelli,
	vincent.guittot, gautham.shenoy, linux-kernel, linux-arm-kernel
  Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
	jonathan.cameron, ego, srikar, linuxarm, 21cnbao, song.bao.hua,
	guodong.xu

On Wed, 2022-01-26 at 18:30 -0800, Tim Chen wrote:
> On Thu, 2022-01-27 at 10:02 +0800, Yicong Yang wrote:
> > On 2022/1/27 9:14, Tim Chen wrote:
> > > On Wed, 2022-01-26 at 16:09 +0800, Yicong Yang wrote:
> > > > 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:
> > > >                             5.17-rc1                patched
> > > > Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> > > > Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> > > > Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> > > > Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> > > > Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> > > > Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> > > > Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> > > > Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> > > > 
> > > > On numa 0-1:
> > > >                             5.17-rc1                patched
> > > > Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> > > > Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> > > > Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> > > > Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> > > > Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> > > > Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> > > > Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> > > > Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> > > > Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> > > > Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> > > > 
> > > > This will also help to improve the MySQL. With MySQL server
> > > > running on numa 0 and client running on numa 1, both QPS and
> > > > latency is imporved on read-write case:
> > > >                         5.17-rc1        patched
> > > > QPS-16threads        143333.2633    145077.4033(+1.22%)
> > > > QPS-24threads        195085.9367    202719.6133(+3.91%)
> > > > QPS-32threads        241165.6867      249020.74(+3.26%)
> > > > QPS-64threads        244586.8433    253387.7567(+3.60%)
> > > > avg-lat-16threads           2.23           2.19(+1.19%)
> > > > avg-lat-24threads           2.46           2.36(+3.79%)
> > > > avg-lat-36threads           2.66           2.57(+3.26%)
> > > > avg-lat-64threads           5.23           5.05(+3.44%)
> > > > 
> > > > 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>
> > > > ---
> > > >  kernel/sched/fair.c | 46
> > > > +++++++++++++++++++++++++++++++++++++++++
> > > > ----
> > > >  1 file changed, 42 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > index 5146163bfabb..2f84a933aedd 100644
> > > > --- a/kernel/sched/fair.c
> > > > +++ b/kernel/sched/fair.c
> > > > @@ -6262,12 +6262,46 @@ 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, int
> > > > prev_cpu,
> > > > int target)
> > > > +{
> > > > +	struct cpumask *cpus =
> > > > this_cpu_cpumask_var_ptr(select_idle_mask);
> > > > +	struct sched_domain *sd =
> > > > rcu_dereference(per_cpu(sd_cluster,
> > > > target));
> > > > +	int cpu, idle_cpu;
> > > > +
> > > > +	/* TODO: Support SMT case while a machine with both
> > > > cluster and
> > > > SMT born */
> > > 
> > > This is probably a clearer comment
> > > 
> > > 	/* TODO: Support SMT system with cluster topology */
> > > 
> > > > +	if (!sched_smt_active() && sd) {
> > > > +		for_each_cpu_and(cpu, cpus,
> > > > sched_domain_span(sd)) {
> > > > +			idle_cpu = __select_idle_cpu(cpu, p);
> > > >   */
> > > > -static int select_idle_cpu(struct task_struct *p, struct
> > > > sched_domain *sd, bool has_idle_core, int target)
> > > > +static int select_idle_cpu(struct task_struct *p, struct
> > > > sched_domain *sd, bool has_idle_core, int prev_cpu, int target)
> > > >  {
> > > >  	struct cpumask *cpus =
> > > > this_cpu_cpumask_var_ptr(select_idle_mask);
> > > >  	int i, cpu, idle_cpu = -1, nr = INT_MAX;
> > > > @@ -6282,6 +6316,10 @@ static int select_idle_cpu(struct
> > > > task_struct
> > > > *p, struct sched_domain *sd, bool
> > > >  
> > > >  	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
> > > >  
> > > > +	idle_cpu = scan_cluster(p, prev_cpu, target);
> > > 
> > > Shouldn't "cpus" from 
> > > 
> > > cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
> > > 
> > > be passed to scan_cluster, to make sure that the cpu returned is 
> > > in the affinity mask of the task? I don't see p->cpus_ptr
> > > being checked in scan_cluster to make sure the cpu found is in
> > > the
> > > affinity mask.
> > > 
> > 
> > The cpus scanned in scan_cluster() is the intersection of
> > select_idle_mask and sched_domain_span(cluster_sd), and
> > we limited the select_idle_mask in the tasks' affinity mask
> > before we enter scan_cluster() here.
> 
> Ah, I missed the fact that cpus point to the select_idle_mask.
> 

I think it will be easier to read the code if you pass "cpus" directly
to scan cluster, rather than making this implicit, and having this
assignment 

*cpus = this_cpu_cpumask_var_ptr(select_idle_mask);

again in scan_cluster.

Tim



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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-27  2:36         ` Tim Chen
@ 2022-01-27  3:05           ` Yicong Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Yicong Yang @ 2022-01-27  3:05 UTC (permalink / raw)
  To: Tim Chen, Yicong Yang, peterz, mingo, juri.lelli,
	vincent.guittot, gautham.shenoy, linux-kernel, linux-arm-kernel
  Cc: dietmar.eggemann, rostedt, bsegall, bristot, prime.zeng,
	jonathan.cameron, ego, srikar, linuxarm, 21cnbao, song.bao.hua,
	guodong.xu

On 2022/1/27 10:36, Tim Chen wrote:
> On Wed, 2022-01-26 at 18:30 -0800, Tim Chen wrote:
>> On Thu, 2022-01-27 at 10:02 +0800, Yicong Yang wrote:
>>> On 2022/1/27 9:14, Tim Chen wrote:
>>>> On Wed, 2022-01-26 at 16:09 +0800, Yicong Yang wrote:
>>>>> 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:
>>>>>                             5.17-rc1                patched
>>>>> Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
>>>>> Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
>>>>> Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
>>>>> Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
>>>>> Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
>>>>> Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
>>>>> Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
>>>>> Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
>>>>>
>>>>> On numa 0-1:
>>>>>                             5.17-rc1                patched
>>>>> Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
>>>>> Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
>>>>> Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
>>>>> Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
>>>>> Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
>>>>> Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
>>>>> Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
>>>>> Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
>>>>> Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
>>>>> Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
>>>>>
>>>>> This will also help to improve the MySQL. With MySQL server
>>>>> running on numa 0 and client running on numa 1, both QPS and
>>>>> latency is imporved on read-write case:
>>>>>                         5.17-rc1        patched
>>>>> QPS-16threads        143333.2633    145077.4033(+1.22%)
>>>>> QPS-24threads        195085.9367    202719.6133(+3.91%)
>>>>> QPS-32threads        241165.6867      249020.74(+3.26%)
>>>>> QPS-64threads        244586.8433    253387.7567(+3.60%)
>>>>> avg-lat-16threads           2.23           2.19(+1.19%)
>>>>> avg-lat-24threads           2.46           2.36(+3.79%)
>>>>> avg-lat-36threads           2.66           2.57(+3.26%)
>>>>> avg-lat-64threads           5.23           5.05(+3.44%)
>>>>>
>>>>> 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>
>>>>> ---
>>>>>  kernel/sched/fair.c | 46
>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>> ----
>>>>>  1 file changed, 42 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>>>> index 5146163bfabb..2f84a933aedd 100644
>>>>> --- a/kernel/sched/fair.c
>>>>> +++ b/kernel/sched/fair.c
>>>>> @@ -6262,12 +6262,46 @@ 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, int
>>>>> prev_cpu,
>>>>> int target)
>>>>> +{
>>>>> +	struct cpumask *cpus =
>>>>> this_cpu_cpumask_var_ptr(select_idle_mask);
>>>>> +	struct sched_domain *sd =
>>>>> rcu_dereference(per_cpu(sd_cluster,
>>>>> target));
>>>>> +	int cpu, idle_cpu;
>>>>> +
>>>>> +	/* TODO: Support SMT case while a machine with both
>>>>> cluster and
>>>>> SMT born */
>>>>
>>>> This is probably a clearer comment
>>>>
>>>> 	/* TODO: Support SMT system with cluster topology */
>>>>
>>>>> +	if (!sched_smt_active() && sd) {
>>>>> +		for_each_cpu_and(cpu, cpus,
>>>>> sched_domain_span(sd)) {
>>>>> +			idle_cpu = __select_idle_cpu(cpu, p);
>>>>>   */
>>>>> -static int select_idle_cpu(struct task_struct *p, struct
>>>>> sched_domain *sd, bool has_idle_core, int target)
>>>>> +static int select_idle_cpu(struct task_struct *p, struct
>>>>> sched_domain *sd, bool has_idle_core, int prev_cpu, int target)
>>>>>  {
>>>>>  	struct cpumask *cpus =
>>>>> this_cpu_cpumask_var_ptr(select_idle_mask);
>>>>>  	int i, cpu, idle_cpu = -1, nr = INT_MAX;
>>>>> @@ -6282,6 +6316,10 @@ static int select_idle_cpu(struct
>>>>> task_struct
>>>>> *p, struct sched_domain *sd, bool
>>>>>  
>>>>>  	cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
>>>>>  
>>>>> +	idle_cpu = scan_cluster(p, prev_cpu, target);
>>>>
>>>> Shouldn't "cpus" from 
>>>>
>>>> cpumask_and(cpus, sched_domain_span(sd), p->cpus_ptr);
>>>>
>>>> be passed to scan_cluster, to make sure that the cpu returned is 
>>>> in the affinity mask of the task? I don't see p->cpus_ptr
>>>> being checked in scan_cluster to make sure the cpu found is in
>>>> the
>>>> affinity mask.
>>>>
>>>
>>> The cpus scanned in scan_cluster() is the intersection of
>>> select_idle_mask and sched_domain_span(cluster_sd), and
>>> we limited the select_idle_mask in the tasks' affinity mask
>>> before we enter scan_cluster() here.
>>
>> Ah, I missed the fact that cpus point to the select_idle_mask.
>>
> 
> I think it will be easier to read the code if you pass "cpus" directly
> to scan cluster, rather than making this implicit, and having this
> assignment 
> 
> *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> 
> again in scan_cluster.

sure. It does look more readable and I think we can change to that. :)

Thanks.

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

* Re: [PATCH v2 1/2] sched: Add per_cpu cluster domain info and cpus_share_resources API
  2022-01-26  8:09 ` [PATCH v2 1/2] sched: Add per_cpu cluster domain info and cpus_share_resources API Yicong Yang
@ 2022-01-27 15:26   ` Gautham R. Shenoy
  0 siblings, 0 replies; 27+ messages in thread
From: Gautham R. Shenoy @ 2022-01-27 15:26 UTC (permalink / raw)
  To: Yicong Yang
  Cc: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen,
	linux-kernel, linux-arm-kernel, dietmar.eggemann, rostedt,
	bsegall, bristot, prime.zeng, jonathan.cameron, ego, srikar,
	linuxarm, 21cnbao, song.bao.hua, guodong.xu

Hello Yicong, Barry,
On Wed, Jan 26, 2022 at 04:09:46PM +0800, Yicong Yang wrote:
> From: Barry Song <song.bao.hua@hisilicon.com>
> 
> Add per-cpu cluster domain info and cpus_share_resources() API.
> This is the preparation for the optimization of select_idle_cpu()
> on platforms with cluster scheduler level.
> 
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>

This patch looks good to me.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

--
Thanks and Regards
gautham.

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-26  8:09 ` [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
  2022-01-27  1:14   ` Tim Chen
@ 2022-01-27 15:41   ` Gautham R. Shenoy
  2022-01-27 20:21     ` Barry Song
  1 sibling, 1 reply; 27+ messages in thread
From: Gautham R. Shenoy @ 2022-01-27 15:41 UTC (permalink / raw)
  To: Yicong Yang
  Cc: peterz, mingo, juri.lelli, vincent.guittot, tim.c.chen,
	linux-kernel, linux-arm-kernel, dietmar.eggemann, rostedt,
	bsegall, bristot, prime.zeng, jonathan.cameron, ego, srikar,
	linuxarm, 21cnbao, song.bao.hua, guodong.xu

On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> 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:
>                             5.17-rc1                patched
> Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> 
> On numa 0-1:
>                             5.17-rc1                patched
> Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> 
> This will also help to improve the MySQL. With MySQL server
> running on numa 0 and client running on numa 1, both QPS and
> latency is imporved on read-write case:
>                         5.17-rc1        patched
> QPS-16threads        143333.2633    145077.4033(+1.22%)
> QPS-24threads        195085.9367    202719.6133(+3.91%)
> QPS-32threads        241165.6867      249020.74(+3.26%)
> QPS-64threads        244586.8433    253387.7567(+3.60%)
> avg-lat-16threads           2.23           2.19(+1.19%)
> avg-lat-24threads           2.46           2.36(+3.79%)
> avg-lat-36threads           2.66           2.57(+3.26%)
> avg-lat-64threads           5.23           5.05(+3.44%)
> 
> 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>
> ---
>  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 5146163bfabb..2f84a933aedd 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6262,12 +6262,46 @@ 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, int prev_cpu, int target)
> +{
> +	struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> +	struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
> +	int cpu, idle_cpu;
> +
> +	/* TODO: Support SMT case while a machine with both cluster and SMT born */
> +	if (!sched_smt_active() && sd) {
> +		for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> +			idle_cpu = __select_idle_cpu(cpu, p);
> +			if ((unsigned int)idle_cpu < nr_cpumask_bits)
> +				return idle_cpu;
> +		}
> +
> +		/* Don't ping-pong tasks in and out cluster frequently */
> +		if (cpus_share_resources(target, prev_cpu))
> +			return target;

We reach here when there aren't any idle CPUs within the
cluster. However there might be idle CPUs in the MC domain. Is a busy
@target preferable to a potentially idle CPU within the larger domain
?


--
Thanks and Regards
gautham.

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-28  7:13       ` Srikar Dronamraju
@ 2022-01-27 18:40         ` Barry Song
  2022-02-01  9:38           ` Srikar Dronamraju
  0 siblings, 1 reply; 27+ messages in thread
From: Barry Song @ 2022-01-27 18:40 UTC (permalink / raw)
  To: Srikar Dronamraju
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
<srikar@linux.vnet.ibm.com> wrote:
>
> * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
>
> > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > <gautham.shenoy@amd.com> wrote:
> > >
> > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > > > 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:
> > > >                             5.17-rc1                patched
> > > > Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> > > > Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> > > > Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> > > > Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> > > > Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> > > > Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> > > > Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> > > > Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> > > >
> > > > On numa 0-1:
> > > >                             5.17-rc1                patched
> > > > Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> > > > Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> > > > Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> > > > Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> > > > Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> > > > Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> > > > Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> > > > Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> > > > Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> > > > Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> > > >
> > > > This will also help to improve the MySQL. With MySQL server
> > > > running on numa 0 and client running on numa 1, both QPS and
> > > > latency is imporved on read-write case:
> > > >                         5.17-rc1        patched
> > > > QPS-16threads        143333.2633    145077.4033(+1.22%)
> > > > QPS-24threads        195085.9367    202719.6133(+3.91%)
> > > > QPS-32threads        241165.6867      249020.74(+3.26%)
> > > > QPS-64threads        244586.8433    253387.7567(+3.60%)
> > > > avg-lat-16threads           2.23           2.19(+1.19%)
> > > > avg-lat-24threads           2.46           2.36(+3.79%)
> > > > avg-lat-36threads           2.66           2.57(+3.26%)
> > > > avg-lat-64threads           5.23           5.05(+3.44%)
> > > >
> > > > 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>
> > > > ---
> > > >  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++----
> > > >  1 file changed, 42 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > index 5146163bfabb..2f84a933aedd 100644
> > > > --- a/kernel/sched/fair.c
> > > > +++ b/kernel/sched/fair.c
> > > > @@ -6262,12 +6262,46 @@ 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, int prev_cpu, int target)
> > > > +{
> > > > +     struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> > > > +     struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
> > > > +     int cpu, idle_cpu;
> > > > +
> > > > +     /* TODO: Support SMT case while a machine with both cluster and SMT born */
> > > > +     if (!sched_smt_active() && sd) {
> > > > +             for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> > > > +                     idle_cpu = __select_idle_cpu(cpu, p);
> > > > +                     if ((unsigned int)idle_cpu < nr_cpumask_bits)
> > > > +                             return idle_cpu;
> > > > +             }
> > > > +
> > > > +             /* Don't ping-pong tasks in and out cluster frequently */
> > > > +             if (cpus_share_resources(target, prev_cpu))
> > > > +                     return target;
> > >
> > > We reach here when there aren't any idle CPUs within the
> > > cluster. However there might be idle CPUs in the MC domain. Is a busy
> > > @target preferable to a potentially idle CPU within the larger domain
> > > ?
> >
> > Hi Gautham,
> >
>
> Hi Barry,
>
>
> > My benchmark showed some performance regression while load was medium or above
> > if we grabbed idle cpu in and out the cluster. it turned out the
> > regression disappeared if
> > we blocked the ping-pong. so the logic here is that if we have scanned
> > and found an
> > idle cpu within the cluster before, we don't let the task jumping back
> > and forth frequently
> > as cache synchronization is higher cost. but the code still allows
> > scanning out of the cluster
> > if we haven't packed waker and wakee together yet.
> >
>
> Like what Gautham said, should we choose the same cluster if we find that
> there are no idle-cpus in the LLC? This way we avoid ping-pong if there are
> no idle-cpus but we still pick an idle-cpu to a busy cpu?

Hi Srikar,
I am sorry I didn't get your question. Currently the code works as below:
if task A wakes up task B, and task A is in LLC0 and task B is in LLC1.
we will scan the cluster of A before scanning the whole LLC0, in this case,
cluster of A is the closest sibling, so it is the better choice than other CPUs
which are in LLC0 but not in the cluster of A. But we do scan all cpus of LLC0
afterwards if we fail to find an idle CPU in the cluster.

After a while, if the cluster of A gets an idle CPU and pulls B into the
cluster, we prefer not pushing B out of the cluster of A again though
there might be an idle CPU outside. as benchmark shows getting an
idle CPU out of the cluster of A doesn't bring performance improvement
but performance decreases as B might be getting in and getting out
the cluster of A very frequently, then cache coherence ping-pong.

Note we are only returning target while if
(cpus_share_resources(target, prev_cpu))
is true. So we are not losing chance to pull B to the LLC of A while LLC0 has
an idle one.

>
> > it might not be a universal win in all kinds of workload. we saw
> > tbench, mysql benefit from
> > the whole change. but pgbench seems not always. so we are still on the
> > way to make possible
> > further tuning here.
> >
>
> > >
> > >
> > > --
> > > Thanks and Regards
> > > gautham.
> >
> > Thanks
> > Barry
>
> --
> Thanks and Regards
> Srikar Dronamraju

Thanks
Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-27 15:41   ` Gautham R. Shenoy
@ 2022-01-27 20:21     ` Barry Song
  2022-01-28  7:13       ` Srikar Dronamraju
  0 siblings, 1 reply; 27+ messages in thread
From: Barry Song @ 2022-01-27 20:21 UTC (permalink / raw)
  To: Gautham R. Shenoy
  Cc: Yicong Yang, Peter Zijlstra, Ingo Molnar, Juri Lelli,
	Vincent Guittot, Tim Chen, LKML, LAK, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Daniel Bristot de Oliveira,
	prime.zeng, Jonathan Cameron, ego, Srikar Dronamraju, Linuxarm,
	Barry Song, Guodong Xu

On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
<gautham.shenoy@amd.com> wrote:
>
> On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > 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:
> >                             5.17-rc1                patched
> > Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> > Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> > Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> > Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> > Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> > Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> > Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> > Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> >
> > On numa 0-1:
> >                             5.17-rc1                patched
> > Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> > Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> > Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> > Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> > Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> > Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> > Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> > Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> > Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> > Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> >
> > This will also help to improve the MySQL. With MySQL server
> > running on numa 0 and client running on numa 1, both QPS and
> > latency is imporved on read-write case:
> >                         5.17-rc1        patched
> > QPS-16threads        143333.2633    145077.4033(+1.22%)
> > QPS-24threads        195085.9367    202719.6133(+3.91%)
> > QPS-32threads        241165.6867      249020.74(+3.26%)
> > QPS-64threads        244586.8433    253387.7567(+3.60%)
> > avg-lat-16threads           2.23           2.19(+1.19%)
> > avg-lat-24threads           2.46           2.36(+3.79%)
> > avg-lat-36threads           2.66           2.57(+3.26%)
> > avg-lat-64threads           5.23           5.05(+3.44%)
> >
> > 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>
> > ---
> >  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 42 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 5146163bfabb..2f84a933aedd 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -6262,12 +6262,46 @@ 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, int prev_cpu, int target)
> > +{
> > +     struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> > +     struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
> > +     int cpu, idle_cpu;
> > +
> > +     /* TODO: Support SMT case while a machine with both cluster and SMT born */
> > +     if (!sched_smt_active() && sd) {
> > +             for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> > +                     idle_cpu = __select_idle_cpu(cpu, p);
> > +                     if ((unsigned int)idle_cpu < nr_cpumask_bits)
> > +                             return idle_cpu;
> > +             }
> > +
> > +             /* Don't ping-pong tasks in and out cluster frequently */
> > +             if (cpus_share_resources(target, prev_cpu))
> > +                     return target;
>
> We reach here when there aren't any idle CPUs within the
> cluster. However there might be idle CPUs in the MC domain. Is a busy
> @target preferable to a potentially idle CPU within the larger domain
> ?

Hi Gautham,

My benchmark showed some performance regression while load was medium or above
if we grabbed idle cpu in and out the cluster. it turned out the
regression disappeared if
we blocked the ping-pong. so the logic here is that if we have scanned
and found an
idle cpu within the cluster before, we don't let the task jumping back
and forth frequently
as cache synchronization is higher cost. but the code still allows
scanning out of the cluster
if we haven't packed waker and wakee together yet.

it might not be a universal win in all kinds of workload. we saw
tbench, mysql benefit from
the whole change. but pgbench seems not always. so we are still on the
way to make possible
further tuning here.

>
>
> --
> Thanks and Regards
> gautham.

Thanks
Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-27 20:21     ` Barry Song
@ 2022-01-28  7:13       ` Srikar Dronamraju
  2022-01-27 18:40         ` Barry Song
  0 siblings, 1 reply; 27+ messages in thread
From: Srikar Dronamraju @ 2022-01-28  7:13 UTC (permalink / raw)
  To: Barry Song
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

* Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:

> On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> <gautham.shenoy@amd.com> wrote:
> >
> > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > > 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:
> > >                             5.17-rc1                patched
> > > Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> > > Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> > > Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> > > Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> > > Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> > > Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> > > Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> > > Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> > >
> > > On numa 0-1:
> > >                             5.17-rc1                patched
> > > Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> > > Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> > > Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> > > Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> > > Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> > > Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> > > Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> > > Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> > > Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> > > Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> > >
> > > This will also help to improve the MySQL. With MySQL server
> > > running on numa 0 and client running on numa 1, both QPS and
> > > latency is imporved on read-write case:
> > >                         5.17-rc1        patched
> > > QPS-16threads        143333.2633    145077.4033(+1.22%)
> > > QPS-24threads        195085.9367    202719.6133(+3.91%)
> > > QPS-32threads        241165.6867      249020.74(+3.26%)
> > > QPS-64threads        244586.8433    253387.7567(+3.60%)
> > > avg-lat-16threads           2.23           2.19(+1.19%)
> > > avg-lat-24threads           2.46           2.36(+3.79%)
> > > avg-lat-36threads           2.66           2.57(+3.26%)
> > > avg-lat-64threads           5.23           5.05(+3.44%)
> > >
> > > 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>
> > > ---
> > >  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++----
> > >  1 file changed, 42 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > index 5146163bfabb..2f84a933aedd 100644
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -6262,12 +6262,46 @@ 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, int prev_cpu, int target)
> > > +{
> > > +     struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> > > +     struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
> > > +     int cpu, idle_cpu;
> > > +
> > > +     /* TODO: Support SMT case while a machine with both cluster and SMT born */
> > > +     if (!sched_smt_active() && sd) {
> > > +             for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> > > +                     idle_cpu = __select_idle_cpu(cpu, p);
> > > +                     if ((unsigned int)idle_cpu < nr_cpumask_bits)
> > > +                             return idle_cpu;
> > > +             }
> > > +
> > > +             /* Don't ping-pong tasks in and out cluster frequently */
> > > +             if (cpus_share_resources(target, prev_cpu))
> > > +                     return target;
> >
> > We reach here when there aren't any idle CPUs within the
> > cluster. However there might be idle CPUs in the MC domain. Is a busy
> > @target preferable to a potentially idle CPU within the larger domain
> > ?
> 
> Hi Gautham,
> 

Hi Barry,


> My benchmark showed some performance regression while load was medium or above
> if we grabbed idle cpu in and out the cluster. it turned out the
> regression disappeared if
> we blocked the ping-pong. so the logic here is that if we have scanned
> and found an
> idle cpu within the cluster before, we don't let the task jumping back
> and forth frequently
> as cache synchronization is higher cost. but the code still allows
> scanning out of the cluster
> if we haven't packed waker and wakee together yet.
> 

Like what Gautham said, should we choose the same cluster if we find that
there are no idle-cpus in the LLC? This way we avoid ping-pong if there are
no idle-cpus but we still pick an idle-cpu to a busy cpu?

> it might not be a universal win in all kinds of workload. we saw
> tbench, mysql benefit from
> the whole change. but pgbench seems not always. so we are still on the
> way to make possible
> further tuning here.
> 

> >
> >
> > --
> > Thanks and Regards
> > gautham.
> 
> Thanks
> Barry

-- 
Thanks and Regards
Srikar Dronamraju

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-01-27 18:40         ` Barry Song
@ 2022-02-01  9:38           ` Srikar Dronamraju
  2022-02-01 20:20             ` Barry Song
  0 siblings, 1 reply; 27+ messages in thread
From: Srikar Dronamraju @ 2022-02-01  9:38 UTC (permalink / raw)
  To: Barry Song
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

* Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:

> On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> <srikar@linux.vnet.ibm.com> wrote:
> >
> > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> >
> > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > <gautham.shenoy@amd.com> wrote:
> > > >
> > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > > > > 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:
> > > > >                             5.17-rc1                patched
> > > > > Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> > > > > Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> > > > > Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> > > > > Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> > > > > Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> > > > > Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> > > > > Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> > > > > Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> > > > >
> > > > > On numa 0-1:
> > > > >                             5.17-rc1                patched
> > > > > Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> > > > > Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> > > > > Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> > > > > Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> > > > > Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> > > > > Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> > > > > Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> > > > > Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> > > > > Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> > > > > Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> > > > >
> > > > > This will also help to improve the MySQL. With MySQL server
> > > > > running on numa 0 and client running on numa 1, both QPS and
> > > > > latency is imporved on read-write case:
> > > > >                         5.17-rc1        patched
> > > > > QPS-16threads        143333.2633    145077.4033(+1.22%)
> > > > > QPS-24threads        195085.9367    202719.6133(+3.91%)
> > > > > QPS-32threads        241165.6867      249020.74(+3.26%)
> > > > > QPS-64threads        244586.8433    253387.7567(+3.60%)
> > > > > avg-lat-16threads           2.23           2.19(+1.19%)
> > > > > avg-lat-24threads           2.46           2.36(+3.79%)
> > > > > avg-lat-36threads           2.66           2.57(+3.26%)
> > > > > avg-lat-64threads           5.23           5.05(+3.44%)
> > > > >
> > > > > 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>
> > > > > ---
> > > > >  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++----
> > > > >  1 file changed, 42 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > > index 5146163bfabb..2f84a933aedd 100644
> > > > > --- a/kernel/sched/fair.c
> > > > > +++ b/kernel/sched/fair.c
> > > > > @@ -6262,12 +6262,46 @@ 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, int prev_cpu, int target)
> > > > > +{
> > > > > +     struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> > > > > +     struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
> > > > > +     int cpu, idle_cpu;
> > > > > +
> > > > > +     /* TODO: Support SMT case while a machine with both cluster and SMT born */
> > > > > +     if (!sched_smt_active() && sd) {
> > > > > +             for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> > > > > +                     idle_cpu = __select_idle_cpu(cpu, p);
> > > > > +                     if ((unsigned int)idle_cpu < nr_cpumask_bits)
> > > > > +                             return idle_cpu;
> > > > > +             }
> > > > > +
> > > > > +             /* Don't ping-pong tasks in and out cluster frequently */
> > > > > +             if (cpus_share_resources(target, prev_cpu))
> > > > > +                     return target;
> > > >
> > > > We reach here when there aren't any idle CPUs within the
> > > > cluster. However there might be idle CPUs in the MC domain. Is a busy
> > > > @target preferable to a potentially idle CPU within the larger domain
> > > > ?
> > >
> > > Hi Gautham,
> > >
> >
> > Hi Barry,
> >
> >
> > > My benchmark showed some performance regression while load was medium or above
> > > if we grabbed idle cpu in and out the cluster. it turned out the
> > > regression disappeared if
> > > we blocked the ping-pong. so the logic here is that if we have scanned
> > > and found an
> > > idle cpu within the cluster before, we don't let the task jumping back
> > > and forth frequently
> > > as cache synchronization is higher cost. but the code still allows
> > > scanning out of the cluster
> > > if we haven't packed waker and wakee together yet.
> > >
> >
> > Like what Gautham said, should we choose the same cluster if we find that
> > there are no idle-cpus in the LLC? This way we avoid ping-pong if there are
> > no idle-cpus but we still pick an idle-cpu to a busy cpu?
> 
> Hi Srikar,
> I am sorry I didn't get your question. Currently the code works as below:
> if task A wakes up task B, and task A is in LLC0 and task B is in LLC1.
> we will scan the cluster of A before scanning the whole LLC0, in this case,
> cluster of A is the closest sibling, so it is the better choice than other CPUs
> which are in LLC0 but not in the cluster of A. 

Yes, this is right.

> But we do scan all cpus of LLC0
> afterwards if we fail to find an idle CPU in the cluster.

However my reading of the patch, before we can scan other clusters within
the LLC (aka LLC0), we have a check in scan cluster which says 

	/* Don't ping-pong tasks in and out cluster frequently */
	if (cpus_share_resources(target, prev_cpu))
	   return target;

My reading of this is, ignore other clusters (at this point, we know there
are no idle CPUs in this cluster. We don't know if there are idle cpus in
them or not) if the previous CPU and target CPU happen to be from the same
cluster. This effectively means we are given preference to cache over idle
CPU.

Or Am I still missing something?

> 
> After a while, if the cluster of A gets an idle CPU and pulls B into the
> cluster, we prefer not pushing B out of the cluster of A again though
> there might be an idle CPU outside. as benchmark shows getting an
> idle CPU out of the cluster of A doesn't bring performance improvement
> but performance decreases as B might be getting in and getting out
> the cluster of A very frequently, then cache coherence ping-pong.
> 

The counter argument can be that Task A and Task B are related and were
running on the same cluster. But Load balancer moved Task B to a different
cluster. Now this check may cause them to continue to run on two different
clusters, even though the underlying load balance issues may have changed.

No?


-- 
Thanks and Regards
Srikar Dronamraju

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-01  9:38           ` Srikar Dronamraju
@ 2022-02-01 20:20             ` Barry Song
  2022-02-04  7:33               ` Srikar Dronamraju
  0 siblings, 1 reply; 27+ messages in thread
From: Barry Song @ 2022-02-01 20:20 UTC (permalink / raw)
  To: Srikar Dronamraju
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Tue, Feb 1, 2022 at 10:39 PM Srikar Dronamraju
<srikar@linux.vnet.ibm.com> wrote:
>
> * Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:
>
> > On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> > <srikar@linux.vnet.ibm.com> wrote:
> > >
> > > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> > >
> > > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > > <gautham.shenoy@amd.com> wrote:
> > > > >
> > > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > > > > > 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:
> > > > > >                             5.17-rc1                patched
> > > > > > Hmean     1        324.73 (   0.00%)      378.01 *  16.41%*
> > > > > > Hmean     2        645.36 (   0.00%)      754.63 *  16.93%*
> > > > > > Hmean     4       1302.09 (   0.00%)     1507.54 *  15.78%*
> > > > > > Hmean     8       2612.03 (   0.00%)     2982.57 *  14.19%*
> > > > > > Hmean     16      5307.12 (   0.00%)     5886.66 *  10.92%*
> > > > > > Hmean     32      9354.22 (   0.00%)     9908.13 *   5.92%*
> > > > > > Hmean     64      7240.35 (   0.00%)     7278.78 *   0.53%*
> > > > > > Hmean     128     6186.40 (   0.00%)     6187.85 (   0.02%)
> > > > > >
> > > > > > On numa 0-1:
> > > > > >                             5.17-rc1                patched
> > > > > > Hmean     1        320.01 (   0.00%)      378.44 *  18.26%*
> > > > > > Hmean     2        643.85 (   0.00%)      752.52 *  16.88%*
> > > > > > Hmean     4       1287.36 (   0.00%)     1505.62 *  16.95%*
> > > > > > Hmean     8       2564.60 (   0.00%)     2955.29 *  15.23%*
> > > > > > Hmean     16      5195.69 (   0.00%)     5814.74 *  11.91%*
> > > > > > Hmean     32      9769.16 (   0.00%)    10872.63 *  11.30%*
> > > > > > Hmean     64     15952.50 (   0.00%)    17281.98 *   8.33%*
> > > > > > Hmean     128    13113.77 (   0.00%)    13895.20 *   5.96%*
> > > > > > Hmean     256    10997.59 (   0.00%)    11244.69 *   2.25%*
> > > > > > Hmean     512    14623.60 (   0.00%)    15526.25 *   6.17%*
> > > > > >
> > > > > > This will also help to improve the MySQL. With MySQL server
> > > > > > running on numa 0 and client running on numa 1, both QPS and
> > > > > > latency is imporved on read-write case:
> > > > > >                         5.17-rc1        patched
> > > > > > QPS-16threads        143333.2633    145077.4033(+1.22%)
> > > > > > QPS-24threads        195085.9367    202719.6133(+3.91%)
> > > > > > QPS-32threads        241165.6867      249020.74(+3.26%)
> > > > > > QPS-64threads        244586.8433    253387.7567(+3.60%)
> > > > > > avg-lat-16threads           2.23           2.19(+1.19%)
> > > > > > avg-lat-24threads           2.46           2.36(+3.79%)
> > > > > > avg-lat-36threads           2.66           2.57(+3.26%)
> > > > > > avg-lat-64threads           5.23           5.05(+3.44%)
> > > > > >
> > > > > > 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>
> > > > > > ---
> > > > > >  kernel/sched/fair.c | 46 +++++++++++++++++++++++++++++++++++++++++----
> > > > > >  1 file changed, 42 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > > > > > index 5146163bfabb..2f84a933aedd 100644
> > > > > > --- a/kernel/sched/fair.c
> > > > > > +++ b/kernel/sched/fair.c
> > > > > > @@ -6262,12 +6262,46 @@ 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, int prev_cpu, int target)
> > > > > > +{
> > > > > > +     struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
> > > > > > +     struct sched_domain *sd = rcu_dereference(per_cpu(sd_cluster, target));
> > > > > > +     int cpu, idle_cpu;
> > > > > > +
> > > > > > +     /* TODO: Support SMT case while a machine with both cluster and SMT born */
> > > > > > +     if (!sched_smt_active() && sd) {
> > > > > > +             for_each_cpu_and(cpu, cpus, sched_domain_span(sd)) {
> > > > > > +                     idle_cpu = __select_idle_cpu(cpu, p);
> > > > > > +                     if ((unsigned int)idle_cpu < nr_cpumask_bits)
> > > > > > +                             return idle_cpu;
> > > > > > +             }
> > > > > > +
> > > > > > +             /* Don't ping-pong tasks in and out cluster frequently */
> > > > > > +             if (cpus_share_resources(target, prev_cpu))
> > > > > > +                     return target;
> > > > >
> > > > > We reach here when there aren't any idle CPUs within the
> > > > > cluster. However there might be idle CPUs in the MC domain. Is a busy
> > > > > @target preferable to a potentially idle CPU within the larger domain
> > > > > ?
> > > >
> > > > Hi Gautham,
> > > >
> > >
> > > Hi Barry,
> > >
> > >
> > > > My benchmark showed some performance regression while load was medium or above
> > > > if we grabbed idle cpu in and out the cluster. it turned out the
> > > > regression disappeared if
> > > > we blocked the ping-pong. so the logic here is that if we have scanned
> > > > and found an
> > > > idle cpu within the cluster before, we don't let the task jumping back
> > > > and forth frequently
> > > > as cache synchronization is higher cost. but the code still allows
> > > > scanning out of the cluster
> > > > if we haven't packed waker and wakee together yet.
> > > >
> > >
> > > Like what Gautham said, should we choose the same cluster if we find that
> > > there are no idle-cpus in the LLC? This way we avoid ping-pong if there are
> > > no idle-cpus but we still pick an idle-cpu to a busy cpu?
> >
> > Hi Srikar,
> > I am sorry I didn't get your question. Currently the code works as below:
> > if task A wakes up task B, and task A is in LLC0 and task B is in LLC1.
> > we will scan the cluster of A before scanning the whole LLC0, in this case,
> > cluster of A is the closest sibling, so it is the better choice than other CPUs
> > which are in LLC0 but not in the cluster of A.
>
> Yes, this is right.
>
> > But we do scan all cpus of LLC0
> > afterwards if we fail to find an idle CPU in the cluster.
>
> However my reading of the patch, before we can scan other clusters within
> the LLC (aka LLC0), we have a check in scan cluster which says
>
>         /* Don't ping-pong tasks in and out cluster frequently */
>         if (cpus_share_resources(target, prev_cpu))
>            return target;
>
> My reading of this is, ignore other clusters (at this point, we know there
> are no idle CPUs in this cluster. We don't know if there are idle cpus in
> them or not) if the previous CPU and target CPU happen to be from the same
> cluster. This effectively means we are given preference to cache over idle
> CPU.

Note we only ignore other cluster while prev_cpu and target are in same
cluster. if the condition is false, we are not ignoring other cpus. typically,
if waker is the target, and wakee is the prev_cpu, that means if they are
already in one cluster, we don't stupidly spread them in select_idle_cpu() path
as benchmark shows we are losing. so, yes, we are giving preference to
cache over CPU.

>
> Or Am I still missing something?
>
> >
> > After a while, if the cluster of A gets an idle CPU and pulls B into the
> > cluster, we prefer not pushing B out of the cluster of A again though
> > there might be an idle CPU outside. as benchmark shows getting an
> > idle CPU out of the cluster of A doesn't bring performance improvement
> > but performance decreases as B might be getting in and getting out
> > the cluster of A very frequently, then cache coherence ping-pong.
> >
>
> The counter argument can be that Task A and Task B are related and were
> running on the same cluster. But Load balancer moved Task B to a different
> cluster. Now this check may cause them to continue to run on two different
> clusters, even though the underlying load balance issues may have changed.
>
> No?

LB is much slower than select_idle_cpu().  select_idle_cpu() can dynamically
work afterwards. so it is always a dynamic balance and task migration.

>
>
> --
> Thanks and Regards
> Srikar Dronamraju

Thanks
Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-01 20:20             ` Barry Song
@ 2022-02-04  7:33               ` Srikar Dronamraju
  2022-02-04 10:28                 ` Barry Song
  0 siblings, 1 reply; 27+ messages in thread
From: Srikar Dronamraju @ 2022-02-04  7:33 UTC (permalink / raw)
  To: Barry Song
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

* Barry Song <21cnbao@gmail.com> [2022-02-02 09:20:32]:

> On Tue, Feb 1, 2022 at 10:39 PM Srikar Dronamraju
> <srikar@linux.vnet.ibm.com> wrote:
> >
> > * Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:
> >
> > > On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> > > <srikar@linux.vnet.ibm.com> wrote:
> > > >
> > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> > > >
> > > > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > > > <gautham.shenoy@amd.com> wrote:
> > > > > >
> > > > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > > > > > > From: Barry Song <song.bao.hua@hisilicon.com>
> > > > > > >
> > > I am sorry I didn't get your question. Currently the code works as below:
> > > if task A wakes up task B, and task A is in LLC0 and task B is in LLC1.
> > > we will scan the cluster of A before scanning the whole LLC0, in this case,
> > > cluster of A is the closest sibling, so it is the better choice than other CPUs
> > > which are in LLC0 but not in the cluster of A.
> >
> > Yes, this is right.
> >
> > > But we do scan all cpus of LLC0
> > > afterwards if we fail to find an idle CPU in the cluster.
> >
> > However my reading of the patch, before we can scan other clusters within
> > the LLC (aka LLC0), we have a check in scan cluster which says
> >
> >         /* Don't ping-pong tasks in and out cluster frequently */
> >         if (cpus_share_resources(target, prev_cpu))
> >            return target;
> >
> > My reading of this is, ignore other clusters (at this point, we know there
> > are no idle CPUs in this cluster. We don't know if there are idle cpus in
> > them or not) if the previous CPU and target CPU happen to be from the same
> > cluster. This effectively means we are given preference to cache over idle
> > CPU.
> 
> Note we only ignore other cluster while prev_cpu and target are in same
> cluster. if the condition is false, we are not ignoring other cpus. typically,
> if waker is the target, and wakee is the prev_cpu, that means if they are
> already in one cluster, we don't stupidly spread them in select_idle_cpu() path
> as benchmark shows we are losing. so, yes, we are giving preference to
> cache over CPU.

We already figured out that there are no idle CPUs in this cluster. So dont
we gain performance by picking a idle CPU/core in the neighbouring cluster.
If there are no idle CPU/core in the neighbouring cluster, then it does make
sense to fallback on the current cluster.

> 
> >
> > Or Am I still missing something?
> >
> > >
> > > After a while, if the cluster of A gets an idle CPU and pulls B into the
> > > cluster, we prefer not pushing B out of the cluster of A again though
> > > there might be an idle CPU outside. as benchmark shows getting an
> > > idle CPU out of the cluster of A doesn't bring performance improvement
> > > but performance decreases as B might be getting in and getting out
> > > the cluster of A very frequently, then cache coherence ping-pong.
> > >
> >
> > The counter argument can be that Task A and Task B are related and were
> > running on the same cluster. But Load balancer moved Task B to a different
> > cluster. Now this check may cause them to continue to run on two different
> > clusters, even though the underlying load balance issues may have changed.
> >
> > No?
> 
> LB is much slower than select_idle_cpu().  select_idle_cpu() can dynamically
> work afterwards. so it is always a dynamic balance and task migration.
> 
> >
> >
> > --
> > Thanks and Regards
> > Srikar Dronamraju
> 
> Thanks
> Barry

-- 
Thanks and Regards
Srikar Dronamraju

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-04  7:33               ` Srikar Dronamraju
@ 2022-02-04 10:28                 ` Barry Song
  2022-02-04 10:49                   ` Barry Song
  2022-02-07 15:14                   ` Gautham R. Shenoy
  0 siblings, 2 replies; 27+ messages in thread
From: Barry Song @ 2022-02-04 10:28 UTC (permalink / raw)
  To: Srikar Dronamraju
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Fri, Feb 4, 2022 at 8:33 PM Srikar Dronamraju
<srikar@linux.vnet.ibm.com> wrote:
>
> * Barry Song <21cnbao@gmail.com> [2022-02-02 09:20:32]:
>
> > On Tue, Feb 1, 2022 at 10:39 PM Srikar Dronamraju
> > <srikar@linux.vnet.ibm.com> wrote:
> > >
> > > * Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:
> > >
> > > > On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > >
> > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> > > > >
> > > > > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > > > > <gautham.shenoy@amd.com> wrote:
> > > > > > >
> > > > > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > > > > > > > From: Barry Song <song.bao.hua@hisilicon.com>
> > > > > > > >
> > > > I am sorry I didn't get your question. Currently the code works as below:
> > > > if task A wakes up task B, and task A is in LLC0 and task B is in LLC1.
> > > > we will scan the cluster of A before scanning the whole LLC0, in this case,
> > > > cluster of A is the closest sibling, so it is the better choice than other CPUs
> > > > which are in LLC0 but not in the cluster of A.
> > >
> > > Yes, this is right.
> > >
> > > > But we do scan all cpus of LLC0
> > > > afterwards if we fail to find an idle CPU in the cluster.
> > >
> > > However my reading of the patch, before we can scan other clusters within
> > > the LLC (aka LLC0), we have a check in scan cluster which says
> > >
> > >         /* Don't ping-pong tasks in and out cluster frequently */
> > >         if (cpus_share_resources(target, prev_cpu))
> > >            return target;
> > >
> > > My reading of this is, ignore other clusters (at this point, we know there
> > > are no idle CPUs in this cluster. We don't know if there are idle cpus in
> > > them or not) if the previous CPU and target CPU happen to be from the same
> > > cluster. This effectively means we are given preference to cache over idle
> > > CPU.
> >
> > Note we only ignore other cluster while prev_cpu and target are in same
> > cluster. if the condition is false, we are not ignoring other cpus. typically,
> > if waker is the target, and wakee is the prev_cpu, that means if they are
> > already in one cluster, we don't stupidly spread them in select_idle_cpu() path
> > as benchmark shows we are losing. so, yes, we are giving preference to
> > cache over CPU.
>
> We already figured out that there are no idle CPUs in this cluster. So dont
> we gain performance by picking a idle CPU/core in the neighbouring cluster.
> If there are no idle CPU/core in the neighbouring cluster, then it does make
> sense to fallback on the current cluster.

What you suggested is exactly the approach we have tried at the first beginning
during debugging. but we didn't gain performance according to benchmark, we
were actually losing. that is why we added this line to stop ping-pong:
         /* Don't ping-pong tasks in and out cluster frequently */
         if (cpus_share_resources(target, prev_cpu))
            return target;

If we delete this, we are seeing a big loss of tbench while system
load is medium
and above.

>
> >
> > >
> > > Or Am I still missing something?
> > >
> > > >
> > > > After a while, if the cluster of A gets an idle CPU and pulls B into the
> > > > cluster, we prefer not pushing B out of the cluster of A again though
> > > > there might be an idle CPU outside. as benchmark shows getting an
> > > > idle CPU out of the cluster of A doesn't bring performance improvement
> > > > but performance decreases as B might be getting in and getting out
> > > > the cluster of A very frequently, then cache coherence ping-pong.
> > > >
> > >
> > > The counter argument can be that Task A and Task B are related and were
> > > running on the same cluster. But Load balancer moved Task B to a different
> > > cluster. Now this check may cause them to continue to run on two different
> > > clusters, even though the underlying load balance issues may have changed.
> > >
> > > No?
> >
> > LB is much slower than select_idle_cpu().  select_idle_cpu() can dynamically
> > work afterwards. so it is always a dynamic balance and task migration.
> >
> > >
> > >
> > > --
> > > Thanks and Regards
> > > Srikar Dronamraju
> >
> > Thanks
> > Barry
>
> --

Thanks
Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-04 10:28                 ` Barry Song
@ 2022-02-04 10:49                   ` Barry Song
  2022-02-04 17:41                     ` Tim Chen
  2022-02-07 15:14                   ` Gautham R. Shenoy
  1 sibling, 1 reply; 27+ messages in thread
From: Barry Song @ 2022-02-04 10:49 UTC (permalink / raw)
  To: Srikar Dronamraju
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Fri, Feb 4, 2022 at 11:28 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Fri, Feb 4, 2022 at 8:33 PM Srikar Dronamraju
> <srikar@linux.vnet.ibm.com> wrote:
> >
> > * Barry Song <21cnbao@gmail.com> [2022-02-02 09:20:32]:
> >
> > > On Tue, Feb 1, 2022 at 10:39 PM Srikar Dronamraju
> > > <srikar@linux.vnet.ibm.com> wrote:
> > > >
> > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:
> > > >
> > > > > On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> > > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > >
> > > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> > > > > >
> > > > > > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > > > > > <gautham.shenoy@amd.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang wrote:
> > > > > > > > > From: Barry Song <song.bao.hua@hisilicon.com>
> > > > > > > > >
> > > > > I am sorry I didn't get your question. Currently the code works as below:
> > > > > if task A wakes up task B, and task A is in LLC0 and task B is in LLC1.
> > > > > we will scan the cluster of A before scanning the whole LLC0, in this case,
> > > > > cluster of A is the closest sibling, so it is the better choice than other CPUs
> > > > > which are in LLC0 but not in the cluster of A.
> > > >
> > > > Yes, this is right.
> > > >
> > > > > But we do scan all cpus of LLC0
> > > > > afterwards if we fail to find an idle CPU in the cluster.
> > > >
> > > > However my reading of the patch, before we can scan other clusters within
> > > > the LLC (aka LLC0), we have a check in scan cluster which says
> > > >
> > > >         /* Don't ping-pong tasks in and out cluster frequently */
> > > >         if (cpus_share_resources(target, prev_cpu))
> > > >            return target;
> > > >
> > > > My reading of this is, ignore other clusters (at this point, we know there
> > > > are no idle CPUs in this cluster. We don't know if there are idle cpus in
> > > > them or not) if the previous CPU and target CPU happen to be from the same
> > > > cluster. This effectively means we are given preference to cache over idle
> > > > CPU.
> > >
> > > Note we only ignore other cluster while prev_cpu and target are in same
> > > cluster. if the condition is false, we are not ignoring other cpus. typically,
> > > if waker is the target, and wakee is the prev_cpu, that means if they are
> > > already in one cluster, we don't stupidly spread them in select_idle_cpu() path
> > > as benchmark shows we are losing. so, yes, we are giving preference to
> > > cache over CPU.
> >
> > We already figured out that there are no idle CPUs in this cluster. So dont
> > we gain performance by picking a idle CPU/core in the neighbouring cluster.
> > If there are no idle CPU/core in the neighbouring cluster, then it does make
> > sense to fallback on the current cluster.
>
> What you suggested is exactly the approach we have tried at the first beginning
> during debugging. but we didn't gain performance according to benchmark, we
> were actually losing. that is why we added this line to stop ping-pong:
>          /* Don't ping-pong tasks in and out cluster frequently */
>          if (cpus_share_resources(target, prev_cpu))
>             return target;
>
> If we delete this, we are seeing a big loss of tbench while system
> load is medium
> and above.

While one system has a high load,  if we scan the neighbour clusters
after we fail to
get an idle cpu in the target. we can really successfully find an idle
cpu in neighbours
sometimes. There is no doubt of this. Our experiments have shown this is 100%
true. But the problem is that actually all cpus are very busy, so each
cpu gets a
very small idle time. After we migrate tasks to neighbours,  shortly
the neighbour
clusters will be full of tasks, then the neighbours might kick their
tasks out afterwards.
so we will see tasks move all around many clusters like monkeys, then we don't
gain performance.

>
> >
> > >
> > > >
> > > > Or Am I still missing something?
> > > >
> > > > >
> > > > > After a while, if the cluster of A gets an idle CPU and pulls B into the
> > > > > cluster, we prefer not pushing B out of the cluster of A again though
> > > > > there might be an idle CPU outside. as benchmark shows getting an
> > > > > idle CPU out of the cluster of A doesn't bring performance improvement
> > > > > but performance decreases as B might be getting in and getting out
> > > > > the cluster of A very frequently, then cache coherence ping-pong.
> > > > >
> > > >
> > > > The counter argument can be that Task A and Task B are related and were
> > > > running on the same cluster. But Load balancer moved Task B to a different
> > > > cluster. Now this check may cause them to continue to run on two different
> > > > clusters, even though the underlying load balance issues may have changed.
> > > >
> > > > No?
> > >
> > > LB is much slower than select_idle_cpu().  select_idle_cpu() can dynamically
> > > work afterwards. so it is always a dynamic balance and task migration.
> > >
> > > >
> > > >
> > > > --
> > > > Thanks and Regards
> > > > Srikar Dronamraju
> > >
> > > Thanks
> > > Barry
> >
> > --
>

Thanks
Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-04 10:49                   ` Barry Song
@ 2022-02-04 17:41                     ` Tim Chen
  2022-02-05 17:16                       ` Chen Yu
  0 siblings, 1 reply; 27+ messages in thread
From: Tim Chen @ 2022-02-04 17:41 UTC (permalink / raw)
  To: Barry Song, Srikar Dronamraju
  Cc: Gautham R. Shenoy, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, LKML, LAK, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Daniel Bristot de Oliveira,
	prime.zeng, Jonathan Cameron, ego, Linuxarm, Barry Song,
	Guodong Xu, yu.c.chen

On Fri, 2022-02-04 at 23:49 +1300, Barry Song wrote:
> On Fri, Feb 4, 2022 at 11:28 PM Barry Song <21cnbao@gmail.com> wrote:
> > On Fri, Feb 4, 2022 at 8:33 PM Srikar Dronamraju
> > <srikar@linux.vnet.ibm.com> wrote:
> > > * Barry Song <21cnbao@gmail.com> [2022-02-02 09:20:32]:
> > > 
> > > > On Tue, Feb 1, 2022 at 10:39 PM Srikar Dronamraju
> > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:
> > > > > 
> > > > > > On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> > > > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> > > > > > > 
> > > > > > > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > > > > > > <gautham.shenoy@amd.com> wrote:
> > > > > > > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang
> > > > > > > > > wrote:
> > > > > > > > > > From: Barry Song <song.bao.hua@hisilicon.com>
> > > > > > > > > > 
> > > > > > I am sorry I didn't get your question. Currently the code
> > > > > > works as below:
> > > > > > if task A wakes up task B, and task A is in LLC0 and task B
> > > > > > is in LLC1.
> > > > > > we will scan the cluster of A before scanning the whole
> > > > > > LLC0, in this case,
> > > > > > cluster of A is the closest sibling, so it is the better
> > > > > > choice than other CPUs
> > > > > > which are in LLC0 but not in the cluster of A.
> > > > > 
> > > > > Yes, this is right.
> > > > > 
> > > > > > But we do scan all cpus of LLC0
> > > > > > afterwards if we fail to find an idle CPU in the cluster.
> > > > > 
> > > > > However my reading of the patch, before we can scan other
> > > > > clusters within
> > > > > the LLC (aka LLC0), we have a check in scan cluster which
> > > > > says
> > > > > 
> > > > >         /* Don't ping-pong tasks in and out cluster
> > > > > frequently */
> > > > >         if (cpus_share_resources(target, prev_cpu))
> > > > >            return target;
> > > > > 
> > > > > My reading of this is, ignore other clusters (at this point,
> > > > > we know there
> > > > > are no idle CPUs in this cluster. We don't know if there are
> > > > > idle cpus in
> > > > > them or not) if the previous CPU and target CPU happen to be
> > > > > from the same
> > > > > cluster. This effectively means we are given preference to
> > > > > cache over idle
> > > > > CPU.
> > > > 
> > > > Note we only ignore other cluster while prev_cpu and target are
> > > > in same
> > > > cluster. if the condition is false, we are not ignoring other
> > > > cpus. typically,
> > > > if waker is the target, and wakee is the prev_cpu, that means
> > > > if they are
> > > > already in one cluster, we don't stupidly spread them in
> > > > select_idle_cpu() path
> > > > as benchmark shows we are losing. so, yes, we are giving
> > > > preference to
> > > > cache over CPU.
> > > 
> > > We already figured out that there are no idle CPUs in this
> > > cluster. So dont
> > > we gain performance by picking a idle CPU/core in the
> > > neighbouring cluster.
> > > If there are no idle CPU/core in the neighbouring cluster, then
> > > it does make
> > > sense to fallback on the current cluster.
> > 
> > 

We may need to take into consideration the utilization and
load average for the source and target cluster to make
better decision of whether it is worth placing the
task in the next cluster.  If the load of the target
cluster is too high, it is not worth pushing the task there.

Those stats can be gathered during load balancing without adding
overhead in the hot task wakeup path.

Chen Yu played around with cutting off the idle CPU search
in a LLC based on such stats and he saw some good
improvements over the default.

Tim


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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-04 17:41                     ` Tim Chen
@ 2022-02-05 17:16                       ` Chen Yu
  2022-02-06  0:26                         ` Barry Song
  0 siblings, 1 reply; 27+ messages in thread
From: Chen Yu @ 2022-02-05 17:16 UTC (permalink / raw)
  To: Tim Chen
  Cc: Barry Song, Srikar Dronamraju, Gautham R. Shenoy, Yicong Yang,
	Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot, LKML,
	LAK, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Fri, Feb 04, 2022 at 09:41:21AM -0800, Tim Chen wrote:
> On Fri, 2022-02-04 at 23:49 +1300, Barry Song wrote:
> > On Fri, Feb 4, 2022 at 11:28 PM Barry Song <21cnbao@gmail.com> wrote:
> > > On Fri, Feb 4, 2022 at 8:33 PM Srikar Dronamraju
> > > <srikar@linux.vnet.ibm.com> wrote:
> > > > * Barry Song <21cnbao@gmail.com> [2022-02-02 09:20:32]:
> > > > 
> > > > > On Tue, Feb 1, 2022 at 10:39 PM Srikar Dronamraju
> > > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:
> > > > > > 
> > > > > > > On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> > > > > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> > > > > > > > 
> > > > > > > > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > > > > > > > <gautham.shenoy@amd.com> wrote:
> > > > > > > > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang
> > > > > > > > > > wrote:
> > > > > > > > > > > From: Barry Song <song.bao.hua@hisilicon.com>
> > > > > > > > > > > 
> > > > > > > I am sorry I didn't get your question. Currently the code
> > > > > > > works as below:
> > > > > > > if task A wakes up task B, and task A is in LLC0 and task B
> > > > > > > is in LLC1.
> > > > > > > we will scan the cluster of A before scanning the whole
> > > > > > > LLC0, in this case,
> > > > > > > cluster of A is the closest sibling, so it is the better
> > > > > > > choice than other CPUs
> > > > > > > which are in LLC0 but not in the cluster of A.
> > > > > > 
> > > > > > Yes, this is right.
> > > > > > 
> > > > > > > But we do scan all cpus of LLC0
> > > > > > > afterwards if we fail to find an idle CPU in the cluster.
> > > > > > 
> > > > > > However my reading of the patch, before we can scan other
> > > > > > clusters within
> > > > > > the LLC (aka LLC0), we have a check in scan cluster which
> > > > > > says
> > > > > > 
> > > > > >         /* Don't ping-pong tasks in and out cluster
> > > > > > frequently */
> > > > > >         if (cpus_share_resources(target, prev_cpu))
> > > > > >            return target;
> > > > > > 
> > > > > > My reading of this is, ignore other clusters (at this point,
> > > > > > we know there
> > > > > > are no idle CPUs in this cluster. We don't know if there are
> > > > > > idle cpus in
> > > > > > them or not) if the previous CPU and target CPU happen to be
> > > > > > from the same
> > > > > > cluster. This effectively means we are given preference to
> > > > > > cache over idle
> > > > > > CPU.
> > > > > 
> > > > > Note we only ignore other cluster while prev_cpu and target are
> > > > > in same
> > > > > cluster. if the condition is false, we are not ignoring other
> > > > > cpus. typically,
> > > > > if waker is the target, and wakee is the prev_cpu, that means
> > > > > if they are
> > > > > already in one cluster, we don't stupidly spread them in
> > > > > select_idle_cpu() path
> > > > > as benchmark shows we are losing. so, yes, we are giving
> > > > > preference to
> > > > > cache over CPU.
> > > > 
> > > > We already figured out that there are no idle CPUs in this
> > > > cluster. So dont
> > > > we gain performance by picking a idle CPU/core in the
> > > > neighbouring cluster.
> > > > If there are no idle CPU/core in the neighbouring cluster, then
> > > > it does make
> > > > sense to fallback on the current cluster.
> > > 
> > > 
> 
> We may need to take into consideration the utilization and
> load average for the source and target cluster to make
> better decision of whether it is worth placing the
> task in the next cluster.  If the load of the target
> cluster is too high, it is not worth pushing the task there.
> 
> Those stats can be gathered during load balancing without adding
> overhead in the hot task wakeup path.
> 
> Chen Yu played around with cutting off the idle CPU search
> in a LLC based on such stats and he saw some good
> improvements over the default.
>
Yes, we used the sum of percpu util_avg to estimate if the LLC domain
is overloaded. If it is too busy, skip searching for an idle cpu/core in
that LLC domain. The util_avg is a metric of accumulated historic
activity, which might be more accurate than instantaneous metrics(such as
rq->nr_running) on calculating the probability of find an idle cpu.
So far this change has shown some benefits in several microbenchmarks and
OLTP benchmark when the system is quite busy. That change has introduced a
per-LLC-domain flag to indicate whether the LLC domain is oveloaded,
it seems that this flag could also be extended for cluster domain.
Maybe I could post the draft patch to see if it would be helpful for this
cluster patch serie.

thanks,
Chenyu
> Tim
> 

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-05 17:16                       ` Chen Yu
@ 2022-02-06  0:26                         ` Barry Song
  0 siblings, 0 replies; 27+ messages in thread
From: Barry Song @ 2022-02-06  0:26 UTC (permalink / raw)
  To: Chen Yu
  Cc: Tim Chen, Srikar Dronamraju, Gautham R. Shenoy, Yicong Yang,
	Peter Zijlstra, Ingo Molnar, Juri Lelli, Vincent Guittot, LKML,
	LAK, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Sun, Feb 6, 2022 at 6:16 AM Chen Yu <yu.c.chen@intel.com> wrote:
>
> On Fri, Feb 04, 2022 at 09:41:21AM -0800, Tim Chen wrote:
> > On Fri, 2022-02-04 at 23:49 +1300, Barry Song wrote:
> > > On Fri, Feb 4, 2022 at 11:28 PM Barry Song <21cnbao@gmail.com> wrote:
> > > > On Fri, Feb 4, 2022 at 8:33 PM Srikar Dronamraju
> > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > * Barry Song <21cnbao@gmail.com> [2022-02-02 09:20:32]:
> > > > >
> > > > > > On Tue, Feb 1, 2022 at 10:39 PM Srikar Dronamraju
> > > > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 07:40:15]:
> > > > > > >
> > > > > > > > On Fri, Jan 28, 2022 at 8:13 PM Srikar Dronamraju
> > > > > > > > <srikar@linux.vnet.ibm.com> wrote:
> > > > > > > > > * Barry Song <21cnbao@gmail.com> [2022-01-28 09:21:08]:
> > > > > > > > >
> > > > > > > > > > On Fri, Jan 28, 2022 at 4:41 AM Gautham R. Shenoy
> > > > > > > > > > <gautham.shenoy@amd.com> wrote:
> > > > > > > > > > > On Wed, Jan 26, 2022 at 04:09:47PM +0800, Yicong Yang
> > > > > > > > > > > wrote:
> > > > > > > > > > > > From: Barry Song <song.bao.hua@hisilicon.com>
> > > > > > > > > > > >
> > > > > > > > I am sorry I didn't get your question. Currently the code
> > > > > > > > works as below:
> > > > > > > > if task A wakes up task B, and task A is in LLC0 and task B
> > > > > > > > is in LLC1.
> > > > > > > > we will scan the cluster of A before scanning the whole
> > > > > > > > LLC0, in this case,
> > > > > > > > cluster of A is the closest sibling, so it is the better
> > > > > > > > choice than other CPUs
> > > > > > > > which are in LLC0 but not in the cluster of A.
> > > > > > >
> > > > > > > Yes, this is right.
> > > > > > >
> > > > > > > > But we do scan all cpus of LLC0
> > > > > > > > afterwards if we fail to find an idle CPU in the cluster.
> > > > > > >
> > > > > > > However my reading of the patch, before we can scan other
> > > > > > > clusters within
> > > > > > > the LLC (aka LLC0), we have a check in scan cluster which
> > > > > > > says
> > > > > > >
> > > > > > >         /* Don't ping-pong tasks in and out cluster
> > > > > > > frequently */
> > > > > > >         if (cpus_share_resources(target, prev_cpu))
> > > > > > >            return target;
> > > > > > >
> > > > > > > My reading of this is, ignore other clusters (at this point,
> > > > > > > we know there
> > > > > > > are no idle CPUs in this cluster. We don't know if there are
> > > > > > > idle cpus in
> > > > > > > them or not) if the previous CPU and target CPU happen to be
> > > > > > > from the same
> > > > > > > cluster. This effectively means we are given preference to
> > > > > > > cache over idle
> > > > > > > CPU.
> > > > > >
> > > > > > Note we only ignore other cluster while prev_cpu and target are
> > > > > > in same
> > > > > > cluster. if the condition is false, we are not ignoring other
> > > > > > cpus. typically,
> > > > > > if waker is the target, and wakee is the prev_cpu, that means
> > > > > > if they are
> > > > > > already in one cluster, we don't stupidly spread them in
> > > > > > select_idle_cpu() path
> > > > > > as benchmark shows we are losing. so, yes, we are giving
> > > > > > preference to
> > > > > > cache over CPU.
> > > > >
> > > > > We already figured out that there are no idle CPUs in this
> > > > > cluster. So dont
> > > > > we gain performance by picking a idle CPU/core in the
> > > > > neighbouring cluster.
> > > > > If there are no idle CPU/core in the neighbouring cluster, then
> > > > > it does make
> > > > > sense to fallback on the current cluster.
> > > >
> > > >
> >
> > We may need to take into consideration the utilization and
> > load average for the source and target cluster to make
> > better decision of whether it is worth placing the
> > task in the next cluster.  If the load of the target
> > cluster is too high, it is not worth pushing the task there.
> >
> > Those stats can be gathered during load balancing without adding
> > overhead in the hot task wakeup path.
> >
> > Chen Yu played around with cutting off the idle CPU search
> > in a LLC based on such stats and he saw some good
> > improvements over the default.
> >
> Yes, we used the sum of percpu util_avg to estimate if the LLC domain
> is overloaded. If it is too busy, skip searching for an idle cpu/core in
> that LLC domain. The util_avg is a metric of accumulated historic
> activity, which might be more accurate than instantaneous metrics(such as
> rq->nr_running) on calculating the probability of find an idle cpu.
> So far this change has shown some benefits in several microbenchmarks and
> OLTP benchmark when the system is quite busy. That change has introduced a
> per-LLC-domain flag to indicate whether the LLC domain is oveloaded,
> it seems that this flag could also be extended for cluster domain.
> Maybe I could post the draft patch to see if it would be helpful for this
> cluster patch serie.

yes. please send. my feeling is that select_idle_cpu() can select an "idle"cpu
which is actually very busy, but can be in "idle" state for a very
short period. it
is not always correct to get this kind of "idle" cpu. It could be
better to be still.
I am not quite sure your patch is directly related with clusters, but we will
try to figure out some connection, maybe we can integrate your patch into
this series afterwards.

>
> thanks,
> Chenyu
> > Tim
> >

Thanks
Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-04 10:28                 ` Barry Song
  2022-02-04 10:49                   ` Barry Song
@ 2022-02-07 15:14                   ` Gautham R. Shenoy
  2022-02-08  5:42                     ` Barry Song
  1 sibling, 1 reply; 27+ messages in thread
From: Gautham R. Shenoy @ 2022-02-07 15:14 UTC (permalink / raw)
  To: Barry Song
  Cc: Srikar Dronamraju, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu


On Fri, Feb 04, 2022 at 11:28:25PM +1300, Barry Song wrote:

> > We already figured out that there are no idle CPUs in this cluster. So dont
> > we gain performance by picking a idle CPU/core in the neighbouring cluster.
> > If there are no idle CPU/core in the neighbouring cluster, then it does make
> > sense to fallback on the current cluster.
> 
> What you suggested is exactly the approach we have tried at the first beginning
> during debugging. but we didn't gain performance according to benchmark, we
> were actually losing. that is why we added this line to stop ping-pong:
>          /* Don't ping-pong tasks in and out cluster frequently */
>          if (cpus_share_resources(target, prev_cpu))
>             return target;
> 
> If we delete this, we are seeing a big loss of tbench while system
> load is medium
> and above.

Thanks for clarifying this Barry. Indeed, if the workload is sensitive
to data ping-ponging across L2 clusters, this heuristic makes sense. I
was thinking of workloads that require lower tail latency, in which
case exploring the larger LLC would have made more sense, assuming
that the larger LLC has an idle core/CPU.

In the absence of any hints from the workload, like something that
Peter had previous suggested
(https://lore.kernel.org/lkml/YVwnsrZWrnWHaoqN@hirez.programming.kicks-ass.net/),
optimizing for cache-access seems to be the right thing to do.


> 
> Thanks
> Barry

--
Thanks and Regards
gautham.

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-07 15:14                   ` Gautham R. Shenoy
@ 2022-02-08  5:42                     ` Barry Song
  2022-02-16  9:12                       ` Barry Song
  0 siblings, 1 reply; 27+ messages in thread
From: Barry Song @ 2022-02-08  5:42 UTC (permalink / raw)
  To: Gautham R. Shenoy
  Cc: Srikar Dronamraju, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Tue, Feb 8, 2022 at 4:14 AM Gautham R. Shenoy <gautham.shenoy@amd.com> wrote:
>
>
> On Fri, Feb 04, 2022 at 11:28:25PM +1300, Barry Song wrote:
>
> > > We already figured out that there are no idle CPUs in this cluster. So dont
> > > we gain performance by picking a idle CPU/core in the neighbouring cluster.
> > > If there are no idle CPU/core in the neighbouring cluster, then it does make
> > > sense to fallback on the current cluster.
> >
> > What you suggested is exactly the approach we have tried at the first beginning
> > during debugging. but we didn't gain performance according to benchmark, we
> > were actually losing. that is why we added this line to stop ping-pong:
> >          /* Don't ping-pong tasks in and out cluster frequently */
> >          if (cpus_share_resources(target, prev_cpu))
> >             return target;
> >
> > If we delete this, we are seeing a big loss of tbench while system
> > load is medium
> > and above.
>
> Thanks for clarifying this Barry. Indeed, if the workload is sensitive
> to data ping-ponging across L2 clusters, this heuristic makes sense. I
> was thinking of workloads that require lower tail latency, in which
> case exploring the larger LLC would have made more sense, assuming
> that the larger LLC has an idle core/CPU.
>
> In the absence of any hints from the workload, like something that
> Peter had previous suggested
> (https://lore.kernel.org/lkml/YVwnsrZWrnWHaoqN@hirez.programming.kicks-ass.net/),
> optimizing for cache-access seems to be the right thing to do.

Thanks, gautham.

Yep. Peter mentioned some hints like SCHED_BATCH and SCHED_IDLE.
To me, the case we are discussing seems to be more complicated than
applying some scheduling policy on separate tasks by SCHED_BATCH
or IDLE.

For example, in case we have a process, and this process has 20 threads.
thread0-9 might care about cache-coherence latency and want to avoid
ping-ponging, and thread10-thread19 might want to have tail-latency
as small as possible. So we need some way to tell kernel, "hey, bro, please
try to keep thread0-9 still as ping-ponging will hurt them while trying your
best to find idle cpu in a wider range for thread10-19". But it seems
SCHED_XXX as a scheduler policy hint can't tell kernel how to organize tasks
into groups, and is also incapable of telling kernel different groups have
different needs.

So it seems we want some special cgroups to organize tasks and we can apply
some special hints on each different group. for example, putting thread0-9
in a cgroup and thread10-19 in another, then:
1. apply "COMMUNCATION-SENSITVE" on the 1st group
2. apply "TAIL-LATENCY-SENTIVE" on the 2nd one.
I am not quite sure how to do this and if this can find its way into
the mainline.

On the other hand, for this particular patch, the most controversial
part is those
two lines to avoid ping-ponging, and I am seeing dropping this can hurt workload
like tbench only when system load is high, so I wonder if the approach[1] from
Chen Yu and Tim can somehow resolve the problem alternatively, thus we can
avoid the controversial part.
since their patch can also shrink the scanning range while llc load is high.

[1] https://lore.kernel.org/lkml/20220207034013.599214-1-yu.c.chen@intel.com/

>
>
> >
> > Thanks
> > Barry
>
> --
> Thanks and Regards
> gautham.

Thanks
Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-08  5:42                     ` Barry Song
@ 2022-02-16  9:12                       ` Barry Song
  2022-02-16  9:19                         ` Song Bao Hua (Barry Song)
  0 siblings, 1 reply; 27+ messages in thread
From: Barry Song @ 2022-02-16  9:12 UTC (permalink / raw)
  To: Gautham R. Shenoy
  Cc: Srikar Dronamraju, Yicong Yang, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, prime.zeng, Jonathan Cameron, ego,
	Linuxarm, Barry Song, Guodong Xu

On Tue, Feb 8, 2022 at 6:42 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Tue, Feb 8, 2022 at 4:14 AM Gautham R. Shenoy <gautham.shenoy@amd.com> wrote:
> >
> >
> > On Fri, Feb 04, 2022 at 11:28:25PM +1300, Barry Song wrote:
> >
> > > > We already figured out that there are no idle CPUs in this cluster. So dont
> > > > we gain performance by picking a idle CPU/core in the neighbouring cluster.
> > > > If there are no idle CPU/core in the neighbouring cluster, then it does make
> > > > sense to fallback on the current cluster.
> > >
> > > What you suggested is exactly the approach we have tried at the first beginning
> > > during debugging. but we didn't gain performance according to benchmark, we
> > > were actually losing. that is why we added this line to stop ping-pong:
> > >          /* Don't ping-pong tasks in and out cluster frequently */
> > >          if (cpus_share_resources(target, prev_cpu))
> > >             return target;
> > >
> > > If we delete this, we are seeing a big loss of tbench while system
> > > load is medium
> > > and above.
> >
> > Thanks for clarifying this Barry. Indeed, if the workload is sensitive
> > to data ping-ponging across L2 clusters, this heuristic makes sense. I
> > was thinking of workloads that require lower tail latency, in which
> > case exploring the larger LLC would have made more sense, assuming
> > that the larger LLC has an idle core/CPU.
> >
> > In the absence of any hints from the workload, like something that
> > Peter had previous suggested
> > (https://lore.kernel.org/lkml/YVwnsrZWrnWHaoqN@hirez.programming.kicks-ass.net/),
> > optimizing for cache-access seems to be the right thing to do.
>
> Thanks, gautham.
>
> Yep. Peter mentioned some hints like SCHED_BATCH and SCHED_IDLE.
> To me, the case we are discussing seems to be more complicated than
> applying some scheduling policy on separate tasks by SCHED_BATCH
> or IDLE.
>
> For example, in case we have a process, and this process has 20 threads.
> thread0-9 might care about cache-coherence latency and want to avoid
> ping-ponging, and thread10-thread19 might want to have tail-latency
> as small as possible. So we need some way to tell kernel, "hey, bro, please
> try to keep thread0-9 still as ping-ponging will hurt them while trying your
> best to find idle cpu in a wider range for thread10-19". But it seems
> SCHED_XXX as a scheduler policy hint can't tell kernel how to organize tasks
> into groups, and is also incapable of telling kernel different groups have
> different needs.
>
> So it seems we want some special cgroups to organize tasks and we can apply
> some special hints on each different group. for example, putting thread0-9
> in a cgroup and thread10-19 in another, then:
> 1. apply "COMMUNCATION-SENSITVE" on the 1st group
> 2. apply "TAIL-LATENCY-SENTIVE" on the 2nd one.
> I am not quite sure how to do this and if this can find its way into
> the mainline.
>
> On the other hand, for this particular patch, the most controversial
> part is those
> two lines to avoid ping-ponging, and I am seeing dropping this can hurt workload
> like tbench only when system load is high, so I wonder if the approach[1] from
> Chen Yu and Tim can somehow resolve the problem alternatively, thus we can
> avoid the controversial part.
> since their patch can also shrink the scanning range while llc load is high.
>
> [1] https://lore.kernel.org/lkml/20220207034013.599214-1-yu.c.chen@intel.com/

Yicong's testing shows the patch from Chen Yu and Tim can somehow resolve the
problem and make sure there is no performance regression for tbench
while load is
high after we remove the code to avoid ping-pong:

5.17-rc1: vanilla
rc1 + chenyu: vanilla + chenyu's LLC overload patch
rc1+chenyu+cls: vanilla + chenyu's  patch + my this patchset
rc1+chenyu+cls-pingpong: vanilla + chenyu's patch + my this patchset -
the code avoiding ping-pong
rc1+cls: vanilla + my this patchset

tbench running on numa 0 &1:
                            5.17-rc1          rc1 + chenyu
rc1+chenyu+cls     rc1+chenyu+cls-pingpong  rc1+cls
Hmean     1        320.01 (   0.00%)      318.03 *  -0.62%*
357.15 *  11.61%*      375.43 *  17.32%*      378.44 *  18.26%*
Hmean     2        643.85 (   0.00%)      637.74 *  -0.95%*
714.36 *  10.95%*      745.82 *  15.84%*      752.52 *  16.88%*
Hmean     4       1287.36 (   0.00%)     1285.20 *  -0.17%*
1431.35 *  11.18%*     1481.71 *  15.10%*     1505.62 *  16.95%*
Hmean     8       2564.60 (   0.00%)     2551.02 *  -0.53%*
2812.74 *   9.68%*     2921.51 *  13.92%*     2955.29 *  15.23%*
Hmean     16      5195.69 (   0.00%)     5163.39 *  -0.62%*
5583.28 *   7.46%*     5726.08 *  10.21%*     5814.74 *  11.91%*
Hmean     32      9769.16 (   0.00%)     9815.63 *   0.48%*
10518.35 *   7.67%*    10852.89 *  11.09%*    10872.63 *  11.30%*
Hmean     64     15952.50 (   0.00%)    15780.41 *  -1.08%*
10608.36 * -33.50%*    17503.42 *   9.72%*    17281.98 *   8.33%*
Hmean     128    13113.77 (   0.00%)    12000.12 *  -8.49%*
13095.50 *  -0.14%*    13991.90 *   6.70%*    13895.20 *   5.96%*
Hmean     256    10997.59 (   0.00%)    12229.20 *  11.20%*
11902.60 *   8.23%*    12214.29 *  11.06%*    11244.69 *   2.25%*
Hmean     512    14623.60 (   0.00%)    15863.25 *   8.48%*
14103.38 *  -3.56%*    16422.56 *  12.30%*    15526.25 *   6.17%*

tbench running on numa 0 only:

                            5.17-rc1          rc1 + chenyu
rc1+chenyu+cls     rc1+chenyu+cls-pingpong   rc1+cls
Hmean     1        324.73 (   0.00%)      330.96 *   1.92%*
358.97 *  10.54%*      376.05 *  15.80%*      378.01 *  16.41%*
Hmean     2        645.36 (   0.00%)      643.13 *  -0.35%*
710.78 *  10.14%*      744.34 *  15.34%*      754.63 *  16.93%*
Hmean     4       1302.09 (   0.00%)     1297.11 *  -0.38%*
1425.22 *   9.46%*     1484.92 *  14.04%*     1507.54 *  15.78%*
Hmean     8       2612.03 (   0.00%)     2623.60 *   0.44%*
2843.15 *   8.85%*     2937.81 *  12.47%*     2982.57 *  14.19%*
Hmean     16      5307.12 (   0.00%)     5304.14 *  -0.06%*
5610.46 *   5.72%*     5763.24 *   8.59%*     5886.66 *  10.92%*
Hmean     32      9354.22 (   0.00%)     9738.21 *   4.11%*
9360.21 *   0.06%*     9699.05 *   3.69%*     9908.13 *   5.92%*
Hmean     64      7240.35 (   0.00%)     7210.75 *  -0.41%*
6992.70 *  -3.42%*     7321.52 *   1.12%*     7278.78 *   0.53%*
Hmean     128     6186.40 (   0.00%)     6314.89 *   2.08%*
6166.44 *  -0.32%*     6279.85 *   1.51%*     6187.85 (   0.02%)
Hmean     256     9231.40 (   0.00%)     9469.26 *   2.58%*
9134.42 *  -1.05%*     9322.88 *   0.99%*     9448.61 *   2.35%*
Hmean     512     8907.13 (   0.00%)     9130.46 *   2.51%*
9023.87 *   1.31%*     9276.19 *   4.14%*     9397.22 *   5.50%*

as you can see rc1+chenyu+cls-pingpong still shows similar improvement
like rc1+cls, in some
cases(256, 512 threads on numa0&1), it is even much better.

Thanks
Barry

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

* RE: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-16  9:12                       ` Barry Song
@ 2022-02-16  9:19                         ` Song Bao Hua (Barry Song)
  2022-02-16 10:00                           ` Yicong Yang
  0 siblings, 1 reply; 27+ messages in thread
From: Song Bao Hua (Barry Song) @ 2022-02-16  9:19 UTC (permalink / raw)
  To: Barry Song, Gautham R. Shenoy
  Cc: Srikar Dronamraju, yangyicong, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, Zengtao (B),
	Jonathan Cameron, ego, Linuxarm, Guodong Xu



> -----Original Message-----
> From: Barry Song [mailto:21cnbao@gmail.com]
> Sent: Wednesday, February 16, 2022 10:13 PM
> To: Gautham R. Shenoy <gautham.shenoy@amd.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>; yangyicong
> <yangyicong@huawei.com>; Peter Zijlstra <peterz@infradead.org>; Ingo Molnar
> <mingo@redhat.com>; Juri Lelli <juri.lelli@redhat.com>; Vincent Guittot
> <vincent.guittot@linaro.org>; Tim Chen <tim.c.chen@linux.intel.com>; LKML
> <linux-kernel@vger.kernel.org>; LAK <linux-arm-kernel@lists.infradead.org>;
> Dietmar Eggemann <dietmar.eggemann@arm.com>; Steven Rostedt
> <rostedt@goodmis.org>; Ben Segall <bsegall@google.com>; Daniel Bristot de
> Oliveira <bristot@redhat.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
> Jonathan Cameron <jonathan.cameron@huawei.com>; ego@linux.vnet.ibm.com;
> Linuxarm <linuxarm@huawei.com>; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; Guodong Xu <guodong.xu@linaro.org>
> Subject: Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in
> wake-up path
> 
> On Tue, Feb 8, 2022 at 6:42 PM Barry Song <21cnbao@gmail.com> wrote:
> >
> > On Tue, Feb 8, 2022 at 4:14 AM Gautham R. Shenoy <gautham.shenoy@amd.com>
> wrote:
> > >
> > >
> > > On Fri, Feb 04, 2022 at 11:28:25PM +1300, Barry Song wrote:
> > >
> > > > > We already figured out that there are no idle CPUs in this cluster.
> So dont
> > > > > we gain performance by picking a idle CPU/core in the neighbouring cluster.
> > > > > If there are no idle CPU/core in the neighbouring cluster, then it does
> make
> > > > > sense to fallback on the current cluster.
> > > >
> > > > What you suggested is exactly the approach we have tried at the first
> beginning
> > > > during debugging. but we didn't gain performance according to benchmark,
> we
> > > > were actually losing. that is why we added this line to stop ping-pong:
> > > >          /* Don't ping-pong tasks in and out cluster frequently */
> > > >          if (cpus_share_resources(target, prev_cpu))
> > > >             return target;
> > > >
> > > > If we delete this, we are seeing a big loss of tbench while system
> > > > load is medium
> > > > and above.
> > >
> > > Thanks for clarifying this Barry. Indeed, if the workload is sensitive
> > > to data ping-ponging across L2 clusters, this heuristic makes sense. I
> > > was thinking of workloads that require lower tail latency, in which
> > > case exploring the larger LLC would have made more sense, assuming
> > > that the larger LLC has an idle core/CPU.
> > >
> > > In the absence of any hints from the workload, like something that
> > > Peter had previous suggested
> > >
> (https://lore.kernel.org/lkml/YVwnsrZWrnWHaoqN@hirez.programming.kicks-ass
> .net/),
> > > optimizing for cache-access seems to be the right thing to do.
> >
> > Thanks, gautham.
> >
> > Yep. Peter mentioned some hints like SCHED_BATCH and SCHED_IDLE.
> > To me, the case we are discussing seems to be more complicated than
> > applying some scheduling policy on separate tasks by SCHED_BATCH
> > or IDLE.
> >
> > For example, in case we have a process, and this process has 20 threads.
> > thread0-9 might care about cache-coherence latency and want to avoid
> > ping-ponging, and thread10-thread19 might want to have tail-latency
> > as small as possible. So we need some way to tell kernel, "hey, bro, please
> > try to keep thread0-9 still as ping-ponging will hurt them while trying your
> > best to find idle cpu in a wider range for thread10-19". But it seems
> > SCHED_XXX as a scheduler policy hint can't tell kernel how to organize tasks
> > into groups, and is also incapable of telling kernel different groups have
> > different needs.
> >
> > So it seems we want some special cgroups to organize tasks and we can apply
> > some special hints on each different group. for example, putting thread0-9
> > in a cgroup and thread10-19 in another, then:
> > 1. apply "COMMUNCATION-SENSITVE" on the 1st group
> > 2. apply "TAIL-LATENCY-SENTIVE" on the 2nd one.
> > I am not quite sure how to do this and if this can find its way into
> > the mainline.
> >
> > On the other hand, for this particular patch, the most controversial
> > part is those
> > two lines to avoid ping-ponging, and I am seeing dropping this can hurt workload
> > like tbench only when system load is high, so I wonder if the approach[1]
> from
> > Chen Yu and Tim can somehow resolve the problem alternatively, thus we can
> > avoid the controversial part.
> > since their patch can also shrink the scanning range while llc load is high.
> >
> > [1]
> https://lore.kernel.org/lkml/20220207034013.599214-1-yu.c.chen@intel.com/
> 
> Yicong's testing shows the patch from Chen Yu and Tim can somehow resolve the
> problem and make sure there is no performance regression for tbench
> while load is
> high after we remove the code to avoid ping-pong:
> 
> 5.17-rc1: vanilla
> rc1 + chenyu: vanilla + chenyu's LLC overload patch
> rc1+chenyu+cls: vanilla + chenyu's  patch + my this patchset
> rc1+chenyu+cls-pingpong: vanilla + chenyu's patch + my this patchset -
> the code avoiding ping-pong
> rc1+cls: vanilla + my this patchset
> 
> tbench running on numa 0 &1:
>                             5.17-rc1          rc1 + chenyu
> rc1+chenyu+cls     rc1+chenyu+cls-pingpong  rc1+cls
> Hmean     1        320.01 (   0.00%)      318.03 *  -0.62%*
> 357.15 *  11.61%*      375.43 *  17.32%*      378.44 *  18.26%*
> Hmean     2        643.85 (   0.00%)      637.74 *  -0.95%*
> 714.36 *  10.95%*      745.82 *  15.84%*      752.52 *  16.88%*
> Hmean     4       1287.36 (   0.00%)     1285.20 *  -0.17%*
> 1431.35 *  11.18%*     1481.71 *  15.10%*     1505.62 *  16.95%*
> Hmean     8       2564.60 (   0.00%)     2551.02 *  -0.53%*
> 2812.74 *   9.68%*     2921.51 *  13.92%*     2955.29 *  15.23%*
> Hmean     16      5195.69 (   0.00%)     5163.39 *  -0.62%*
> 5583.28 *   7.46%*     5726.08 *  10.21%*     5814.74 *  11.91%*
> Hmean     32      9769.16 (   0.00%)     9815.63 *   0.48%*
> 10518.35 *   7.67%*    10852.89 *  11.09%*    10872.63 *  11.30%*
> Hmean     64     15952.50 (   0.00%)    15780.41 *  -1.08%*
> 10608.36 * -33.50%*    17503.42 *   9.72%*    17281.98 *   8.33%*
> Hmean     128    13113.77 (   0.00%)    12000.12 *  -8.49%*
> 13095.50 *  -0.14%*    13991.90 *   6.70%*    13895.20 *   5.96%*
> Hmean     256    10997.59 (   0.00%)    12229.20 *  11.20%*
> 11902.60 *   8.23%*    12214.29 *  11.06%*    11244.69 *   2.25%*
> Hmean     512    14623.60 (   0.00%)    15863.25 *   8.48%*
> 14103.38 *  -3.56%*    16422.56 *  12.30%*    15526.25 *   6.17%*
> 
> tbench running on numa 0 only:
> 
>                             5.17-rc1          rc1 + chenyu
> rc1+chenyu+cls     rc1+chenyu+cls-pingpong   rc1+cls
> Hmean     1        324.73 (   0.00%)      330.96 *   1.92%*
> 358.97 *  10.54%*      376.05 *  15.80%*      378.01 *  16.41%*
> Hmean     2        645.36 (   0.00%)      643.13 *  -0.35%*
> 710.78 *  10.14%*      744.34 *  15.34%*      754.63 *  16.93%*
> Hmean     4       1302.09 (   0.00%)     1297.11 *  -0.38%*
> 1425.22 *   9.46%*     1484.92 *  14.04%*     1507.54 *  15.78%*
> Hmean     8       2612.03 (   0.00%)     2623.60 *   0.44%*
> 2843.15 *   8.85%*     2937.81 *  12.47%*     2982.57 *  14.19%*
> Hmean     16      5307.12 (   0.00%)     5304.14 *  -0.06%*
> 5610.46 *   5.72%*     5763.24 *   8.59%*     5886.66 *  10.92%*
> Hmean     32      9354.22 (   0.00%)     9738.21 *   4.11%*
> 9360.21 *   0.06%*     9699.05 *   3.69%*     9908.13 *   5.92%*
> Hmean     64      7240.35 (   0.00%)     7210.75 *  -0.41%*
> 6992.70 *  -3.42%*     7321.52 *   1.12%*     7278.78 *   0.53%*
> Hmean     128     6186.40 (   0.00%)     6314.89 *   2.08%*
> 6166.44 *  -0.32%*     6279.85 *   1.51%*     6187.85 (   0.02%)
> Hmean     256     9231.40 (   0.00%)     9469.26 *   2.58%*
> 9134.42 *  -1.05%*     9322.88 *   0.99%*     9448.61 *   2.35%*
> Hmean     512     8907.13 (   0.00%)     9130.46 *   2.51%*
> 9023.87 *   1.31%*     9276.19 *   4.14%*     9397.22 *   5.50%*
> 

Sorry, it seems the format is broken. Let me re-post the data.

 5.17-rc1: vanilla
 rc1 + chenyu: vanilla + chenyu's LLC overload patch
 rc1+chenyu+cls: vanilla + chenyu's  patch + my this patchset
 rc1+chenyu+cls-pingpong: vanilla + chenyu's patch + my this patchset - the code avoiding ping-pong
 rc1+cls: vanilla + my this patchset

tbench running on numa 0&1:
                            5.17-rc1          rc1 + chenyu          rc1+chenyu+cls     rc1+chenyu+cls-pingpong  rc1+cls
Hmean     1        320.01 (   0.00%)      318.03 *  -0.62%*      357.15 *  11.61%*      375.43 *  17.32%*      378.44 *  18.26%*
Hmean     2        643.85 (   0.00%)      637.74 *  -0.95%*      714.36 *  10.95%*      745.82 *  15.84%*      752.52 *  16.88%*
Hmean     4       1287.36 (   0.00%)     1285.20 *  -0.17%*     1431.35 *  11.18%*     1481.71 *  15.10%*     1505.62 *  16.95%*
Hmean     8       2564.60 (   0.00%)     2551.02 *  -0.53%*     2812.74 *   9.68%*     2921.51 *  13.92%*     2955.29 *  15.23%*
Hmean     16      5195.69 (   0.00%)     5163.39 *  -0.62%*     5583.28 *   7.46%*     5726.08 *  10.21%*     5814.74 *  11.91%*
Hmean     32      9769.16 (   0.00%)     9815.63 *   0.48%*    10518.35 *   7.67%*    10852.89 *  11.09%*    10872.63 *  11.30%*
Hmean     64     15952.50 (   0.00%)    15780.41 *  -1.08%*    10608.36 * -33.50%*    17503.42 *   9.72%*    17281.98 *   8.33%*
Hmean     128    13113.77 (   0.00%)    12000.12 *  -8.49%*    13095.50 *  -0.14%*    13991.90 *   6.70%*    13895.20 *   5.96%*
Hmean     256    10997.59 (   0.00%)    12229.20 *  11.20%*    11902.60 *   8.23%*    12214.29 *  11.06%*    11244.69 *   2.25%*
Hmean     512    14623.60 (   0.00%)    15863.25 *   8.48%*    14103.38 *  -3.56%*    16422.56 *  12.30%*    15526.25 *   6.17%*

tbench running on numa 0 only:
                            5.17-rc1          rc1 + chenyu          rc1+chenyu+cls     rc1+chenyu+cls-pingpong   rc1+cls
Hmean     1        324.73 (   0.00%)      330.96 *   1.92%*      358.97 *  10.54%*      376.05 *  15.80%*      378.01 *  16.41%*
Hmean     2        645.36 (   0.00%)      643.13 *  -0.35%*      710.78 *  10.14%*      744.34 *  15.34%*      754.63 *  16.93%*
Hmean     4       1302.09 (   0.00%)     1297.11 *  -0.38%*     1425.22 *   9.46%*     1484.92 *  14.04%*     1507.54 *  15.78%*
Hmean     8       2612.03 (   0.00%)     2623.60 *   0.44%*     2843.15 *   8.85%*     2937.81 *  12.47%*     2982.57 *  14.19%*
Hmean     16      5307.12 (   0.00%)     5304.14 *  -0.06%*     5610.46 *   5.72%*     5763.24 *   8.59%*     5886.66 *  10.92%*
Hmean     32      9354.22 (   0.00%)     9738.21 *   4.11%*     9360.21 *   0.06%*     9699.05 *   3.69%*     9908.13 *   5.92%*
Hmean     64      7240.35 (   0.00%)     7210.75 *  -0.41%*     6992.70 *  -3.42%*     7321.52 *   1.12%*     7278.78 *   0.53%*
Hmean     128     6186.40 (   0.00%)     6314.89 *   2.08%*     6166.44 *  -0.32%*     6279.85 *   1.51%*     6187.85 (   0.02%)
Hmean     256     9231.40 (   0.00%)     9469.26 *   2.58%*     9134.42 *  -1.05%*     9322.88 *   0.99%*     9448.61 *   2.35%*
Hmean     512     8907.13 (   0.00%)     9130.46 *   2.51%*     9023.87 *   1.31%*     9276.19 *   4.14%*     9397.22 *   5.50%*

> like rc1+cls, in some
> cases(256, 512 threads on numa0&1), it is even much better.
> 
> Thanks
> Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-16  9:19                         ` Song Bao Hua (Barry Song)
@ 2022-02-16 10:00                           ` Yicong Yang
  2022-02-17 18:00                             ` Tim Chen
  0 siblings, 1 reply; 27+ messages in thread
From: Yicong Yang @ 2022-02-16 10:00 UTC (permalink / raw)
  To: Song Bao Hua (Barry Song), Barry Song, Gautham R. Shenoy
  Cc: yangyicong, Srikar Dronamraju, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Tim Chen, LKML, LAK,
	Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Daniel Bristot de Oliveira, Zengtao (B),
	Jonathan Cameron, ego, Linuxarm, Guodong Xu, Chen Yu

On 2022/2/16 17:19, Song Bao Hua (Barry Song) wrote:
> 
> 
>> -----Original Message-----
>> From: Barry Song [mailto:21cnbao@gmail.com]
>> Sent: Wednesday, February 16, 2022 10:13 PM
>> To: Gautham R. Shenoy <gautham.shenoy@amd.com>
>> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>; yangyicong
>> <yangyicong@huawei.com>; Peter Zijlstra <peterz@infradead.org>; Ingo Molnar
>> <mingo@redhat.com>; Juri Lelli <juri.lelli@redhat.com>; Vincent Guittot
>> <vincent.guittot@linaro.org>; Tim Chen <tim.c.chen@linux.intel.com>; LKML
>> <linux-kernel@vger.kernel.org>; LAK <linux-arm-kernel@lists.infradead.org>;
>> Dietmar Eggemann <dietmar.eggemann@arm.com>; Steven Rostedt
>> <rostedt@goodmis.org>; Ben Segall <bsegall@google.com>; Daniel Bristot de
>> Oliveira <bristot@redhat.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
>> Jonathan Cameron <jonathan.cameron@huawei.com>; ego@linux.vnet.ibm.com;
>> Linuxarm <linuxarm@huawei.com>; Song Bao Hua (Barry Song)
>> <song.bao.hua@hisilicon.com>; Guodong Xu <guodong.xu@linaro.org>
>> Subject: Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in
>> wake-up path
>>
>> On Tue, Feb 8, 2022 at 6:42 PM Barry Song <21cnbao@gmail.com> wrote:
>>>
>>> On Tue, Feb 8, 2022 at 4:14 AM Gautham R. Shenoy <gautham.shenoy@amd.com>
>> wrote:
>>>>
>>>>
>>>> On Fri, Feb 04, 2022 at 11:28:25PM +1300, Barry Song wrote:
>>>>
>>>>>> We already figured out that there are no idle CPUs in this cluster.
>> So dont
>>>>>> we gain performance by picking a idle CPU/core in the neighbouring cluster.
>>>>>> If there are no idle CPU/core in the neighbouring cluster, then it does
>> make
>>>>>> sense to fallback on the current cluster.
>>>>>
>>>>> What you suggested is exactly the approach we have tried at the first
>> beginning
>>>>> during debugging. but we didn't gain performance according to benchmark,
>> we
>>>>> were actually losing. that is why we added this line to stop ping-pong:
>>>>>          /* Don't ping-pong tasks in and out cluster frequently */
>>>>>          if (cpus_share_resources(target, prev_cpu))
>>>>>             return target;
>>>>>
>>>>> If we delete this, we are seeing a big loss of tbench while system
>>>>> load is medium
>>>>> and above.
>>>>
>>>> Thanks for clarifying this Barry. Indeed, if the workload is sensitive
>>>> to data ping-ponging across L2 clusters, this heuristic makes sense. I
>>>> was thinking of workloads that require lower tail latency, in which
>>>> case exploring the larger LLC would have made more sense, assuming
>>>> that the larger LLC has an idle core/CPU.
>>>>
>>>> In the absence of any hints from the workload, like something that
>>>> Peter had previous suggested
>>>>
>> (https://lore.kernel.org/lkml/YVwnsrZWrnWHaoqN@hirez.programming.kicks-ass
>> .net/),
>>>> optimizing for cache-access seems to be the right thing to do.
>>>
>>> Thanks, gautham.
>>>
>>> Yep. Peter mentioned some hints like SCHED_BATCH and SCHED_IDLE.
>>> To me, the case we are discussing seems to be more complicated than
>>> applying some scheduling policy on separate tasks by SCHED_BATCH
>>> or IDLE.
>>>
>>> For example, in case we have a process, and this process has 20 threads.
>>> thread0-9 might care about cache-coherence latency and want to avoid
>>> ping-ponging, and thread10-thread19 might want to have tail-latency
>>> as small as possible. So we need some way to tell kernel, "hey, bro, please
>>> try to keep thread0-9 still as ping-ponging will hurt them while trying your
>>> best to find idle cpu in a wider range for thread10-19". But it seems
>>> SCHED_XXX as a scheduler policy hint can't tell kernel how to organize tasks
>>> into groups, and is also incapable of telling kernel different groups have
>>> different needs.
>>>
>>> So it seems we want some special cgroups to organize tasks and we can apply
>>> some special hints on each different group. for example, putting thread0-9
>>> in a cgroup and thread10-19 in another, then:
>>> 1. apply "COMMUNCATION-SENSITVE" on the 1st group
>>> 2. apply "TAIL-LATENCY-SENTIVE" on the 2nd one.
>>> I am not quite sure how to do this and if this can find its way into
>>> the mainline.
>>>
>>> On the other hand, for this particular patch, the most controversial
>>> part is those
>>> two lines to avoid ping-ponging, and I am seeing dropping this can hurt workload
>>> like tbench only when system load is high, so I wonder if the approach[1]
>> from
>>> Chen Yu and Tim can somehow resolve the problem alternatively, thus we can
>>> avoid the controversial part.
>>> since their patch can also shrink the scanning range while llc load is high.
>>>
>>> [1]
>> https://lore.kernel.org/lkml/20220207034013.599214-1-yu.c.chen@intel.com/
>>
>> Yicong's testing shows the patch from Chen Yu and Tim can somehow resolve the
>> problem and make sure there is no performance regression for tbench
>> while load is
>> high after we remove the code to avoid ping-pong:
>>
>> 5.17-rc1: vanilla
>> rc1 + chenyu: vanilla + chenyu's LLC overload patch
>> rc1+chenyu+cls: vanilla + chenyu's  patch + my this patchset
>> rc1+chenyu+cls-pingpong: vanilla + chenyu's patch + my this patchset -
>> the code avoiding ping-pong
>> rc1+cls: vanilla + my this patchset
>>
>> tbench running on numa 0 &1:
>>                             5.17-rc1          rc1 + chenyu
>> rc1+chenyu+cls     rc1+chenyu+cls-pingpong  rc1+cls
>> Hmean     1        320.01 (   0.00%)      318.03 *  -0.62%*
>> 357.15 *  11.61%*      375.43 *  17.32%*      378.44 *  18.26%*
>> Hmean     2        643.85 (   0.00%)      637.74 *  -0.95%*
>> 714.36 *  10.95%*      745.82 *  15.84%*      752.52 *  16.88%*
>> Hmean     4       1287.36 (   0.00%)     1285.20 *  -0.17%*
>> 1431.35 *  11.18%*     1481.71 *  15.10%*     1505.62 *  16.95%*
>> Hmean     8       2564.60 (   0.00%)     2551.02 *  -0.53%*
>> 2812.74 *   9.68%*     2921.51 *  13.92%*     2955.29 *  15.23%*
>> Hmean     16      5195.69 (   0.00%)     5163.39 *  -0.62%*
>> 5583.28 *   7.46%*     5726.08 *  10.21%*     5814.74 *  11.91%*
>> Hmean     32      9769.16 (   0.00%)     9815.63 *   0.48%*
>> 10518.35 *   7.67%*    10852.89 *  11.09%*    10872.63 *  11.30%*
>> Hmean     64     15952.50 (   0.00%)    15780.41 *  -1.08%*
>> 10608.36 * -33.50%*    17503.42 *   9.72%*    17281.98 *   8.33%*
>> Hmean     128    13113.77 (   0.00%)    12000.12 *  -8.49%*
>> 13095.50 *  -0.14%*    13991.90 *   6.70%*    13895.20 *   5.96%*
>> Hmean     256    10997.59 (   0.00%)    12229.20 *  11.20%*
>> 11902.60 *   8.23%*    12214.29 *  11.06%*    11244.69 *   2.25%*
>> Hmean     512    14623.60 (   0.00%)    15863.25 *   8.48%*
>> 14103.38 *  -3.56%*    16422.56 *  12.30%*    15526.25 *   6.17%*
>>
>> tbench running on numa 0 only:
>>
>>                             5.17-rc1          rc1 + chenyu
>> rc1+chenyu+cls     rc1+chenyu+cls-pingpong   rc1+cls
>> Hmean     1        324.73 (   0.00%)      330.96 *   1.92%*
>> 358.97 *  10.54%*      376.05 *  15.80%*      378.01 *  16.41%*
>> Hmean     2        645.36 (   0.00%)      643.13 *  -0.35%*
>> 710.78 *  10.14%*      744.34 *  15.34%*      754.63 *  16.93%*
>> Hmean     4       1302.09 (   0.00%)     1297.11 *  -0.38%*
>> 1425.22 *   9.46%*     1484.92 *  14.04%*     1507.54 *  15.78%*
>> Hmean     8       2612.03 (   0.00%)     2623.60 *   0.44%*
>> 2843.15 *   8.85%*     2937.81 *  12.47%*     2982.57 *  14.19%*
>> Hmean     16      5307.12 (   0.00%)     5304.14 *  -0.06%*
>> 5610.46 *   5.72%*     5763.24 *   8.59%*     5886.66 *  10.92%*
>> Hmean     32      9354.22 (   0.00%)     9738.21 *   4.11%*
>> 9360.21 *   0.06%*     9699.05 *   3.69%*     9908.13 *   5.92%*
>> Hmean     64      7240.35 (   0.00%)     7210.75 *  -0.41%*
>> 6992.70 *  -3.42%*     7321.52 *   1.12%*     7278.78 *   0.53%*
>> Hmean     128     6186.40 (   0.00%)     6314.89 *   2.08%*
>> 6166.44 *  -0.32%*     6279.85 *   1.51%*     6187.85 (   0.02%)
>> Hmean     256     9231.40 (   0.00%)     9469.26 *   2.58%*
>> 9134.42 *  -1.05%*     9322.88 *   0.99%*     9448.61 *   2.35%*
>> Hmean     512     8907.13 (   0.00%)     9130.46 *   2.51%*
>> 9023.87 *   1.31%*     9276.19 *   4.14%*     9397.22 *   5.50%*
>>
> 
> Sorry, it seems the format is broken. Let me re-post the data.
> 
>  5.17-rc1: vanilla
>  rc1 + chenyu: vanilla + chenyu's LLC overload patch
>  rc1+chenyu+cls: vanilla + chenyu's  patch + my this patchset
>  rc1+chenyu+cls-pingpong: vanilla + chenyu's patch + my this patchset - the code avoiding ping-pong
>  rc1+cls: vanilla + my this patchset
> 
> tbench running on numa 0&1:
>                             5.17-rc1          rc1 + chenyu          rc1+chenyu+cls     rc1+chenyu+cls-pingpong  rc1+cls
> Hmean     1        320.01 (   0.00%)      318.03 *  -0.62%*      357.15 *  11.61%*      375.43 *  17.32%*      378.44 *  18.26%*
> Hmean     2        643.85 (   0.00%)      637.74 *  -0.95%*      714.36 *  10.95%*      745.82 *  15.84%*      752.52 *  16.88%*
> Hmean     4       1287.36 (   0.00%)     1285.20 *  -0.17%*     1431.35 *  11.18%*     1481.71 *  15.10%*     1505.62 *  16.95%*
> Hmean     8       2564.60 (   0.00%)     2551.02 *  -0.53%*     2812.74 *   9.68%*     2921.51 *  13.92%*     2955.29 *  15.23%*
> Hmean     16      5195.69 (   0.00%)     5163.39 *  -0.62%*     5583.28 *   7.46%*     5726.08 *  10.21%*     5814.74 *  11.91%*
> Hmean     32      9769.16 (   0.00%)     9815.63 *   0.48%*    10518.35 *   7.67%*    10852.89 *  11.09%*    10872.63 *  11.30%*
> Hmean     64     15952.50 (   0.00%)    15780.41 *  -1.08%*    10608.36 * -33.50%*    17503.42 *   9.72%*    17281.98 *   8.33%*
> Hmean     128    13113.77 (   0.00%)    12000.12 *  -8.49%*    13095.50 *  -0.14%*    13991.90 *   6.70%*    13895.20 *   5.96%*
> Hmean     256    10997.59 (   0.00%)    12229.20 *  11.20%*    11902.60 *   8.23%*    12214.29 *  11.06%*    11244.69 *   2.25%*
> Hmean     512    14623.60 (   0.00%)    15863.25 *   8.48%*    14103.38 *  -3.56%*    16422.56 *  12.30%*    15526.25 *   6.17%*
> 

Yes I think it'll also benefit for the cluster's conditon.

But 128 threads seems like a weired point that Chen's patch on 5.17-rc1 (without this series) causes degradation,
which in Chen's tbench test it doesn't cause that much when the 2 * cpu number == threads[*]:

case            	load    	baseline(std%)	compare%( std%)
loopback        	thread-224	 1.00 (  0.17)	 +2.30 (  0.10)

[*] https://lore.kernel.org/lkml/20220207034013.599214-1-yu.c.chen@intel.com/

> tbench running on numa 0 only:
>                             5.17-rc1          rc1 + chenyu          rc1+chenyu+cls     rc1+chenyu+cls-pingpong   rc1+cls
> Hmean     1        324.73 (   0.00%)      330.96 *   1.92%*      358.97 *  10.54%*      376.05 *  15.80%*      378.01 *  16.41%*
> Hmean     2        645.36 (   0.00%)      643.13 *  -0.35%*      710.78 *  10.14%*      744.34 *  15.34%*      754.63 *  16.93%*
> Hmean     4       1302.09 (   0.00%)     1297.11 *  -0.38%*     1425.22 *   9.46%*     1484.92 *  14.04%*     1507.54 *  15.78%*
> Hmean     8       2612.03 (   0.00%)     2623.60 *   0.44%*     2843.15 *   8.85%*     2937.81 *  12.47%*     2982.57 *  14.19%*
> Hmean     16      5307.12 (   0.00%)     5304.14 *  -0.06%*     5610.46 *   5.72%*     5763.24 *   8.59%*     5886.66 *  10.92%*
> Hmean     32      9354.22 (   0.00%)     9738.21 *   4.11%*     9360.21 *   0.06%*     9699.05 *   3.69%*     9908.13 *   5.92%*
> Hmean     64      7240.35 (   0.00%)     7210.75 *  -0.41%*     6992.70 *  -3.42%*     7321.52 *   1.12%*     7278.78 *   0.53%*
> Hmean     128     6186.40 (   0.00%)     6314.89 *   2.08%*     6166.44 *  -0.32%*     6279.85 *   1.51%*     6187.85 (   0.02%)
> Hmean     256     9231.40 (   0.00%)     9469.26 *   2.58%*     9134.42 *  -1.05%*     9322.88 *   0.99%*     9448.61 *   2.35%*
> Hmean     512     8907.13 (   0.00%)     9130.46 *   2.51%*     9023.87 *   1.31%*     9276.19 *   4.14%*     9397.22 *   5.50%*
> 
>> like rc1+cls, in some
>> cases(256, 512 threads on numa0&1), it is even much better.
>>
>> Thanks
>> Barry

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

* Re: [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path
  2022-02-16 10:00                           ` Yicong Yang
@ 2022-02-17 18:00                             ` Tim Chen
  0 siblings, 0 replies; 27+ messages in thread
From: Tim Chen @ 2022-02-17 18:00 UTC (permalink / raw)
  To: Yicong Yang, Song Bao Hua (Barry Song), Barry Song, Gautham R. Shenoy
  Cc: yangyicong, Srikar Dronamraju, Peter Zijlstra, Ingo Molnar,
	Juri Lelli, Vincent Guittot, LKML, LAK, Dietmar Eggemann,
	Steven Rostedt, Ben Segall, Daniel Bristot de Oliveira,
	Zengtao (B),
	Jonathan Cameron, ego, Linuxarm, Guodong Xu, Chen Yu

On Wed, 2022-02-16 at 18:00 +0800, Yicong Yang wrote:
> On 2022/2/16 17:19, Song Bao Hua (Barry Song) wrote:
> > 
> > tbench running on numa 0&1:
> >                             5.17-rc1          rc1 + chenyu          rc1+chenyu+cls     rc1+chenyu+cls-pingpong  rc1+cls
> > Hmean     1        320.01 (   0.00%)      318.03 *  -0.62%*      357.15 *  11.61%*      375.43 *  17.32%*      378.44 *  18.26%*
> > Hmean     2        643.85 (   0.00%)      637.74 *  -0.95%*      714.36 *  10.95%*      745.82 *  15.84%*      752.52 *  16.88%*
> > Hmean     4       1287.36 (   0.00%)     1285.20 *  -0.17%*     1431.35 *  11.18%*     1481.71 *  15.10%*     1505.62 *  16.95%*
> > Hmean     8       2564.60 (   0.00%)     2551.02 *  -0.53%*     2812.74 *   9.68%*     2921.51 *  13.92%*     2955.29 *  15.23%*
> > Hmean     16      5195.69 (   0.00%)     5163.39 *  -0.62%*     5583.28 *   7.46%*     5726.08 *  10.21%*     5814.74 *  11.91%*
> > Hmean     32      9769.16 (   0.00%)     9815.63 *   0.48%*    10518.35 *   7.67%*    10852.89 *  11.09%*    10872.63 *  11.30%*
> > Hmean     64     15952.50 (   0.00%)    15780.41 *  -1.08%*    10608.36 * -33.50%*    17503.42 *   9.72%*    17281.98 *   8.33%*
> > Hmean     128    13113.77 (   0.00%)    12000.12 *  -8.49%*    13095.50 *  -0.14%*    13991.90 *   6.70%*    13895.20 *   5.96%*
> > Hmean     256    10997.59 (   0.00%)    12229.20 *  11.20%*    11902.60 *   8.23%*    12214.29 *  11.06%*    11244.69 *   2.25%*
> > Hmean     512    14623.60 (   0.00%)    15863.25 *   8.48%*    14103.38 *  -3.56%*    16422.56 *  12.30%*    15526.25 *   6.17%*
> > 
> 
> Yes I think it'll also benefit for the cluster's conditon.
> 
> But 128 threads seems like a weired point that Chen's patch on 5.17-rc1 (without this series) causes degradation,
> which in Chen's tbench test it doesn't cause that much when the 2 * cpu number == threads[*]:
> 

From the data, it seems like Chen Yu's patch benefits the overloaded condition (as expected) while
the cluster scheduling has benefit most at the low end (also expected).  It is nice that
by combining these two approaches we can get the most benefit.
 
Chen Yu's patch has a hard transition to stop search for idle CPU at about 85% utilization.
So we may be hitting that knee and we may benefit from not stopping search completely
but reducing number of CPUs searched, as Peter pointed out.

Tim 

> case            	load    	baseline(std%)	compare%( std%)
> loopback        	thread-224	 1.00 (  0.17)	 +2.30 (  0.10)
> 
> [*] https://lore.kernel.org/lkml/20220207034013.599214-1-yu.c.chen@intel.com/
> 
> > tbench running on numa 0 only:
> >                             5.17-rc1          rc1 + chenyu          rc1+chenyu+cls     rc1+chenyu+cls-pingpong   rc1+cls
> > Hmean     1        324.73 (   0.00%)      330.96 *   1.92%*      358.97 *  10.54%*      376.05 *  15.80%*      378.01 *  16.41%*
> > Hmean     2        645.36 (   0.00%)      643.13 *  -0.35%*      710.78 *  10.14%*      744.34 *  15.34%*      754.63 *  16.93%*
> > Hmean     4       1302.09 (   0.00%)     1297.11 *  -0.38%*     1425.22 *   9.46%*     1484.92 *  14.04%*     1507.54 *  15.78%*
> > Hmean     8       2612.03 (   0.00%)     2623.60 *   0.44%*     2843.15 *   8.85%*     2937.81 *  12.47%*     2982.57 *  14.19%*
> > Hmean     16      5307.12 (   0.00%)     5304.14 *  -0.06%*     5610.46 *   5.72%*     5763.24 *   8.59%*     5886.66 *  10.92%*
> > Hmean     32      9354.22 (   0.00%)     9738.21 *   4.11%*     9360.21 *   0.06%*     9699.05 *   3.69%*     9908.13 *   5.92%*
> > Hmean     64      7240.35 (   0.00%)     7210.75 *  -0.41%*     6992.70 *  -3.42%*     7321.52 *   1.12%*     7278.78 *   0.53%*
> > Hmean     128     6186.40 (   0.00%)     6314.89 *   2.08%*     6166.44 *  -0.32%*     6279.85 *   1.51%*     6187.85 (   0.02%)
> > Hmean     256     9231.40 (   0.00%)     9469.26 *   2.58%*     9134.42 *  -1.05%*     9322.88 *   0.99%*     9448.61 *   2.35%*
> > Hmean     512     8907.13 (   0.00%)     9130.46 *   2.51%*     9023.87 *   1.31%*     9276.19 *   4.14%*     9397.22 *   5.50%*
> > 
> > > like rc1+cls, in some
> > > cases(256, 512 threads on numa0&1), it is even much better.
> > > 
> > > Thanks
> > > Barry


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

end of thread, other threads:[~2022-02-17 18:00 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26  8:09 [PATCH v2 0/2] sched/fair: Wake task within the cluster when possible Yicong Yang
2022-01-26  8:09 ` [PATCH v2 1/2] sched: Add per_cpu cluster domain info and cpus_share_resources API Yicong Yang
2022-01-27 15:26   ` Gautham R. Shenoy
2022-01-26  8:09 ` [PATCH v2 2/2] sched/fair: Scan cluster before scanning LLC in wake-up path Yicong Yang
2022-01-27  1:14   ` Tim Chen
2022-01-27  2:02     ` Yicong Yang
2022-01-27  2:30       ` Tim Chen
2022-01-27  2:36         ` Tim Chen
2022-01-27  3:05           ` Yicong Yang
2022-01-27 15:41   ` Gautham R. Shenoy
2022-01-27 20:21     ` Barry Song
2022-01-28  7:13       ` Srikar Dronamraju
2022-01-27 18:40         ` Barry Song
2022-02-01  9:38           ` Srikar Dronamraju
2022-02-01 20:20             ` Barry Song
2022-02-04  7:33               ` Srikar Dronamraju
2022-02-04 10:28                 ` Barry Song
2022-02-04 10:49                   ` Barry Song
2022-02-04 17:41                     ` Tim Chen
2022-02-05 17:16                       ` Chen Yu
2022-02-06  0:26                         ` Barry Song
2022-02-07 15:14                   ` Gautham R. Shenoy
2022-02-08  5:42                     ` Barry Song
2022-02-16  9:12                       ` Barry Song
2022-02-16  9:19                         ` Song Bao Hua (Barry Song)
2022-02-16 10:00                           ` Yicong Yang
2022-02-17 18:00                             ` Tim Chen

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