[PATCH] Add busy loop polling for idle SMT

[PATCH] Add busy loop polling for idle SMT

[Peng Wang]

Now we have cpu_idle_force_poll which uses cpu_relax() waiting for
an arriving IPI, while sometimes busy loop on idle cpu is also
useful to provide consistent pipeline interference for hardware SMT.

When hardware SMT is enabled, the switching between idle and
busy state of one cpu will cause performance fluctuation of
other sibling cpus on the same core.

In pay-for-execution-time scenario, cloud service providers prefer
stable performance data to set stabel price for same workload.
Different execution time of the same workload caused by different
idle or busy state of sibling SMT cpus will make different bills, which
is confused for customers.

Since there is no dynamic CPU time scaling based on SMT pipeline interference,
to coordinate sibling SMT noise no matter whether they are idle or not,
busy loop in idle state can provide approximately consistent pipeline interference.

For example, a workload computing tangent and cotangent will finish in 9071ms when
sibling SMT cpus are idle, and 13299ms when sibling SMT cpus are computiing other workload.
This generate 32% performance fluctuation.

SMT idle polling makes things slower, but we can set bigger cpu quota to make up
a deficiency. This also increase power consumption by 2.2%, which is acceptable.

There may be some other possible solutions, while each has its own problem:
a) disbale hardware SMT, which means half of SMT is unused and more hardware cost.
b) busy loop in a userspace thread, but the cpu usage is confusing.

We propose this patch to discuss the performance fluctuation problem related to SMT
pipeline interference, and any comments are welcome.

Peng Wang (1):
  sched/idle: support busy loop polling on idle SMT cpus

 arch/Kconfig         | 10 ++++++
 kernel/sched/core.c  | 39 ++++++++++++++++++++++
 kernel/sched/idle.c  | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 kernel/sched/sched.h | 18 +++++++++++
 4 files changed, 158 insertions(+)

-- 
2.9.5

[PATCH] sched/idle: support busy loop polling on idle SMT cpus

[Peng Wang]

When hardware SMT is enabled, the switching between one cpu's idle and busy
state will cause performance fluctuation of sibling cpus on the same core.

Consistent noise is needed when requiring stable performance on one SMT cpu
regardless of whether the sibling cpus on the same core is idle.

The orginal cpu_idle_force_poll uses cpu_relax() waiting an arriving IPI,
while this smt_idle_force_poll uses busy loop to provide consistent SMT
pipeline interference.

Use a CGroup config to enable busy loop polling for specific tasks.

Interface: cgroup/cpu.smt_idle_poll
 0: default behavior
 1: SMT idle sibling cpus will do busy loop polling

Co-developed-by: Huaixin Chang <changhuaixin@linux.alibaba.com>
Signed-off-by: Huaixin Chang <changhuaixin@linux.alibaba.com>
Co-developed-by: Shanpei Chen <shanpeic@linux.alibaba.com>
Signed-off-by: Shanpei Chen <shanpeic@linux.alibaba.com>
Signed-off-by: Peng Wang <rocking@linux.alibaba.com>
---
 arch/Kconfig         | 10 ++++++
 kernel/sched/core.c  | 39 ++++++++++++++++++++++
 kernel/sched/idle.c  | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 kernel/sched/sched.h | 18 +++++++++++
 4 files changed, 158 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index 26b8ed1..805c3b9 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -233,6 +233,16 @@ config GENERIC_SMP_IDLE_THREAD
 config GENERIC_IDLE_POLL_SETUP
 	bool
 
+config SMT_IDLE_POLL
+	bool "SMT idle poll"
+	depends on SCHED_SMT
+	help
+	  When hardware SMT is enabled, the switching between idle or busy
+	  state of one cpu will cause performance fluctuation of other
+	  cpus on the same core. This provides a way to support consistent
+	  SMT noise.
+	  If in doubt, say "N".
+
 config ARCH_HAS_FORTIFY_SOURCE
 	bool
 	help
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3c9b0fd..ef478d6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4845,6 +4845,7 @@ static struct rq *finish_task_switch(struct task_struct *prev)
 	prev_state = READ_ONCE(prev->__state);
 	vtime_task_switch(prev);
 	perf_event_task_sched_in(prev, current);
+	smt_idle_poll_switch(rq);
 	finish_task(prev);
 	tick_nohz_task_switch();
 	finish_lock_switch(rq);
@@ -10189,6 +10190,30 @@ static u64 cpu_shares_read_u64(struct cgroup_subsys_state *css,
 	return (u64) scale_load_down(tg->shares);
 }
 
+#ifdef CONFIG_SMT_IDLE_POLL
+static atomic_t smt_idle_poll_count;
+static int smt_idle_poll_write_u64(struct cgroup_subsys_state *css,
+				struct cftype *cftype, u64 smt_idle_poll)
+{
+	if (smt_idle_poll == 1) {
+		if (atomic_inc_return(&smt_idle_poll_count) == 1)
+			static_branch_enable(&__smt_idle_poll_enabled);
+	} else if (smt_idle_poll == 0) {
+		if (atomic_dec_return(&smt_idle_poll_count) == 0)
+			static_branch_disable(&__smt_idle_poll_enabled);
+	} else
+		return -EINVAL;
+	css_tg(css)->need_smt_idle_poll = smt_idle_poll;
+	return 0;
+}
+
+static u64 smt_idle_poll_read_u64(struct cgroup_subsys_state *css,
+			       struct cftype *cft)
+{
+	return (u64)css_tg(css)->need_smt_idle_poll;
+}
+#endif
+
 #ifdef CONFIG_CFS_BANDWIDTH
 static DEFINE_MUTEX(cfs_constraints_mutex);
 
@@ -10565,6 +10590,13 @@ static struct cftype cpu_legacy_files[] = {
 		.read_s64 = cpu_idle_read_s64,
 		.write_s64 = cpu_idle_write_s64,
 	},
+#ifdef CONFIG_SMT_IDLE_POLL
+	{
+		.name = "smt_idle_poll",
+		.read_u64 = smt_idle_poll_read_u64,
+		.write_u64 = smt_idle_poll_write_u64,
+	},
+#endif
 #endif
 #ifdef CONFIG_CFS_BANDWIDTH
 	{
@@ -10782,6 +10814,13 @@ static struct cftype cpu_files[] = {
 		.read_s64 = cpu_idle_read_s64,
 		.write_s64 = cpu_idle_write_s64,
 	},
+#ifdef CONFIG_SMT_IDLE_POLL
+	{
+		.name = "smt_idle_poll",
+		.read_u64 = smt_idle_poll_read_u64,
+		.write_u64 = smt_idle_poll_write_u64,
+	},
+#endif
 #endif
 #ifdef CONFIG_CFS_BANDWIDTH
 	{
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index d17b0a5..bfb8f56 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -52,6 +52,90 @@ static int __init cpu_idle_nopoll_setup(char *__unused)
 __setup("hlt", cpu_idle_nopoll_setup);
 #endif
 
+#ifdef CONFIG_SMT_IDLE_POLL
+DEFINE_STATIC_KEY_FALSE(__smt_idle_poll_enabled);
+
+void smt_idle_poll_switch(struct rq *rq)
+{
+	struct rq *smt_rq;
+	struct task_group *tg;
+	int smt, cpu;
+
+	if (!static_branch_unlikely(&__smt_idle_poll_enabled))
+		return;
+
+	if (rq->idle == current) {
+		rq->need_smt_idle_poll = false;
+		return;
+	}
+
+	rcu_read_lock();
+	tg = container_of(task_css_check(current, cpu_cgrp_id, true),
+		struct task_group, css);
+	rq->need_smt_idle_poll = tg->need_smt_idle_poll;
+	rcu_read_unlock();
+
+	if (!rq->need_smt_idle_poll)
+		return;
+
+	cpu = rq->cpu;
+	for_each_cpu(smt, cpu_smt_mask(cpu)) {
+		if (cpu == smt || !cpu_online(smt))
+			continue;
+		smt_rq = cpu_rq(smt);
+		if (smt_rq->idle == smt_rq->curr && !smt_rq->in_smt_idle_poll)
+			smp_send_reschedule(smt);
+	}
+}
+
+/**
+ * need_smt_idle_poll - Decide whether to continue SMT idle polling.
+ */
+static bool need_smt_idle_poll(void)
+{
+	int cpu, ht;
+	struct rq *rq;
+
+	if (!static_branch_unlikely(&__smt_idle_poll_enabled))
+		return false;
+
+	if (tif_need_resched())
+		return false;
+
+	cpu = smp_processor_id();
+	for_each_cpu(ht, cpu_smt_mask(cpu)) {
+		if (cpu == ht || !cpu_online(ht))
+			continue;
+
+		rq = cpu_rq(ht);
+		if (rq->need_smt_idle_poll)
+			return true;
+	}
+
+	return false;
+}
+
+static noinline int __cpuidle smt_idle_poll(void)
+{
+	trace_cpu_idle(0, smp_processor_id());
+	stop_critical_timings();
+	rcu_idle_enter();
+	local_irq_enable();
+
+	this_rq()->in_smt_idle_poll = true;
+	while (need_smt_idle_poll())
+		;
+	this_rq()->in_smt_idle_poll = false;
+
+	rcu_idle_exit();
+	start_critical_timings();
+	trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id());
+
+	return 1;
+}
+
+#endif
+
 static noinline int __cpuidle cpu_idle_poll(void)
 {
 	trace_cpu_idle(0, smp_processor_id());
@@ -293,6 +377,13 @@ static void do_idle(void)
 		arch_cpu_idle_enter();
 		rcu_nocb_flush_deferred_wakeup();
 
+#ifdef CONFIG_SMT_IDLE_POLL
+		if (need_smt_idle_poll()) {
+			tick_nohz_idle_restart_tick();
+			smt_idle_poll();
+		}
+#endif
+
 		/*
 		 * In poll mode we reenable interrupts and spin. Also if we
 		 * detected in the wakeup from idle path that the tick
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 0e66749..58b3af3 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -401,6 +401,10 @@ struct task_group {
 	/* A positive value indicates that this is a SCHED_IDLE group. */
 	int			idle;
 
+#ifdef CONFIG_SMT_IDLE_POLL
+	bool			need_smt_idle_poll;
+#endif
+
 #ifdef	CONFIG_SMP
 	/*
 	 * load_avg can be heavily contended at clock tick time, so put
@@ -1114,6 +1118,11 @@ struct rq {
 	unsigned char		core_forceidle;
 	unsigned int		core_forceidle_seq;
 #endif
+
+#ifdef CONFIG_SMT_IDLE_POLL
+	bool			need_smt_idle_poll;
+	bool			in_smt_idle_poll;
+#endif
 };
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
@@ -1302,6 +1311,15 @@ static inline bool sched_group_cookie_match(struct rq *rq,
 }
 #endif /* CONFIG_SCHED_CORE */
 
+#ifdef CONFIG_SMT_IDLE_POLL
+DECLARE_STATIC_KEY_FALSE(__smt_idle_poll_enabled);
+extern void smt_idle_poll_switch(struct rq *rq);
+#else
+static inline void smt_idle_poll_switch(struct rq *rq)
+{
+}
+#endif
+
 static inline void lockdep_assert_rq_held(struct rq *rq)
 {
 	lockdep_assert_held(__rq_lockp(rq));
-- 
2.9.5

Re: [PATCH] sched/idle: support busy loop polling on idle SMT cpus

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4565 bytes --]

Hi Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on tip/master linux/master linus/master v5.16-rc1 next-20211116]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peng-Wang/sched-idle-support-busy-loop-polling-on-idle-SMT-cpus/20211116-195600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8ea9183db4ad8afbcb7089a77c23eaf965b0cacd
config: i386-randconfig-r034-20211116 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project fbe72e41b99dc7994daac300d208a955be3e4a0a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/11c6a63e5c0f496b43058351f1e2a488ee27a5ec
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peng-Wang/sched-idle-support-busy-loop-polling-on-idle-SMT-cpus/20211116-195600
        git checkout 11c6a63e5c0f496b43058351f1e2a488ee27a5ec
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> kernel/sched/idle.c:73:44: error: use of undeclared identifier 'cpu_cgrp_id'
           tg = container_of(task_css_check(current, cpu_cgrp_id, true),
                                                     ^
>> kernel/sched/idle.c:73:44: error: use of undeclared identifier 'cpu_cgrp_id'
>> kernel/sched/idle.c:73:44: error: use of undeclared identifier 'cpu_cgrp_id'
>> kernel/sched/idle.c:73:7: error: offsetof of incomplete type 'struct task_group'
           tg = container_of(task_css_check(current, cpu_cgrp_id, true),
                ^
   include/linux/kernel.h:498:21: note: expanded from macro 'container_of'
           ((type *)(__mptr - offsetof(type, member))); })
                              ^        ~~~~
   include/linux/stddef.h:17:32: note: expanded from macro 'offsetof'
   #define offsetof(TYPE, MEMBER)  __compiler_offsetof(TYPE, MEMBER)
                                   ^                   ~~~~
   include/linux/compiler_types.h:140:35: note: expanded from macro '__compiler_offsetof'
   #define __compiler_offsetof(a, b)       __builtin_offsetof(a, b)
                                           ^                  ~
   include/linux/sched.h:69:8: note: forward declaration of 'struct task_group'
   struct task_group;
          ^
>> kernel/sched/idle.c:73:5: error: assigning to 'struct task_group *' from incompatible type 'void'
           tg = container_of(task_css_check(current, cpu_cgrp_id, true),
              ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> kernel/sched/idle.c:75:29: error: incomplete definition of type 'struct task_group'
           rq->need_smt_idle_poll = tg->need_smt_idle_poll;
                                    ~~^
   include/linux/sched.h:69:8: note: forward declaration of 'struct task_group'
   struct task_group;
          ^
   6 errors generated.


vim +/cpu_cgrp_id +73 kernel/sched/idle.c

    57	
    58	void smt_idle_poll_switch(struct rq *rq)
    59	{
    60		struct rq *smt_rq;
    61		struct task_group *tg;
    62		int smt, cpu;
    63	
    64		if (!static_branch_unlikely(&__smt_idle_poll_enabled))
    65			return;
    66	
    67		if (rq->idle == current) {
    68			rq->need_smt_idle_poll = false;
    69			return;
    70		}
    71	
    72		rcu_read_lock();
  > 73		tg = container_of(task_css_check(current, cpu_cgrp_id, true),
    74			struct task_group, css);
  > 75		rq->need_smt_idle_poll = tg->need_smt_idle_poll;
    76		rcu_read_unlock();
    77	
    78		if (!rq->need_smt_idle_poll)
    79			return;
    80	
    81		cpu = rq->cpu;
    82		for_each_cpu(smt, cpu_smt_mask(cpu)) {
    83			if (cpu == smt || !cpu_online(smt))
    84				continue;
    85			smt_rq = cpu_rq(smt);
    86			if (smt_rq->idle == smt_rq->curr && !smt_rq->in_smt_idle_poll)
    87				smp_send_reschedule(smt);
    88		}
    89	}
    90	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39128 bytes --]

Re: [PATCH] Add busy loop polling for idle SMT

[Peter Zijlstra]

On Tue, Nov 16, 2021 at 07:51:35PM +0800, Peng Wang wrote:
> Now we have cpu_idle_force_poll which uses cpu_relax() waiting for
> an arriving IPI, while sometimes busy loop on idle cpu is also
> useful to provide consistent pipeline interference for hardware SMT.
> 
> When hardware SMT is enabled, the switching between idle and
> busy state of one cpu will cause performance fluctuation of
> other sibling cpus on the same core.
> 
> In pay-for-execution-time scenario, cloud service providers prefer
> stable performance data to set stabel price for same workload.
> Different execution time of the same workload caused by different
> idle or busy state of sibling SMT cpus will make different bills, which
> is confused for customers.
> 
> Since there is no dynamic CPU time scaling based on SMT pipeline interference,
> to coordinate sibling SMT noise no matter whether they are idle or not,
> busy loop in idle state can provide approximately consistent pipeline interference.
> 
> For example, a workload computing tangent and cotangent will finish in 9071ms when
> sibling SMT cpus are idle, and 13299ms when sibling SMT cpus are computiing other workload.
> This generate 32% performance fluctuation.
> 
> SMT idle polling makes things slower, but we can set bigger cpu quota to make up
> a deficiency. This also increase power consumption by 2.2%, which is acceptable.
> 
> There may be some other possible solutions, while each has its own problem:
> a) disbale hardware SMT, which means half of SMT is unused and more hardware cost.
> b) busy loop in a userspace thread, but the cpu usage is confusing.
> 
> We propose this patch to discuss the performance fluctuation problem related to SMT
> pipeline interference, and any comments are welcome.

I think you missed April Fools' Day by a wide margin.

Lowering performance and increasing power usage is a direct
contradiction to sanity. It also doesn't really work as advertised,
if the siblings are competing for AVX resources the performance is a
*lot* lower than when an AVX task is competing against a spinner like
this.

Re: [PATCH] sched/idle: support busy loop polling on idle SMT cpus

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 7078 bytes --]

Hi Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on tip/master linux/master linus/master v5.16-rc1 next-20211118]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peng-Wang/sched-idle-support-busy-loop-polling-on-idle-SMT-cpus/20211116-195600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8ea9183db4ad8afbcb7089a77c23eaf965b0cacd
config: sparc-randconfig-p001-20211118 (attached as .config)
compiler: sparc64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/11c6a63e5c0f496b43058351f1e2a488ee27a5ec
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peng-Wang/sched-idle-support-busy-loop-polling-on-idle-SMT-cpus/20211116-195600
        git checkout 11c6a63e5c0f496b43058351f1e2a488ee27a5ec
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=sparc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/list.h:9,
                    from include/linux/rculist.h:10,
                    from include/linux/pid.h:5,
                    from include/linux/sched.h:14,
                    from kernel/sched/sched.h:5,
                    from kernel/sched/idle.c:9:
   kernel/sched/idle.c: In function 'smt_idle_poll_switch':
>> kernel/sched/idle.c:73:27: error: implicit declaration of function 'task_css_check' [-Werror=implicit-function-declaration]
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |                           ^~~~~~~~~~~~~~
   include/linux/kernel.h:494:33: note: in definition of macro 'container_of'
     494 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^~~
   kernel/sched/idle.c:73:51: error: 'cpu_cgrp_id' undeclared (first use in this function); did you mean 'mem_cgroup_id'?
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |                                                   ^~~~~~~~~~~
   include/linux/kernel.h:494:33: note: in definition of macro 'container_of'
     494 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^~~
   kernel/sched/idle.c:73:51: note: each undeclared identifier is reported only once for each function it appears in
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |                                                   ^~~~~~~~~~~
   include/linux/kernel.h:494:33: note: in definition of macro 'container_of'
     494 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^~~
   In file included from <command-line>:
   include/linux/kernel.h:495:58: error: invalid use of undefined type 'struct task_group'
     495 |         BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
         |                                                          ^~
   include/linux/compiler_types.h:302:23: note: in definition of macro '__compiletime_assert'
     302 |                 if (!(condition))                                       \
         |                       ^~~~~~~~~
   include/linux/compiler_types.h:322:9: note: in expansion of macro '_compiletime_assert'
     322 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:495:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     495 |         BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
         |         ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:495:27: note: in expansion of macro '__same_type'
     495 |         BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
         |                           ^~~~~~~~~~~
   kernel/sched/idle.c:73:14: note: in expansion of macro 'container_of'
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |              ^~~~~~~~~~~~
   include/linux/compiler_types.h:140:41: error: invalid use of undefined type 'struct task_group'
     140 | #define __compiler_offsetof(a, b)       __builtin_offsetof(a, b)
         |                                         ^~~~~~~~~~~~~~~~~~
   include/linux/stddef.h:17:33: note: in expansion of macro '__compiler_offsetof'
      17 | #define offsetof(TYPE, MEMBER)  __compiler_offsetof(TYPE, MEMBER)
         |                                 ^~~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:498:28: note: in expansion of macro 'offsetof'
     498 |         ((type *)(__mptr - offsetof(type, member))); })
         |                            ^~~~~~~~
   kernel/sched/idle.c:73:14: note: in expansion of macro 'container_of'
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |              ^~~~~~~~~~~~
   kernel/sched/idle.c:75:36: error: invalid use of undefined type 'struct task_group'
      75 |         rq->need_smt_idle_poll = tg->need_smt_idle_poll;
         |                                    ^~
   cc1: some warnings being treated as errors


vim +/task_css_check +73 kernel/sched/idle.c

    57	
    58	void smt_idle_poll_switch(struct rq *rq)
    59	{
    60		struct rq *smt_rq;
    61		struct task_group *tg;
    62		int smt, cpu;
    63	
    64		if (!static_branch_unlikely(&__smt_idle_poll_enabled))
    65			return;
    66	
    67		if (rq->idle == current) {
    68			rq->need_smt_idle_poll = false;
    69			return;
    70		}
    71	
    72		rcu_read_lock();
  > 73		tg = container_of(task_css_check(current, cpu_cgrp_id, true),
    74			struct task_group, css);
    75		rq->need_smt_idle_poll = tg->need_smt_idle_poll;
    76		rcu_read_unlock();
    77	
    78		if (!rq->need_smt_idle_poll)
    79			return;
    80	
    81		cpu = rq->cpu;
    82		for_each_cpu(smt, cpu_smt_mask(cpu)) {
    83			if (cpu == smt || !cpu_online(smt))
    84				continue;
    85			smt_rq = cpu_rq(smt);
    86			if (smt_rq->idle == smt_rq->curr && !smt_rq->in_smt_idle_poll)
    87				smp_send_reschedule(smt);
    88		}
    89	}
    90	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38159 bytes --]

Re: [PATCH] Add busy loop polling for idle SMT

[Peng Wang]

On 17/11/2021 18:58, , Peter Zijlstra wrote:
> On Tue, Nov 16, 2021 at 07:51:35PM +0800, Peng Wang wrote:
>> Now we have cpu_idle_force_poll which uses cpu_relax() waiting for
>> an arriving IPI, while sometimes busy loop on idle cpu is also
>> useful to provide consistent pipeline interference for hardware SMT.
>>
>> When hardware SMT is enabled, the switching between idle and
>> busy state of one cpu will cause performance fluctuation of
>> other sibling cpus on the same core.
>>
>> In pay-for-execution-time scenario, cloud service providers prefer
>> stable performance data to set stabel price for same workload.
>> Different execution time of the same workload caused by different
>> idle or busy state of sibling SMT cpus will make different bills, which
>> is confused for customers.
>>
>> Since there is no dynamic CPU time scaling based on SMT pipeline interference,
>> to coordinate sibling SMT noise no matter whether they are idle or not,
>> busy loop in idle state can provide approximately consistent pipeline interference.
>>
>> For example, a workload computing tangent and cotangent will finish in 9071ms when
>> sibling SMT cpus are idle, and 13299ms when sibling SMT cpus are computiing other workload.
>> This generate 32% performance fluctuation.
>>
>> SMT idle polling makes things slower, but we can set bigger cpu quota to make up
>> a deficiency. This also increase power consumption by 2.2%, which is acceptable.
>>
>> There may be some other possible solutions, while each has its own problem:
>> a) disbale hardware SMT, which means half of SMT is unused and more hardware cost.
>> b) busy loop in a userspace thread, but the cpu usage is confusing.
>>
>> We propose this patch to discuss the performance fluctuation problem related to SMT
>> pipeline interference, and any comments are welcome.
> 
> I think you missed April Fools' Day by a wide margin.
> 
> Lowering performance and increasing power usage is a direct

Siblings' noise depends on workloads, when persuing performance 
stability, we have to consider what performance data to keep:

a) the worst with all-time noise
b) the best with monopolizing a whole core by disabling SMT or using 
core scheduling, while wasting some logic CPUs
c) A number between the worst and the best which is hard to decide

That's where lowering performance comes from.

> contradiction to sanity. It also doesn't really work as advertised,
> if the siblings are competing for AVX resources the performance is a
> *lot* lower than when an AVX task is competing against a spinner like
> this.
> 

Yes, idle SMT busy loop polling can only provide approximately pipeline 
interference for normal instructions.

When it comes to AVX works, we notice an idea modifing CPU time 
accounting[1], do you think the combination can lead to a feasible
solution, or any other better ideas?

[1] https://www.usenix.org/conference/atc21/presentation/gottschlag

Re: [PATCH] Add busy loop polling for idle SMT

[Peter Zijlstra]

On Fri, Nov 19, 2021 at 11:19:03AM +0800, Peng Wang wrote:
> 
> Yes, idle SMT busy loop polling can only provide approximately pipeline
> interference for normal instructions.
> 
> When it comes to AVX works, we notice an idea modifing CPU time
> accounting[1], do you think the combination can lead to a feasible
> solution, or any other better ideas?

I'm not at all interested in solving this business model induced brain
trauma.

Re: [PATCH] sched/idle: support busy loop polling on idle SMT cpus

[kernel test robot]

Hi Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on tip/master linux/master linus/master v5.16-rc2 next-20211126]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Peng-Wang/sched-idle-support-busy-loop-polling-on-idle-SMT-cpus/20211116-195600
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8ea9183db4ad8afbcb7089a77c23eaf965b0cacd
config: sparc64-randconfig-r035-20211116 (https://download.01.org/0day-ci/archive/20211127/202111272343.0rQFsj1v-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/11c6a63e5c0f496b43058351f1e2a488ee27a5ec
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peng-Wang/sched-idle-support-busy-loop-polling-on-idle-SMT-cpus/20211116-195600
        git checkout 11c6a63e5c0f496b43058351f1e2a488ee27a5ec
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/list.h:9,
                    from include/linux/rculist.h:10,
                    from include/linux/pid.h:5,
                    from include/linux/sched.h:14,
                    from kernel/sched/sched.h:5,
                    from kernel/sched/idle.c:9:
   kernel/sched/idle.c: In function 'smt_idle_poll_switch':
>> kernel/sched/idle.c:73:51: error: 'cpu_cgrp_id' undeclared (first use in this function); did you mean 'io_cgrp_id'?
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |                                                   ^~~~~~~~~~~
   include/linux/kernel.h:494:33: note: in definition of macro 'container_of'
     494 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^~~
   kernel/sched/idle.c:73:27: note: in expansion of macro 'task_css_check'
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |                           ^~~~~~~~~~~~~~
   kernel/sched/idle.c:73:51: note: each undeclared identifier is reported only once for each function it appears in
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |                                                   ^~~~~~~~~~~
   include/linux/kernel.h:494:33: note: in definition of macro 'container_of'
     494 |         void *__mptr = (void *)(ptr);                                   \
         |                                 ^~~
   kernel/sched/idle.c:73:27: note: in expansion of macro 'task_css_check'
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |                           ^~~~~~~~~~~~~~
   In file included from <command-line>:
   include/linux/kernel.h:495:58: error: invalid use of undefined type 'struct task_group'
     495 |         BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
         |                                                          ^~
   include/linux/compiler_types.h:302:23: note: in definition of macro '__compiletime_assert'
     302 |                 if (!(condition))                                       \
         |                       ^~~~~~~~~
   include/linux/compiler_types.h:322:9: note: in expansion of macro '_compiletime_assert'
     322 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:495:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     495 |         BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
         |         ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:495:27: note: in expansion of macro '__same_type'
     495 |         BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
         |                           ^~~~~~~~~~~
   kernel/sched/idle.c:73:14: note: in expansion of macro 'container_of'
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |              ^~~~~~~~~~~~
>> include/linux/compiler_types.h:140:41: error: invalid use of undefined type 'struct task_group'
     140 | #define __compiler_offsetof(a, b)       __builtin_offsetof(a, b)
         |                                         ^~~~~~~~~~~~~~~~~~
   include/linux/stddef.h:17:33: note: in expansion of macro '__compiler_offsetof'
      17 | #define offsetof(TYPE, MEMBER)  __compiler_offsetof(TYPE, MEMBER)
         |                                 ^~~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:498:28: note: in expansion of macro 'offsetof'
     498 |         ((type *)(__mptr - offsetof(type, member))); })
         |                            ^~~~~~~~
   kernel/sched/idle.c:73:14: note: in expansion of macro 'container_of'
      73 |         tg = container_of(task_css_check(current, cpu_cgrp_id, true),
         |              ^~~~~~~~~~~~
>> kernel/sched/idle.c:75:36: error: invalid use of undefined type 'struct task_group'
      75 |         rq->need_smt_idle_poll = tg->need_smt_idle_poll;
         |                                    ^~


vim +73 kernel/sched/idle.c

    57	
    58	void smt_idle_poll_switch(struct rq *rq)
    59	{
    60		struct rq *smt_rq;
    61		struct task_group *tg;
    62		int smt, cpu;
    63	
    64		if (!static_branch_unlikely(&__smt_idle_poll_enabled))
    65			return;
    66	
    67		if (rq->idle == current) {
    68			rq->need_smt_idle_poll = false;
    69			return;
    70		}
    71	
    72		rcu_read_lock();
  > 73		tg = container_of(task_css_check(current, cpu_cgrp_id, true),
    74			struct task_group, css);
  > 75		rq->need_smt_idle_poll = tg->need_smt_idle_poll;
    76		rcu_read_unlock();
    77	
    78		if (!rq->need_smt_idle_poll)
    79			return;
    80	
    81		cpu = rq->cpu;
    82		for_each_cpu(smt, cpu_smt_mask(cpu)) {
    83			if (cpu == smt || !cpu_online(smt))
    84				continue;
    85			smt_rq = cpu_rq(smt);
    86			if (smt_rq->idle == smt_rq->curr && !smt_rq->in_smt_idle_poll)
    87				smp_send_reschedule(smt);
    88		}
    89	}
    90	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org