linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: Consider SMT idle status when halt polling
@ 2021-07-22  3:58 Li RongQing
  2021-07-22  4:15 ` Mika Penttilä
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Li RongQing @ 2021-07-22  3:58 UTC (permalink / raw)
  To: pbonzini, mingo, peterz, kvm, linux-kernel

SMT siblings share caches and other hardware, halt polling
will degrade its sibling performance if its sibling is busy

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 include/linux/kvm_host.h |  5 ++++-
 include/linux/sched.h    | 17 +++++++++++++++++
 kernel/sched/fair.c      | 17 -----------------
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ae7735b..15b3ef4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -269,7 +269,10 @@ static inline bool kvm_vcpu_mapped(struct kvm_host_map *map)
 
 static inline bool kvm_vcpu_can_poll(ktime_t cur, ktime_t stop)
 {
-	return single_task_running() && !need_resched() && ktime_before(cur, stop);
+	return single_task_running() &&
+		   !need_resched() &&
+		   ktime_before(cur, stop) &&
+		   is_core_idle(raw_smp_processor_id());
 }
 
 /*
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ec8d07d..c333218 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -34,6 +34,7 @@
 #include <linux/rseq.h>
 #include <linux/seqlock.h>
 #include <linux/kcsan.h>
+#include <linux/topology.h>
 #include <asm/kmap_size.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */
@@ -2191,6 +2192,22 @@ int sched_trace_rq_nr_running(struct rq *rq);
 
 const struct cpumask *sched_trace_rd_span(struct root_domain *rd);
 
+static inline bool is_core_idle(int cpu)
+{
+#ifdef CONFIG_SCHED_SMT
+	int sibling;
+
+	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
+		if (cpu == sibling)
+			continue;
+
+		if (!idle_cpu(cpu))
+			return false;
+	}
+#endif
+	return true;
+}
+
 #ifdef CONFIG_SCHED_CORE
 extern void sched_core_free(struct task_struct *tsk);
 extern void sched_core_fork(struct task_struct *p);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 44c4520..5b0259c 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1477,23 +1477,6 @@ struct numa_stats {
 	int idle_cpu;
 };
 
-static inline bool is_core_idle(int cpu)
-{
-#ifdef CONFIG_SCHED_SMT
-	int sibling;
-
-	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
-		if (cpu == sibling)
-			continue;
-
-		if (!idle_cpu(cpu))
-			return false;
-	}
-#endif
-
-	return true;
-}
-
 struct task_numa_env {
 	struct task_struct *p;
 
-- 
2.9.4


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

* Re: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  3:58 [PATCH] KVM: Consider SMT idle status when halt polling Li RongQing
@ 2021-07-22  4:15 ` Mika Penttilä
  2021-07-22  5:22   ` 答复: " Li,Rongqing
  2021-07-22  5:55 ` Wanpeng Li
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Mika Penttilä @ 2021-07-22  4:15 UTC (permalink / raw)
  To: Li RongQing, pbonzini, mingo, peterz, kvm, linux-kernel



On 22.7.2021 6.58, Li RongQing wrote:
> SMT siblings share caches and other hardware, halt polling
> will degrade its sibling performance if its sibling is busy
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>   include/linux/kvm_host.h |  5 ++++-
>   include/linux/sched.h    | 17 +++++++++++++++++
>   kernel/sched/fair.c      | 17 -----------------
>   3 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ae7735b..15b3ef4 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -269,7 +269,10 @@ static inline bool kvm_vcpu_mapped(struct kvm_host_map *map)
>   
>   static inline bool kvm_vcpu_can_poll(ktime_t cur, ktime_t stop)
>   {
> -	return single_task_running() && !need_resched() && ktime_before(cur, stop);
> +	return single_task_running() &&
> +		   !need_resched() &&
> +		   ktime_before(cur, stop) &&
> +		   is_core_idle(raw_smp_processor_id());
>   }
>   
>   /*
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index ec8d07d..c333218 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -34,6 +34,7 @@
>   #include <linux/rseq.h>
>   #include <linux/seqlock.h>
>   #include <linux/kcsan.h>
> +#include <linux/topology.h>
>   #include <asm/kmap_size.h>
>   
>   /* task_struct member predeclarations (sorted alphabetically): */
> @@ -2191,6 +2192,22 @@ int sched_trace_rq_nr_running(struct rq *rq);
>   
>   const struct cpumask *sched_trace_rd_span(struct root_domain *rd);
>   
> +static inline bool is_core_idle(int cpu)
> +{
> +#ifdef CONFIG_SCHED_SMT
> +	int sibling;
> +
> +	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
> +		if (cpu == sibling)
> +			continue;
> +
> +		if (!idle_cpu(cpu))
> +			return false;

if (!idle_cpu(sibling))  instead, now it returns always false.



> +	}
> +#endif
> +	return true;
> +}
> +
>   #ifdef CONFIG_SCHED_CORE
>   extern void sched_core_free(struct task_struct *tsk);
>   extern void sched_core_fork(struct task_struct *p);
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 44c4520..5b0259c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1477,23 +1477,6 @@ struct numa_stats {
>   	int idle_cpu;
>   };
>   
> -static inline bool is_core_idle(int cpu)
> -{
> -#ifdef CONFIG_SCHED_SMT
> -	int sibling;
> -
> -	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
> -		if (cpu == sibling)
> -			continue;
> -
> -		if (!idle_cpu(cpu))
> -			return false;
> -	}
> -#endif
> -
> -	return true;
> -}
> -
>   struct task_numa_env {
>   	struct task_struct *p;
>   


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

* 答复: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  4:15 ` Mika Penttilä
@ 2021-07-22  5:22   ` Li,Rongqing
  0 siblings, 0 replies; 12+ messages in thread
From: Li,Rongqing @ 2021-07-22  5:22 UTC (permalink / raw)
  To: Mika Penttilä, pbonzini, mingo, peterz, kvm, linux-kernel

> > diff --git a/include/linux/sched.h b/include/linux/sched.h index
> > ec8d07d..c333218 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -34,6 +34,7 @@
> >   #include <linux/rseq.h>
> >   #include <linux/seqlock.h>
> >   #include <linux/kcsan.h>
> > +#include <linux/topology.h>
> >   #include <asm/kmap_size.h>
> >
> >   /* task_struct member predeclarations (sorted alphabetically): */ @@
> > -2191,6 +2192,22 @@ int sched_trace_rq_nr_running(struct rq *rq);
> >
> >   const struct cpumask *sched_trace_rd_span(struct root_domain *rd);
> >
> > +static inline bool is_core_idle(int cpu) { #ifdef CONFIG_SCHED_SMT
> > +	int sibling;
> > +
> > +	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
> > +		if (cpu == sibling)
> > +			continue;
> > +
> > +		if (!idle_cpu(cpu))
> > +			return false;
> 
> if (!idle_cpu(sibling))  instead, now it returns always false.
> 

Good Catch. this is history bug.

Do you like to submit by yourself, or I submit on behalf you

Thanks

-Li

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

* Re: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  3:58 [PATCH] KVM: Consider SMT idle status when halt polling Li RongQing
  2021-07-22  4:15 ` Mika Penttilä
@ 2021-07-22  5:55 ` Wanpeng Li
  2021-07-22  7:10   ` 答复: " Li,Rongqing
  2021-07-22 12:16   ` Li,Rongqing
  2021-07-22 13:35 ` kernel test robot
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Wanpeng Li @ 2021-07-22  5:55 UTC (permalink / raw)
  To: Li RongQing; +Cc: Paolo Bonzini, Ingo Molnar, Peter Zijlstra, kvm, LKML

On Thu, 22 Jul 2021 at 12:07, Li RongQing <lirongqing@baidu.com> wrote:
>
> SMT siblings share caches and other hardware, halt polling
> will degrade its sibling performance if its sibling is busy

Do you have any real scenario benefits? As the polling nature, some
cloud providers will configure to their preferred balance of cpu usage
and performance, and other cloud providers for their NFV scenarios
which are more sensitive to latency are vCPU and pCPU 1:1 pin,you
destroy these setups.

    Wanpeng

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

* 答复: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  5:55 ` Wanpeng Li
@ 2021-07-22  7:10   ` Li,Rongqing
  2021-07-22 12:16   ` Li,Rongqing
  1 sibling, 0 replies; 12+ messages in thread
From: Li,Rongqing @ 2021-07-22  7:10 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Paolo Bonzini, Ingo Molnar, Peter Zijlstra, kvm, LKML



> -----邮件原件-----
> 发件人: Wanpeng Li <kernellwp@gmail.com>
> 发送时间: 2021年7月22日 13:55
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: Paolo Bonzini <pbonzini@redhat.com>; Ingo Molnar
> <mingo@redhat.com>; Peter Zijlstra <peterz@infradead.org>; kvm
> <kvm@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>
> 主题: Re: [PATCH] KVM: Consider SMT idle status when halt polling
> 
> On Thu, 22 Jul 2021 at 12:07, Li RongQing <lirongqing@baidu.com> wrote:
> >
> > SMT siblings share caches and other hardware, halt polling will
> > degrade its sibling performance if its sibling is busy
> 
> Do you have any real scenario benefits? As the polling nature, some cloud
> providers will configure to their preferred balance of cpu usage and
> performance, and other cloud providers for their NFV scenarios which are more
> sensitive to latency are vCPU and pCPU 1:1 pin,you
> destroy these setups.
>
>     Wanpeng

True, it benefits for our real scenario.

this patch can lower our workload compute latency in our multiple cores VM which vCPU and pCPU is 1:1 pin, and the workload with lots of computation and networking packets.

-Li

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

* 答复: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  5:55 ` Wanpeng Li
  2021-07-22  7:10   ` 答复: " Li,Rongqing
@ 2021-07-22 12:16   ` Li,Rongqing
  2021-07-27  1:25     ` Sean Christopherson
  1 sibling, 1 reply; 12+ messages in thread
From: Li,Rongqing @ 2021-07-22 12:16 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Paolo Bonzini, Ingo Molnar, Peter Zijlstra, kvm, LKML

> > > SMT siblings share caches and other hardware, halt polling will
> > > degrade its sibling performance if its sibling is busy
> >
> > Do you have any real scenario benefits? As the polling nature, some
> > cloud providers will configure to their preferred balance of cpu usage
> > and performance, and other cloud providers for their NFV scenarios
> > which are more sensitive to latency are vCPU and pCPU 1:1 pin,you
> > destroy these setups.
> >
> >     Wanpeng
> 


Run a copy (single thread) Unixbench, with or without a busy poll program in its SMT sibling,  and Unixbench score can lower 1/3 with SMT busy polling program

Can this case show this issue?

-Li 


> True, it benefits for our real scenario.
> 
> this patch can lower our workload compute latency in our multiple cores VM
> which vCPU and pCPU is 1:1 pin, and the workload with lots of computation and
> networking packets.
> 
> -Li

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

* Re: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  3:58 [PATCH] KVM: Consider SMT idle status when halt polling Li RongQing
  2021-07-22  4:15 ` Mika Penttilä
  2021-07-22  5:55 ` Wanpeng Li
@ 2021-07-22 13:35 ` kernel test robot
  2021-07-22 14:52 ` kernel test robot
  2021-07-22 15:07 ` Dongli Zhang
  4 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-07-22 13:35 UTC (permalink / raw)
  To: Li RongQing, pbonzini, mingo, peterz, kvm, linux-kernel
  Cc: clang-built-linux, kbuild-all

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

Hi Li,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on vhost/linux-next v5.14-rc2 next-20210722]
[cannot apply to kvm/queue]
[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/Li-RongQing/KVM-Consider-SMT-idle-status-when-halt-polling/20210722-120700
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 031e3bd8986fffe31e1ddbf5264cccfe30c9abd7
config: s390-buildonly-randconfig-r003-20210722 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9625ca5b602616b2f5584e8a49ba93c52c141e40)
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
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/df205655d183524ffe8547bcc0f230a6f3217566
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Li-RongQing/KVM-Consider-SMT-idle-status-when-halt-polling/20210722-120700
        git checkout df205655d183524ffe8547bcc0f230a6f3217566
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

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 >>, old ones prefixed by <<):

>> ERROR: modpost: "idle_cpu" [arch/s390/kvm/kvm.ko] undefined!

---
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: 34545 bytes --]

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

* Re: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  3:58 [PATCH] KVM: Consider SMT idle status when halt polling Li RongQing
                   ` (2 preceding siblings ...)
  2021-07-22 13:35 ` kernel test robot
@ 2021-07-22 14:52 ` kernel test robot
  2021-07-22 15:07 ` Dongli Zhang
  4 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2021-07-22 14:52 UTC (permalink / raw)
  To: Li RongQing, pbonzini, mingo, peterz, kvm, linux-kernel; +Cc: kbuild-all

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

Hi Li,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/sched/core]
[also build test ERROR on vhost/linux-next v5.14-rc2 next-20210722]
[cannot apply to kvm/queue]
[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/Li-RongQing/KVM-Consider-SMT-idle-status-when-halt-polling/20210722-120700
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 031e3bd8986fffe31e1ddbf5264cccfe30c9abd7
config: x86_64-kexec (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/df205655d183524ffe8547bcc0f230a6f3217566
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Li-RongQing/KVM-Consider-SMT-idle-status-when-halt-polling/20210722-120700
        git checkout df205655d183524ffe8547bcc0f230a6f3217566
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

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 >>, old ones prefixed by <<):

>> ERROR: modpost: "idle_cpu" [arch/x86/kvm/kvm.ko] undefined!

---
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: 28808 bytes --]

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

* Re: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22  3:58 [PATCH] KVM: Consider SMT idle status when halt polling Li RongQing
                   ` (3 preceding siblings ...)
  2021-07-22 14:52 ` kernel test robot
@ 2021-07-22 15:07 ` Dongli Zhang
  2021-07-22 15:20   ` Dongli Zhang
  4 siblings, 1 reply; 12+ messages in thread
From: Dongli Zhang @ 2021-07-22 15:07 UTC (permalink / raw)
  To: Li RongQing, pbonzini, mingo, peterz, kvm, linux-kernel

Hi RongQing,

Would you mind share if there is any performance data to demonstrate how much
performance can be improved?

Thank you very much!

Dongli Zhang

On 7/21/21 8:58 PM, Li RongQing wrote:
> SMT siblings share caches and other hardware, halt polling
> will degrade its sibling performance if its sibling is busy
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  include/linux/kvm_host.h |  5 ++++-
>  include/linux/sched.h    | 17 +++++++++++++++++
>  kernel/sched/fair.c      | 17 -----------------
>  3 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ae7735b..15b3ef4 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -269,7 +269,10 @@ static inline bool kvm_vcpu_mapped(struct kvm_host_map *map)
>  
>  static inline bool kvm_vcpu_can_poll(ktime_t cur, ktime_t stop)
>  {
> -	return single_task_running() && !need_resched() && ktime_before(cur, stop);
> +	return single_task_running() &&
> +		   !need_resched() &&
> +		   ktime_before(cur, stop) &&
> +		   is_core_idle(raw_smp_processor_id());
>  }
>  
>  /*
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index ec8d07d..c333218 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -34,6 +34,7 @@
>  #include <linux/rseq.h>
>  #include <linux/seqlock.h>
>  #include <linux/kcsan.h>
> +#include <linux/topology.h>
>  #include <asm/kmap_size.h>
>  
>  /* task_struct member predeclarations (sorted alphabetically): */
> @@ -2191,6 +2192,22 @@ int sched_trace_rq_nr_running(struct rq *rq);
>  
>  const struct cpumask *sched_trace_rd_span(struct root_domain *rd);
>  
> +static inline bool is_core_idle(int cpu)
> +{
> +#ifdef CONFIG_SCHED_SMT
> +	int sibling;
> +
> +	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
> +		if (cpu == sibling)
> +			continue;
> +
> +		if (!idle_cpu(cpu))
> +			return false;
> +	}
> +#endif
> +	return true;
> +}
> +
>  #ifdef CONFIG_SCHED_CORE
>  extern void sched_core_free(struct task_struct *tsk);
>  extern void sched_core_fork(struct task_struct *p);
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 44c4520..5b0259c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1477,23 +1477,6 @@ struct numa_stats {
>  	int idle_cpu;
>  };
>  
> -static inline bool is_core_idle(int cpu)
> -{
> -#ifdef CONFIG_SCHED_SMT
> -	int sibling;
> -
> -	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
> -		if (cpu == sibling)
> -			continue;
> -
> -		if (!idle_cpu(cpu))
> -			return false;
> -	}
> -#endif
> -
> -	return true;
> -}
> -
>  struct task_numa_env {
>  	struct task_struct *p;
>  
> 

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

* Re: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22 15:07 ` Dongli Zhang
@ 2021-07-22 15:20   ` Dongli Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Dongli Zhang @ 2021-07-22 15:20 UTC (permalink / raw)
  To: Li RongQing, pbonzini, mingo, peterz, kvm, linux-kernel

Please ignore my question. I saw the data from discussion.

"Run a copy (single thread) Unixbench, with or without a busy poll program in
its SMT sibling,  and Unixbench score can lower 1/3 with SMT busy polling program"

Dongli Zhang

On 7/22/21 8:07 AM, Dongli Zhang wrote:
> Hi RongQing,
> 
> Would you mind share if there is any performance data to demonstrate how much
> performance can be improved?
> 
> Thank you very much!
> 
> Dongli Zhang
> 
> On 7/21/21 8:58 PM, Li RongQing wrote:
>> SMT siblings share caches and other hardware, halt polling
>> will degrade its sibling performance if its sibling is busy
>>
>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
>> ---
>>  include/linux/kvm_host.h |  5 ++++-
>>  include/linux/sched.h    | 17 +++++++++++++++++
>>  kernel/sched/fair.c      | 17 -----------------
>>  3 files changed, 21 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index ae7735b..15b3ef4 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -269,7 +269,10 @@ static inline bool kvm_vcpu_mapped(struct kvm_host_map *map)
>>  
>>  static inline bool kvm_vcpu_can_poll(ktime_t cur, ktime_t stop)
>>  {
>> -	return single_task_running() && !need_resched() && ktime_before(cur, stop);
>> +	return single_task_running() &&
>> +		   !need_resched() &&
>> +		   ktime_before(cur, stop) &&
>> +		   is_core_idle(raw_smp_processor_id());
>>  }
>>  
>>  /*
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index ec8d07d..c333218 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -34,6 +34,7 @@
>>  #include <linux/rseq.h>
>>  #include <linux/seqlock.h>
>>  #include <linux/kcsan.h>
>> +#include <linux/topology.h>
>>  #include <asm/kmap_size.h>
>>  
>>  /* task_struct member predeclarations (sorted alphabetically): */
>> @@ -2191,6 +2192,22 @@ int sched_trace_rq_nr_running(struct rq *rq);
>>  
>>  const struct cpumask *sched_trace_rd_span(struct root_domain *rd);
>>  
>> +static inline bool is_core_idle(int cpu)
>> +{
>> +#ifdef CONFIG_SCHED_SMT
>> +	int sibling;
>> +
>> +	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
>> +		if (cpu == sibling)
>> +			continue;
>> +
>> +		if (!idle_cpu(cpu))
>> +			return false;
>> +	}
>> +#endif
>> +	return true;
>> +}
>> +
>>  #ifdef CONFIG_SCHED_CORE
>>  extern void sched_core_free(struct task_struct *tsk);
>>  extern void sched_core_fork(struct task_struct *p);
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 44c4520..5b0259c 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -1477,23 +1477,6 @@ struct numa_stats {
>>  	int idle_cpu;
>>  };
>>  
>> -static inline bool is_core_idle(int cpu)
>> -{
>> -#ifdef CONFIG_SCHED_SMT
>> -	int sibling;
>> -
>> -	for_each_cpu(sibling, cpu_smt_mask(cpu)) {
>> -		if (cpu == sibling)
>> -			continue;
>> -
>> -		if (!idle_cpu(cpu))
>> -			return false;
>> -	}
>> -#endif
>> -
>> -	return true;
>> -}
>> -
>>  struct task_numa_env {
>>  	struct task_struct *p;
>>  
>>

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

* Re: 答复: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-22 12:16   ` Li,Rongqing
@ 2021-07-27  1:25     ` Sean Christopherson
  2021-07-27  6:39       ` 答复: " Li,Rongqing
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2021-07-27  1:25 UTC (permalink / raw)
  To: Li,Rongqing
  Cc: Wanpeng Li, Paolo Bonzini, Ingo Molnar, Peter Zijlstra, kvm, LKML

On Thu, Jul 22, 2021, Li,Rongqing wrote:
> > > > SMT siblings share caches and other hardware, halt polling will
> > > > degrade its sibling performance if its sibling is busy
> > >
> > > Do you have any real scenario benefits? As the polling nature, some
> > > cloud providers will configure to their preferred balance of cpu usage
> > > and performance, and other cloud providers for their NFV scenarios
> > > which are more sensitive to latency are vCPU and pCPU 1:1 pin,you
> > > destroy these setups.
> > >
> > >     Wanpeng
> > 
> 
> 
> Run a copy (single thread) Unixbench, with or without a busy poll program in
> its SMT sibling,  and Unixbench score can lower 1/3 with SMT busy polling
> program

Rather than disallowing halt-polling entirely, on x86 it should be sufficient to
simply have the hardware thread yield to its sibling(s) via PAUSE.  It probably
won't get back all performance, but I would expect it to be close. 

This compiles on all KVM architectures, and AFAICT the intended usage of
cpu_relax() is identical for all architectures.

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6980dabe9df5..a07ecb3c67fb 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3111,6 +3111,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
                                goto out;
                        }
                        poll_end = cur = ktime_get();
+                       cpu_relax();
                } while (kvm_vcpu_can_poll(cur, stop));
        }



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

* 答复: 答复: [PATCH] KVM: Consider SMT idle status when halt polling
  2021-07-27  1:25     ` Sean Christopherson
@ 2021-07-27  6:39       ` Li,Rongqing
  0 siblings, 0 replies; 12+ messages in thread
From: Li,Rongqing @ 2021-07-27  6:39 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Wanpeng Li, Paolo Bonzini, Ingo Molnar, Peter Zijlstra, kvm, LKML



> -----邮件原件-----
> 发件人: Sean Christopherson <seanjc@google.com>
> 发送时间: 2021年7月27日 9:26
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: Wanpeng Li <kernellwp@gmail.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Ingo Molnar <mingo@redhat.com>; Peter Zijlstra
> <peterz@infradead.org>; kvm <kvm@vger.kernel.org>; LKML
> <linux-kernel@vger.kernel.org>
> 主题: Re: 答复: [PATCH] KVM: Consider SMT idle status when halt polling
> 
> Rather than disallowing halt-polling entirely, on x86 it should be sufficient to
> simply have the hardware thread yield to its sibling(s) via PAUSE.  It probably
> won't get back all performance, but I would expect it to be close.
> 
> This compiles on all KVM architectures, and AFAICT the intended usage of
> cpu_relax() is identical for all architectures.
> 

Reasonable, thanks, I will resend it

-Li


> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index
> 6980dabe9df5..a07ecb3c67fb 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3111,6 +3111,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
>                                 goto out;
>                         }
>                         poll_end = cur = ktime_get();
> +                       cpu_relax();
>                 } while (kvm_vcpu_can_poll(cur, stop));
>         }
> 


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

end of thread, other threads:[~2021-07-27  6:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22  3:58 [PATCH] KVM: Consider SMT idle status when halt polling Li RongQing
2021-07-22  4:15 ` Mika Penttilä
2021-07-22  5:22   ` 答复: " Li,Rongqing
2021-07-22  5:55 ` Wanpeng Li
2021-07-22  7:10   ` 答复: " Li,Rongqing
2021-07-22 12:16   ` Li,Rongqing
2021-07-27  1:25     ` Sean Christopherson
2021-07-27  6:39       ` 答复: " Li,Rongqing
2021-07-22 13:35 ` kernel test robot
2021-07-22 14:52 ` kernel test robot
2021-07-22 15:07 ` Dongli Zhang
2021-07-22 15:20   ` Dongli Zhang

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