All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler
@ 2012-07-12 19:17 Raghavendra K T
  2012-07-12 19:17 ` [PATCH RFC V3 1/3] kvm/config: Add config to support ple or cpu relax optimzation Raghavendra K T
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Raghavendra K T @ 2012-07-12 19:17 UTC (permalink / raw)
  To: H. Peter Anvin, Thomas Gleixner, Marcelo Tosatti, Ingo Molnar,
	Avi Kivity, Rik van Riel
  Cc: S390, Carsten Otte, Christian Borntraeger, KVM, Raghavendra K T,
	chegu vinod, Andrew M. Theurer, LKML, X86, Gleb Natapov,
	linux390, Srivatsa Vaddagiri, Joerg Roedel


Currently Pause Loop Exit (PLE) handler is doing directed yield to a
random vcpu on pl-exit. We already have filtering while choosing
the candidate to yield_to. This change adds more checks while choosing
a candidate to yield_to.

On a large vcpu guests, there is a high probability of
yielding to the same vcpu who had recently done a pause-loop exit. 
Such a yield can lead to the vcpu spinning again.

The patchset keeps track of the pause loop exit and gives chance to a
vcpu which has:

 (a) Not done pause loop exit at all (probably he is preempted lock-holder)

 (b) vcpu skipped in last iteration because it did pause loop exit, and
 probably has become eligible now (next eligible lock holder)

This concept also helps in cpu relax interception cases which use same handler.

Changes since v2:
 - Move ple structure to common code (Avi)
 - rename pause_loop_exited to cpu_relax_intercepted (Avi)
 - add config HAVE_KVM_CPU_RELAX_INTERCEPT (Avi)
 - Drop superfluous curly braces (Ingo)

Changes since v1:
 - Add more documentation for structure and algorithm and Rename
   plo ==> ple (Rik).
 - change dy_eligible initial value to false. (otherwise very first directed
    yield will not be skipped. (Nikunj)
 - fixup signoff/from issue

Future enhancements:
  (1) Currently we have a boolean to decide on eligibility of vcpu. It
    would be nice if I get feedback on guest (>32 vcpu) whether we can
    improve better with integer counter. (with counter = say f(log n )).
  
  (2) We have not considered system load during iteration of vcpu. With
   that information we can limit the scan and also decide whether schedule()
   is better. [ I am able to use #kicked vcpus to decide on this But may
   be there are better ideas like information from global loadavg.]

  (3) We can exploit this further with PV patches since it also knows about
   next eligible lock-holder.

Summary: There is a very good improvement for moderate / no over-commit scenario
 for kvm based guest on PLE machine.

 Results: kernbench improves by around 30%, 6% for 1x,2x
          ebizzy improves by around 87%, 23% for 1x,2x

 Links 
  V1: https://lkml.org/lkml/2012/7/9/32

  V2: https://lkml.org/lkml/2012/7/10/392

 Raghavendra K T (3):
   config: Add config to support ple or cpu relax optimzation 
   kvm : Note down when cpu relax intercepted or pause loop exited 
   kvm : Choose a better candidate for directed yield 
---
 arch/s390/kvm/Kconfig    |    1 +
 arch/x86/kvm/Kconfig     |    1 +
 include/linux/kvm_host.h |   12 ++++++++++++
 virt/kvm/Kconfig         |    3 +++
 virt/kvm/kvm_main.c      |   38 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 0 deletions(-)


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

* [PATCH RFC V3 1/3] kvm/config: Add config to support ple or cpu relax optimzation
  2012-07-12 19:17 [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
@ 2012-07-12 19:17 ` Raghavendra K T
  2012-07-12 19:18 ` [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited Raghavendra K T
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Raghavendra K T @ 2012-07-12 19:17 UTC (permalink / raw)
  To: H. Peter Anvin, Thomas Gleixner, Avi Kivity, Ingo Molnar,
	Marcelo Tosatti, Rik van Riel
  Cc: S390, Carsten Otte, Christian Borntraeger, KVM, Raghavendra K T,
	chegu vinod, Andrew M. Theurer, LKML, X86, Gleb Natapov,
	linux390, Srivatsa Vaddagiri, Joerg Roedel

From: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>

Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
---

 arch/s390/kvm/Kconfig |    1 +
 arch/x86/kvm/Kconfig  |    1 +
 virt/kvm/Kconfig      |    3 +++
 3 files changed, 5 insertions(+), 0 deletions(-)


diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig
index 78eb984..a6e2677 100644
--- a/arch/s390/kvm/Kconfig
+++ b/arch/s390/kvm/Kconfig
@@ -21,6 +21,7 @@ config KVM
 	depends on HAVE_KVM && EXPERIMENTAL
 	select PREEMPT_NOTIFIERS
 	select ANON_INODES
+	select HAVE_KVM_CPU_RELAX_INTERCEPT
 	---help---
 	  Support hosting paravirtualized guest machines using the SIE
 	  virtualization capability on the mainframe. This should work
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index a28f338..45c044f 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -37,6 +37,7 @@ config KVM
 	select TASK_DELAY_ACCT
 	select PERF_EVENTS
 	select HAVE_KVM_MSI
+	select HAVE_KVM_CPU_RELAX_INTERCEPT
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
 	  virtualization extensions.  You will need a fairly recent
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 28694f4..d01b24b 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -21,3 +21,6 @@ config KVM_ASYNC_PF
 
 config HAVE_KVM_MSI
        bool
+
+config HAVE_KVM_CPU_RELAX_INTERCEPT
+       bool


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

* [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited
  2012-07-12 19:17 [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
  2012-07-12 19:17 ` [PATCH RFC V3 1/3] kvm/config: Add config to support ple or cpu relax optimzation Raghavendra K T
@ 2012-07-12 19:18 ` Raghavendra K T
  2012-07-12 20:02   ` Christian Borntraeger
  2012-07-12 19:18 ` [PATCH RFC V3 3/3] kvm: Choose better candidate for directed yield Raghavendra K T
  2012-07-12 19:23 ` [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
  3 siblings, 1 reply; 12+ messages in thread
From: Raghavendra K T @ 2012-07-12 19:18 UTC (permalink / raw)
  To: H. Peter Anvin, Thomas Gleixner, Marcelo Tosatti, Ingo Molnar,
	Avi Kivity, Rik van Riel
  Cc: S390, Carsten Otte, Christian Borntraeger, KVM, Raghavendra K T,
	chegu vinod, Andrew M. Theurer, LKML, X86, Gleb Natapov,
	linux390, Srivatsa Vaddagiri, Joerg Roedel

From: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>

Noting pause loop exited or cpu relax intercepted vcpu helps in
filtering right candidate to yield. Wrong selection of vcpu;
i.e., a vcpu that just did a pl-exit or cpu relax intercepted may
contribute to performance degradation.

Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
---
v2 patches were
Reviewed-by: Rik van Riel <riel@redhat.com>

 include/linux/kvm_host.h |   12 ++++++++++++
 virt/kvm/kvm_main.c      |    4 ++++
 2 files changed, 16 insertions(+), 0 deletions(-)


diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c446435..4ec1cf0 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -183,6 +183,18 @@ struct kvm_vcpu {
 	} async_pf;
 #endif
 
+#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
+	/*
+	 * Cpu relax intercept or pause loop exit optimization
+	 * cpu_relax_intercepted: set when a vcpu does a pause loop exit
+	 *  or cpu relax intercepted.
+	 * dy_eligible: indicates whether vcpu is eligible for directed yield.
+	 */
+	struct {
+		bool cpu_relax_intercepted;
+		bool dy_eligible;
+	} ple;
+#endif
 	struct kvm_vcpu_arch arch;
 };
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7e14068..4ec0120 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -235,6 +235,8 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
 		goto fail;
 	}
 	vcpu->run = page_address(page);
+	vcpu->ple.cpu_relax_intercepted = false;
+	vcpu->ple.dy_eligible = false;
 
 	r = kvm_arch_vcpu_init(vcpu);
 	if (r < 0)
@@ -1577,6 +1579,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 	int pass;
 	int i;
 
+	me->ple.cpu_relax_intercepted = true;
 	/*
 	 * We boost the priority of a VCPU that is runnable but not
 	 * currently running, because it got preempted by something
@@ -1602,6 +1605,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 			}
 		}
 	}
+	me->ple.cpu_relax_intercepted = false;
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_on_spin);
 


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

* [PATCH RFC V3 3/3] kvm: Choose better candidate for directed yield
  2012-07-12 19:17 [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
  2012-07-12 19:17 ` [PATCH RFC V3 1/3] kvm/config: Add config to support ple or cpu relax optimzation Raghavendra K T
  2012-07-12 19:18 ` [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited Raghavendra K T
@ 2012-07-12 19:18 ` Raghavendra K T
  2012-07-12 19:23 ` [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
  3 siblings, 0 replies; 12+ messages in thread
From: Raghavendra K T @ 2012-07-12 19:18 UTC (permalink / raw)
  To: H. Peter Anvin, Thomas Gleixner, Avi Kivity, Ingo Molnar,
	Marcelo Tosatti, Rik van Riel
  Cc: S390, Carsten Otte, Christian Borntraeger, KVM, Raghavendra K T,
	chegu vinod, Andrew M. Theurer, LKML, X86, Gleb Natapov,
	linux390, Srivatsa Vaddagiri, Joerg Roedel

From: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>

Currently, on a large vcpu guests, there is a high probability of
yielding to the same vcpu who had recently done a pause-loop exit or
cpu relax intercepted. Such a yield can lead to the vcpu spinning
again and hence degrade the performance.

The patchset keeps track of the pause loop exit/cpu relax interception
and gives chance to a vcpu which:
 (a) Has not done pause loop exit or cpu relax intercepted at all
     (probably he is preempted lock-holder)
 (b) Was skipped in last iteration because it did pause loop exit or
     cpu relax intercepted, and probably has become eligible now
     (next eligible lock holder)

Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
---
v2 patches were:
Reviewed-by: Rik van Riel <riel@redhat.com>

 virt/kvm/kvm_main.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)


diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4ec0120..50f6e60 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1570,6 +1570,38 @@ bool kvm_vcpu_yield_to(struct kvm_vcpu *target)
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_yield_to);
 
+/*
+ * Helper that checks whether a VCPU is eligible for directed yield.
+ * Most eligible candidate to yield is decided by following heuristics:
+ *
+ *  (a) VCPU which has not done pl-exit or cpu relax intercepted recently
+ *  (preempted lock holder), indicated by @cpu_relax_intercepted.
+ *  Set at the beiginning and cleared at the end of interception/PLE handler.
+ *
+ *  (b) VCPU which has done pl-exit/ cpu relax intercepted but did not get
+ *  chance last time (mostly it has become eligible now since we have probably
+ *  yielded to lockholder in last iteration. This is done by toggling
+ *  @dy_eligible each time a VCPU checked for eligibility.)
+ *
+ *  Yielding to a recently pl-exited/cpu relax intercepted VCPU before yielding
+ *  to preempted lock-holder could result in wrong VCPU selection and CPU
+ *  burning. Giving priority for a potential lock-holder increases lock
+ *  progress.
+ */
+bool kvm_vcpu_check_and_update_eligible(struct kvm_vcpu *vcpu)
+{
+	bool eligible;
+
+	eligible = !vcpu->ple.cpu_relax_intercepted ||
+			(vcpu->ple.cpu_relax_intercepted &&
+			 vcpu->ple.dy_eligible);
+
+	if (vcpu->ple.cpu_relax_intercepted)
+		vcpu->ple.dy_eligible = !vcpu->ple.dy_eligible;
+
+	return eligible;
+}
+
 void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 {
 	struct kvm *kvm = me->kvm;
@@ -1598,6 +1630,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 				continue;
 			if (waitqueue_active(&vcpu->wq))
 				continue;
+			if (!kvm_vcpu_check_and_update_eligible(vcpu))
+				continue;
 			if (kvm_vcpu_yield_to(vcpu)) {
 				kvm->last_boosted_vcpu = i;
 				yielded = 1;


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

* Re: [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler
  2012-07-12 19:17 [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
                   ` (2 preceding siblings ...)
  2012-07-12 19:18 ` [PATCH RFC V3 3/3] kvm: Choose better candidate for directed yield Raghavendra K T
@ 2012-07-12 19:23 ` Raghavendra K T
  2012-07-19  9:15   ` [RESEND PATCH " Raghavendra K T
  3 siblings, 1 reply; 12+ messages in thread
From: Raghavendra K T @ 2012-07-12 19:23 UTC (permalink / raw)
  To: S390, Carsten Otte, Christian Borntraeger, linux390
  Cc: Raghavendra K T, H. Peter Anvin, Thomas Gleixner,
	Marcelo Tosatti, Ingo Molnar, Avi Kivity, Rik van Riel, KVM,
	chegu vinod, Andrew M. Theurer, LKML, X86, Gleb Natapov,
	Srivatsa Vaddagiri, Joerg Roedel

On 07/13/2012 12:47 AM, Raghavendra K T wrote:
> Currently Pause Loop Exit (PLE) handler is doing directed yield to a
> random vcpu on pl-exit. We already have filtering while choosing
> the candidate to yield_to. This change adds more checks while choosing
> a candidate to yield_to.
>
> On a large vcpu guests, there is a high probability of
> yielding to the same vcpu who had recently done a pause-loop exit.
> Such a yield can lead to the vcpu spinning again.
>
> The patchset keeps track of the pause loop exit and gives chance to a
> vcpu which has:
>
>   (a) Not done pause loop exit at all (probably he is preempted lock-holder)
>
>   (b) vcpu skipped in last iteration because it did pause loop exit, and
>   probably has become eligible now (next eligible lock holder)
>
> This concept also helps in cpu relax interception cases which use same handler.

The patches are tested on x86 only since I don't have access to s390
machine. Please let me know if changes are ok on s390.


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

* Re: [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited
  2012-07-12 19:18 ` [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited Raghavendra K T
@ 2012-07-12 20:02   ` Christian Borntraeger
  2012-07-13  3:35     ` Raghavendra K T
  2012-07-13 13:54     ` Srikar Dronamraju
  0 siblings, 2 replies; 12+ messages in thread
From: Christian Borntraeger @ 2012-07-12 20:02 UTC (permalink / raw)
  To: Raghavendra K T
  Cc: H. Peter Anvin, Thomas Gleixner, Marcelo Tosatti, Ingo Molnar,
	Avi Kivity, Rik van Riel, S390, Carsten Otte, KVM, chegu vinod,
	Andrew M. Theurer, LKML, X86, Gleb Natapov, linux390,
	Srivatsa Vaddagiri, Joerg Roedel

On 12/07/12 21:18, Raghavendra K T wrote:
> +#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
[...]
> +	struct {
> +		bool cpu_relax_intercepted;
> +		bool dy_eligible;
> +	} ple;
> +#endif
[...]
>  	}
>  	vcpu->run = page_address(page);
> +	vcpu->ple.cpu_relax_intercepted = false;
> +	vcpu->ple.dy_eligible = false;

This struct is only defined if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is set, but here it
is always accessed. Will break on !x86 && !s390.
> 
>  	r = kvm_arch_vcpu_init(vcpu);
>  	if (r < 0)
> @@ -1577,6 +1579,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>  	int pass;
>  	int i;
> 
> +	me->ple.cpu_relax_intercepted = true;

dito
>  	/*
>  	 * We boost the priority of a VCPU that is runnable but not
>  	 * currently running, because it got preempted by something
> @@ -1602,6 +1605,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>  			}
>  		}
>  	}
> +	me->ple.cpu_relax_intercepted = false;

again.

maybe define static inline access functions in kvm_host.h that are no-ops
if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is not set.


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

* Re: [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited
  2012-07-12 20:02   ` Christian Borntraeger
@ 2012-07-13  3:35     ` Raghavendra K T
  2012-07-13  6:13       ` Christian Borntraeger
  2012-07-13 13:54     ` Srikar Dronamraju
  1 sibling, 1 reply; 12+ messages in thread
From: Raghavendra K T @ 2012-07-13  3:35 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: H. Peter Anvin, Thomas Gleixner, Marcelo Tosatti, Ingo Molnar,
	Avi Kivity, Rik van Riel, S390, Carsten Otte, KVM, chegu vinod,
	Andrew M. Theurer, LKML, X86, Gleb Natapov, linux390,
	Srivatsa Vaddagiri, Joerg Roedel

On 07/13/2012 01:32 AM, Christian Borntraeger wrote:
> On 12/07/12 21:18, Raghavendra K T wrote:
>> +#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
> [...]
>> +	struct {
>> +		bool cpu_relax_intercepted;
>> +		bool dy_eligible;
>> +	} ple;
>> +#endif
> [...]
>>   	}
>>   	vcpu->run = page_address(page);
>> +	vcpu->ple.cpu_relax_intercepted = false;
>> +	vcpu->ple.dy_eligible = false;
>
> This struct is only defined if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is set, but here it
> is always accessed. Will break on !x86&&  !s390.

Yes! I forgot about archs in init function.
How about having
#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
vcpu->ple.cpu_relax_intercepted = false;
vcpu->ple.dy_eligible = false;
#endif

This would solve all the problem.

>>
>>   	r = kvm_arch_vcpu_init(vcpu);
>>   	if (r<  0)
>> @@ -1577,6 +1579,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>   	int pass;
>>   	int i;
>>
>> +	me->ple.cpu_relax_intercepted = true;
>
> dito

currently vcpu_on_spin is used only by x86 and s390. so if some other
arch in future uses vcpu_on_spin, I believe they also have to enable
CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
what do you think?

otherwise we have to add hook everywhere
>>   	/*
>>   	 * We boost the priority of a VCPU that is runnable but not
>>   	 * currently running, because it got preempted by something
>> @@ -1602,6 +1605,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>   			}
>>   		}
>>   	}
>> +	me->ple.cpu_relax_intercepted = false;
>
> again.
>
> maybe define static inline access functions in kvm_host.h that are no-ops
> if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is not set.
>


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

* Re: [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited
  2012-07-13  3:35     ` Raghavendra K T
@ 2012-07-13  6:13       ` Christian Borntraeger
  2012-07-13 10:11         ` Raghavendra K T
  0 siblings, 1 reply; 12+ messages in thread
From: Christian Borntraeger @ 2012-07-13  6:13 UTC (permalink / raw)
  To: Raghavendra K T
  Cc: H. Peter Anvin, Thomas Gleixner, Marcelo Tosatti, Ingo Molnar,
	Avi Kivity, Rik van Riel, S390, Carsten Otte, KVM, chegu vinod,
	Andrew M. Theurer, LKML, X86, Gleb Natapov, linux390,
	Srivatsa Vaddagiri, Joerg Roedel

On 13/07/12 05:35, Raghavendra K T wrote:
> Yes! I forgot about archs in init function.
> How about having
> #ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
> vcpu->ple.cpu_relax_intercepted = false;
> vcpu->ple.dy_eligible = false;
> #endif
> 
> This would solve all the problem.

No, you need to mask all places....

> 
>>>
>>>       r = kvm_arch_vcpu_init(vcpu);
>>>       if (r<  0)
>>> @@ -1577,6 +1579,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
>>>       int pass;
>>>       int i;
>>>
>>> +    me->ple.cpu_relax_intercepted = true;
>>
>> dito
> 
> currently vcpu_on_spin is used only by x86 and s390. so if some other
> arch in future uses vcpu_on_spin, I believe they also have to enable
> CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
> what do you think?

...because  this function is compiled no matter if called or not.
> 
>> maybe define static inline access functions in kvm_host.h that are no-ops
>> if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is not set.

As I already said, can you have a look at using access functions?





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

* Re: [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited
  2012-07-13  6:13       ` Christian Borntraeger
@ 2012-07-13 10:11         ` Raghavendra K T
  0 siblings, 0 replies; 12+ messages in thread
From: Raghavendra K T @ 2012-07-13 10:11 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: H. Peter Anvin, Thomas Gleixner, Marcelo Tosatti, Ingo Molnar,
	Avi Kivity, Rik van Riel, S390, Carsten Otte, KVM, chegu vinod,
	Andrew M. Theurer, LKML, X86, Gleb Natapov, linux390,
	Srivatsa Vaddagiri, Joerg Roedel

On 07/13/2012 11:43 AM, Christian Borntraeger wrote:
> On 13/07/12 05:35, Raghavendra K T wrote:
>>> maybe define static inline access functions in kvm_host.h that are no-ops
>>> if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is not set.
>
> As I already said, can you have a look at using access functions?

Yes. will do.


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

* Re: [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited
  2012-07-12 20:02   ` Christian Borntraeger
  2012-07-13  3:35     ` Raghavendra K T
@ 2012-07-13 13:54     ` Srikar Dronamraju
  2012-07-16  7:38       ` Raghavendra K T
  1 sibling, 1 reply; 12+ messages in thread
From: Srikar Dronamraju @ 2012-07-13 13:54 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Raghavendra K T, H. Peter Anvin, Thomas Gleixner,
	Marcelo Tosatti, Ingo Molnar, Avi Kivity, Rik van Riel, S390,
	Carsten Otte, KVM, chegu vinod, Andrew M. Theurer, LKML, X86,
	Gleb Natapov, linux390, Srivatsa Vaddagiri, Joerg Roedel

> On 12/07/12 21:18, Raghavendra K T wrote:
> > +#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
> [...]
> > +	struct {
> > +		bool cpu_relax_intercepted;
> > +		bool dy_eligible;
> > +	} ple;
> > +#endif
> [...]
> >  	}
> >  	vcpu->run = page_address(page);
> > +	vcpu->ple.cpu_relax_intercepted = false;
> > +	vcpu->ple.dy_eligible = false;
> 
> This struct is only defined if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is set, but here it
> is always accessed. Will break on !x86 && !s390.

How about moving this struct definition outside the CONFIG.
i.e it would be available by default.
If any arch cares to use vcpu_on_spin(), they would get the benefit by
default. 

This would avoid all the CONFIG magic that we would have to do
otherwise.

-- 
Thanks and Regards
Srikar


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

* Re: [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited
  2012-07-13 13:54     ` Srikar Dronamraju
@ 2012-07-16  7:38       ` Raghavendra K T
  0 siblings, 0 replies; 12+ messages in thread
From: Raghavendra K T @ 2012-07-16  7:38 UTC (permalink / raw)
  To: Srikar Dronamraju
  Cc: Christian Borntraeger, H. Peter Anvin, Thomas Gleixner,
	Marcelo Tosatti, Ingo Molnar, Avi Kivity, Rik van Riel, S390,
	Carsten Otte, KVM, chegu vinod, Andrew M. Theurer, LKML, X86,
	Gleb Natapov, linux390, Srivatsa Vaddagiri, Joerg Roedel

On 07/13/2012 07:24 PM, Srikar Dronamraju wrote:
>> On 12/07/12 21:18, Raghavendra K T wrote:
>>> +#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
>> [...]
>>> +	struct {
>>> +		bool cpu_relax_intercepted;
>>> +		bool dy_eligible;
>>> +	} ple;
>>> +#endif
>> [...]
>>>   	}
>>>   	vcpu->run = page_address(page);
>>> +	vcpu->ple.cpu_relax_intercepted = false;
>>> +	vcpu->ple.dy_eligible = false;
>>
>> This struct is only defined if CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT is set, but here it
>> is always accessed. Will break on !x86&&  !s390.
>
> How about moving this struct definition outside the CONFIG.
> i.e it would be available by default.
> If any arch cares to use vcpu_on_spin(), they would get the benefit by
> default.
>
> This would avoid all the CONFIG magic that we would have to do
> otherwise.
>

Okay, after discussing with Christian,
- even if ppc uses vcpu_on spin we will still be left with ia64 (though
broken currently) and arm (is on way).
- those who want to opt-out of this optimization but still wish to use
vcpu_spin, we have flexibility.

So with that in mind I am spinning V4 with
CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT. Let us see how it goes.


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

* [RESEND PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler
  2012-07-12 19:23 ` [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
@ 2012-07-19  9:15   ` Raghavendra K T
  0 siblings, 0 replies; 12+ messages in thread
From: Raghavendra K T @ 2012-07-19  9:15 UTC (permalink / raw)
  To: Christian Borntraeger, linux390, H. Peter Anvin, Thomas Gleixner,
	Marcelo Tosatti, Ingo Molnar, Avi Kivity, Rik van Riel, KVM
  Cc: S390, Carsten Otte, chegu vinod, Andrew M. Theurer, LKML, X86,
	Gleb Natapov, Srivatsa Vaddagiri, Joerg Roedel

From: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>

Currently, on a large vcpu guests, there is a high probability of
yielding to the same vcpu who had recently done a pause-loop exit or
cpu relax intercepted. Such a yield can lead to the vcpu spinning
again and hence degrade the performance.

The patchset keeps track of the pause loop exit/cpu relax interception
and gives chance to a vcpu which:
 (a) Has not done pause loop exit or cpu relax intercepted at all
     (probably he is preempted lock-holder)
 (b) Was skipped in last iteration because it did pause loop exit or
     cpu relax intercepted, and probably has become eligible now
     (next eligible lock holder)

Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
---
V2 was:
Reviewed-by: Rik van Riel <riel@redhat.com>

Changelog: Added comment on locking as suggested by Avi

 include/linux/kvm_host.h |    5 +++++
 virt/kvm/kvm_main.c      |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 34ce296..952427d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -923,6 +923,11 @@ static inline void kvm_vcpu_set_dy_eligible(struct kvm_vcpu *vcpu, bool val)
 {
 }
 
+static inline bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
+{
+	return true;
+}
+
 #endif /* CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT */
 #endif
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 3d6ffc8..8fda756 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1571,6 +1571,43 @@ bool kvm_vcpu_yield_to(struct kvm_vcpu *target)
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_yield_to);
 
+#ifdef CONFIG_HAVE_KVM_CPU_RELAX_INTERCEPT
+/*
+ * Helper that checks whether a VCPU is eligible for directed yield.
+ * Most eligible candidate to yield is decided by following heuristics:
+ *
+ *  (a) VCPU which has not done pl-exit or cpu relax intercepted recently
+ *  (preempted lock holder), indicated by @in_spin_loop.
+ *  Set at the beiginning and cleared at the end of interception/PLE handler.
+ *
+ *  (b) VCPU which has done pl-exit/ cpu relax intercepted but did not get
+ *  chance last time (mostly it has become eligible now since we have probably
+ *  yielded to lockholder in last iteration. This is done by toggling
+ *  @dy_eligible each time a VCPU checked for eligibility.)
+ *
+ *  Yielding to a recently pl-exited/cpu relax intercepted VCPU before yielding
+ *  to preempted lock-holder could result in wrong VCPU selection and CPU
+ *  burning. Giving priority for a potential lock-holder increases lock
+ *  progress.
+ *
+ *  Since algorithm is based on heuristics, accessing another VCPU data without
+ *  locking does not harm. It may result in trying to yield to same VCPU, fail
+ *  and continue with next VCPU and so on.
+ */
+bool kvm_vcpu_eligible_for_directed_yield(struct kvm_vcpu *vcpu)
+{
+	bool eligible;
+
+	eligible = !vcpu->spin_loop.in_spin_loop ||
+			(vcpu->spin_loop.in_spin_loop &&
+			 vcpu->spin_loop.dy_eligible);
+
+	if (vcpu->spin_loop.in_spin_loop)
+		kvm_vcpu_set_dy_eligible(vcpu, !vcpu->spin_loop.dy_eligible);
+
+	return eligible;
+}
+#endif
 void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 {
 	struct kvm *kvm = me->kvm;
@@ -1599,6 +1636,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 				continue;
 			if (waitqueue_active(&vcpu->wq))
 				continue;
+			if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
+				continue;
 			if (kvm_vcpu_yield_to(vcpu)) {
 				kvm->last_boosted_vcpu = i;
 				yielded = 1;
@@ -1607,6 +1646,9 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 		}
 	}
 	kvm_vcpu_set_in_spin_loop(me, false);
+
+	/* Ensure vcpu is not eligible during next spinloop */
+	kvm_vcpu_set_dy_eligible(me, false);
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_on_spin);
 


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

end of thread, other threads:[~2012-07-19  9:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-12 19:17 [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
2012-07-12 19:17 ` [PATCH RFC V3 1/3] kvm/config: Add config to support ple or cpu relax optimzation Raghavendra K T
2012-07-12 19:18 ` [PATCH RFC V3 2/3] kvm: Note down when cpu relax intercepted or pause loop exited Raghavendra K T
2012-07-12 20:02   ` Christian Borntraeger
2012-07-13  3:35     ` Raghavendra K T
2012-07-13  6:13       ` Christian Borntraeger
2012-07-13 10:11         ` Raghavendra K T
2012-07-13 13:54     ` Srikar Dronamraju
2012-07-16  7:38       ` Raghavendra K T
2012-07-12 19:18 ` [PATCH RFC V3 3/3] kvm: Choose better candidate for directed yield Raghavendra K T
2012-07-12 19:23 ` [PATCH RFC V3 0/3] kvm: Improving directed yield in PLE handler Raghavendra K T
2012-07-19  9:15   ` [RESEND PATCH " Raghavendra K T

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.