LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH RFC] kvm: x86: add halt_poll module parameter
@ 2015-02-05 16:05 Paolo Bonzini
  2015-02-05 17:46 ` Rik van Riel
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Paolo Bonzini @ 2015-02-05 16:05 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: riel, rkrcmar, mtosatti

This patch introduces a new module parameter for the KVM module; when it
is present, KVM attempts a bit of polling on every HLT before scheduling
itself out via kvm_vcpu_block.

This parameter helps a lot for latency-bound workloads---in particular
I tested it with O_DSYNC writes with a battery-backed disk in the host.
In this case, writes are fast (because the data doesn't have to go all
the way to the platters) but they cannot be merged by either the host or
the guest.  KVM's performance here is usually around 30% of bare metal,
or 50% if you use cache=directsync or cache=writethrough (these
parameters avoid that the guest sends pointless flush requests, and
at the same time they are not slow because of the battery-backed cache).
The bad performance happens because on every halt the host CPU decides
to halt itself too.  When the interrupt comes, the vCPU thread is then
migrated to a new physical CPU, and in general the latency is horrible
because the vCPU thread has to be scheduled back in.

With this patch performance reaches 60-65% of bare metal and, more
important, 99% of what you get if you use idle=poll in the guest.  This
means that the tunable gets rid of this particular bottleneck, and more
work can be done to improve performance in the kernel or QEMU.

Of course there is some price to pay; every time an otherwise idle vCPUs
is interrupted by an interrupt, it will poll unnecessarily and thus
impose a little load on the host.  The above results were obtained with
a mostly random value of the parameter (2000000), and the load was around
1.5-2.5% CPU usage on one of the host's core for each idle guest vCPU.

The patch also adds a new stat, /sys/kernel/debug/kvm/halt_successful_poll,
that can be used to tune the parameter.  It counts how many HLT
instructions received an interrupt during the polling period; each
successful poll avoids that Linux schedules the VCPU thread out and back
in, and may also avoid a likely trip to C1 and back for the physical CPU.

While the VM is idle, a Linux 4 VCPU VM halts around 10 times per second.
Of these halts, almost all are failed polls.  During the benchmark,
instead, basically all halts end within the polling period, except a more
or less constant stream of 50 per second coming from vCPUs that are not
running the benchmark.  The wasted time is thus very low.  Things may
be slightly different for Windows VMs, which have a ~10 ms timer tick.

The effect is also visible on Marcelo's recently-introduced latency
test for the TSC deadline timer.  Though of course a non-RT kernel has
awful latency bounds, the latency of the timer is around 8000-10000 clock
cycles compared to 20000-120000 without setting halt_poll.  For the TSC
deadline timer, thus, the effect is both a smaller average latency and
a smaller variance.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/x86.c              | 28 ++++++++++++++++++++++++----
 include/linux/kvm_host.h        |  1 +
 virt/kvm/kvm_main.c             | 22 +++++++++++++++-------
 4 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 848947ac6ade..a236e39cc385 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -655,6 +655,7 @@ struct kvm_vcpu_stat {
 	u32 irq_window_exits;
 	u32 nmi_window_exits;
 	u32 halt_exits;
+	u32 halt_successful_poll;
 	u32 halt_wakeup;
 	u32 request_irq_exits;
 	u32 irq_exits;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1373e04e1f19..b7b20828f01c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -96,6 +96,9 @@ EXPORT_SYMBOL_GPL(kvm_x86_ops);
 static bool ignore_msrs = 0;
 module_param(ignore_msrs, bool, S_IRUGO | S_IWUSR);
 
+unsigned int halt_poll = 0;
+module_param(halt_poll, uint, S_IRUGO | S_IWUSR);
+
 unsigned int min_timer_period_us = 500;
 module_param(min_timer_period_us, uint, S_IRUGO | S_IWUSR);
 
@@ -145,6 +148,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ "irq_window", VCPU_STAT(irq_window_exits) },
 	{ "nmi_window", VCPU_STAT(nmi_window_exits) },
 	{ "halt_exits", VCPU_STAT(halt_exits) },
+	{ "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
 	{ "halt_wakeup", VCPU_STAT(halt_wakeup) },
 	{ "hypercalls", VCPU_STAT(hypercalls) },
 	{ "request_irq", VCPU_STAT(request_irq_exits) },
@@ -5819,13 +5823,29 @@ void kvm_arch_exit(void)
 int kvm_emulate_halt(struct kvm_vcpu *vcpu)
 {
 	++vcpu->stat.halt_exits;
-	if (irqchip_in_kernel(vcpu->kvm)) {
-		vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
-		return 1;
-	} else {
+	if (!irqchip_in_kernel(vcpu->kvm)) {
 		vcpu->run->exit_reason = KVM_EXIT_HLT;
 		return 0;
 	}
+
+	vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
+	if (halt_poll) {
+		u64 start, curr;
+		rdtscll(start);
+		do {
+			/*
+			 * This sets KVM_REQ_UNHALT if an interrupt
+			 * arrives.
+			 */
+			if (kvm_vcpu_check_block(vcpu) < 0) {
+				++vcpu->stat.halt_successful_poll;
+				break;
+			}
+			rdtscll(curr);
+		} while(!need_resched() && curr - start < halt_poll);
+	}
+
+	return 1;
 }
 EXPORT_SYMBOL_GPL(kvm_emulate_halt);
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 8a82838034f1..1519d48d956f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -584,6 +584,7 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
 unsigned long kvm_host_page_size(struct kvm *kvm, gfn_t gfn);
 void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 
+int kvm_vcpu_check_block(struct kvm_vcpu *vcpu);
 void kvm_vcpu_block(struct kvm_vcpu *vcpu);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
 int kvm_vcpu_yield_to(struct kvm_vcpu *target);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0c281760a1c5..825fc3ec0509 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1813,6 +1813,20 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
 }
 EXPORT_SYMBOL_GPL(mark_page_dirty);
 
+int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)
+{
+	if (kvm_arch_vcpu_runnable(vcpu)) {
+		kvm_make_request(KVM_REQ_UNHALT, vcpu);
+		return -EINTR;
+	}
+	if (kvm_cpu_has_pending_timer(vcpu))
+		return -EINTR;
+	if (signal_pending(current))
+		return -EINTR;
+
+	return 0;
+}
+
 /*
  * The vCPU has executed a HLT instruction with in-kernel mode enabled.
  */
@@ -1823,13 +1837,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
 	for (;;) {
 		prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
 
-		if (kvm_arch_vcpu_runnable(vcpu)) {
-			kvm_make_request(KVM_REQ_UNHALT, vcpu);
-			break;
-		}
-		if (kvm_cpu_has_pending_timer(vcpu))
-			break;
-		if (signal_pending(current))
+		if (kvm_vcpu_check_block(vcpu) < 0)
 			break;
 
 		schedule();
-- 
1.8.3.1


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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 16:05 [PATCH RFC] kvm: x86: add halt_poll module parameter Paolo Bonzini
@ 2015-02-05 17:46 ` Rik van Riel
  2015-02-05 17:53 ` Radim Krčmář
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Rik van Riel @ 2015-02-05 17:46 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm; +Cc: rkrcmar, mtosatti

On 02/05/2015 11:05 AM, Paolo Bonzini wrote:
> This patch introduces a new module parameter for the KVM module; when it
> is present, KVM attempts a bit of polling on every HLT before scheduling
> itself out via kvm_vcpu_block.
> 
> This parameter helps a lot for latency-bound workloads---in particular
> I tested it with O_DSYNC writes with a battery-backed disk in the host.
> In this case, writes are fast (because the data doesn't have to go all
> the way to the platters) but they cannot be merged by either the host or
> the guest.  KVM's performance here is usually around 30% of bare metal,
> or 50% if you use cache=directsync or cache=writethrough (these
> parameters avoid that the guest sends pointless flush requests, and
> at the same time they are not slow because of the battery-backed cache).
> The bad performance happens because on every halt the host CPU decides
> to halt itself too.  When the interrupt comes, the vCPU thread is then
> migrated to a new physical CPU, and in general the latency is horrible
> because the vCPU thread has to be scheduled back in.
> 
> With this patch performance reaches 60-65% of bare metal and, more
> important, 99% of what you get if you use idle=poll in the guest.  This
> means that the tunable gets rid of this particular bottleneck, and more
> work can be done to improve performance in the kernel or QEMU.
> 
> Of course there is some price to pay; every time an otherwise idle vCPUs
> is interrupted by an interrupt, it will poll unnecessarily and thus
> impose a little load on the host.  The above results were obtained with
> a mostly random value of the parameter (2000000), and the load was around
> 1.5-2.5% CPU usage on one of the host's core for each idle guest vCPU.
> 
> The patch also adds a new stat, /sys/kernel/debug/kvm/halt_successful_poll,
> that can be used to tune the parameter.  It counts how many HLT
> instructions received an interrupt during the polling period; each
> successful poll avoids that Linux schedules the VCPU thread out and back
> in, and may also avoid a likely trip to C1 and back for the physical CPU.

In the long run, this value should probably be auto-tuned.
However, it seems like a good idea to introduce this kind
of thing one step at a time.

> While the VM is idle, a Linux 4 VCPU VM halts around 10 times per second.
> Of these halts, almost all are failed polls.  During the benchmark,
> instead, basically all halts end within the polling period, except a more
> or less constant stream of 50 per second coming from vCPUs that are not
> running the benchmark.  The wasted time is thus very low.  Things may
> be slightly different for Windows VMs, which have a ~10 ms timer tick.
> 
> The effect is also visible on Marcelo's recently-introduced latency
> test for the TSC deadline timer.  Though of course a non-RT kernel has
> awful latency bounds, the latency of the timer is around 8000-10000 clock
> cycles compared to 20000-120000 without setting halt_poll.  For the TSC
> deadline timer, thus, the effect is both a smaller average latency and
> a smaller variance.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Acked-by: Rik van Riel <riel@redhat.com>


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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 16:05 [PATCH RFC] kvm: x86: add halt_poll module parameter Paolo Bonzini
  2015-02-05 17:46 ` Rik van Riel
@ 2015-02-05 17:53 ` Radim Krčmář
  2015-02-05 19:18   ` Paolo Bonzini
  2015-02-05 18:55 ` Jan Kiszka
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Radim Krčmář @ 2015-02-05 17:53 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, riel, mtosatti

2015-02-05 17:05+0100, Paolo Bonzini:
> This patch introduces a new module parameter for the KVM module; when it
> is present, KVM attempts a bit of polling on every HLT before scheduling
> itself out via kvm_vcpu_block.
> 
> This parameter helps a lot for latency-bound workloads---in particular
> I tested it with O_DSYNC writes with a battery-backed disk in the host.
> In this case, writes are fast (because the data doesn't have to go all
> the way to the platters) but they cannot be merged by either the host or
> the guest.  KVM's performance here is usually around 30% of bare metal,
> or 50% if you use cache=directsync or cache=writethrough (these
> parameters avoid that the guest sends pointless flush requests, and
> at the same time they are not slow because of the battery-backed cache).
> The bad performance happens because on every halt the host CPU decides
> to halt itself too.
>                      When the interrupt comes, the vCPU thread is then
> migrated to a new physical CPU,

Unrelated:  are drawbacks of migrating vs waking up evaluated correctly?

>                                 and in general the latency is horrible
> because the vCPU thread has to be scheduled back in.
> 
> With this patch performance reaches 60-65% of bare metal and, more
> important,
>            99% of what you get if you use idle=poll in the guest.

(Hm, I would have thought that this can outperform idle=poll ...)

>                                                                    This
> means that the tunable gets rid of this particular bottleneck, and more
> work can be done to improve performance in the kernel or QEMU.
> 
> Of course there is some price to pay; every time an otherwise idle vCPUs
> is interrupted by an interrupt, it will poll unnecessarily and thus
> impose a little load on the host.  The above results were obtained with
> a mostly random value of the parameter (2000000)

(I guess that translated to about 0.66 ms.)

>                                                   and the load was around
> 1.5-2.5% CPU usage on one of the host's core for each idle guest vCPU.

(10 exits_per_second / 4 VCPUs * 0.0066 second_per_exit = 1.65% load.)

> The patch also adds a new stat, /sys/kernel/debug/kvm/halt_successful_poll,
> that can be used to tune the parameter.  It counts how many HLT
> instructions received an interrupt during the polling period; each
> successful poll avoids that Linux schedules the VCPU thread out and back
> in, and may also avoid a likely trip to C1 and back for the physical CPU.
> 
> While the VM is idle, a Linux 4 VCPU VM halts around 10 times per second.

(Looks like tickless.)

> Of these halts, almost all are failed polls.  During the benchmark,
> instead, basically all halts end within the polling period, except a more
> or less constant stream of 50 per second coming from vCPUs that are not
> running the benchmark.  The wasted time is thus very low.  Things may
> be slightly different for Windows VMs, which have a ~10 ms timer tick.

(Windows userspace can force timer tick down to every 0.5 ms :)

> The effect is also visible on Marcelo's recently-introduced latency
> test for the TSC deadline timer.  Though of course a non-RT kernel has
> awful latency bounds, the latency of the timer is around 8000-10000 clock
> cycles compared to 20000-120000 without setting halt_poll.  For the TSC
> deadline timer, thus, the effect is both a smaller average latency and
> a smaller variance.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

It is going to be hard to balance between performance and wasted idle
time, so it's good to have it disabled by default,
(I guess the value is going to be overestimated when used.)

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>

> +++ b/arch/x86/kvm/x86.c
> @@ -5819,13 +5823,29 @@ void kvm_arch_exit(void)
>  int kvm_emulate_halt(struct kvm_vcpu *vcpu)
> +	if (halt_poll) {
> +		u64 start, curr;
> +		rdtscll(start);

(I agree that all x86 with VMM have TSC.)

> +		do {
> +			/*
> +			 * This sets KVM_REQ_UNHALT if an interrupt
> +			 * arrives.
> +			 */
> +			if (kvm_vcpu_check_block(vcpu) < 0) {
> +				++vcpu->stat.halt_successful_poll;
> +				break;
> +			}
> +			rdtscll(curr);
> +		} while(!need_resched() && curr - start < halt_poll);

(We can get preempted and possibly rescheduled to another CPU.
 With unstable TSC, this could loop longer than was requested;
 likely not by much thanks to unsigned though -- ok -- rare cases
 are ignored.)

> +++ b/virt/kvm/kvm_main.c
> @@ -1813,6 +1813,20 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
>  }
>  EXPORT_SYMBOL_GPL(mark_page_dirty);
>  
> +int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)

(I think the returned 'int' will be wasted on us, so it could be a
 'bool', which would allow a better name ...)

> +{
> +	if (kvm_arch_vcpu_runnable(vcpu)) {
> +		kvm_make_request(KVM_REQ_UNHALT, vcpu);
> +		return -EINTR;
> +	}
> +	if (kvm_cpu_has_pending_timer(vcpu))
> +		return -EINTR;
> +	if (signal_pending(current))
> +		return -EINTR;
> +
> +	return 0;
> +}

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 16:05 [PATCH RFC] kvm: x86: add halt_poll module parameter Paolo Bonzini
  2015-02-05 17:46 ` Rik van Riel
  2015-02-05 17:53 ` Radim Krčmář
@ 2015-02-05 18:55 ` Jan Kiszka
  2015-02-05 19:20   ` Paolo Bonzini
  2015-02-05 20:39 ` David Matlack
  2015-02-05 23:34 ` Marcelo Tosatti
  4 siblings, 1 reply; 13+ messages in thread
From: Jan Kiszka @ 2015-02-05 18:55 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm; +Cc: riel, rkrcmar, mtosatti

On 2015-02-05 17:05, Paolo Bonzini wrote:
> This patch introduces a new module parameter for the KVM module; when it
> is present, KVM attempts a bit of polling on every HLT before scheduling
> itself out via kvm_vcpu_block.

Wouldn't it be better to tune this on a per-VM basis? Think of mixed
workloads with some latency-sensitive and some standard VMs.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 17:53 ` Radim Krčmář
@ 2015-02-05 19:18   ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2015-02-05 19:18 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: linux-kernel, kvm, riel, mtosatti



On 05/02/2015 18:53, Radim Krčmář wrote:
> >            99% of what you get if you use idle=poll in the guest.
> 
> (Hm, I would have thought that this can outperform idle=poll ...)

It outperforms idle=poll in the host.  A vmexit is probably too cheap to
outperform having idle=poll in the guest.

Paolo

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 18:55 ` Jan Kiszka
@ 2015-02-05 19:20   ` Paolo Bonzini
  2015-02-05 19:23     ` Rik van Riel
  0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2015-02-05 19:20 UTC (permalink / raw)
  To: Jan Kiszka, linux-kernel, kvm; +Cc: riel, rkrcmar, mtosatti



On 05/02/2015 19:55, Jan Kiszka wrote:
> > This patch introduces a new module parameter for the KVM module; when it
> > is present, KVM attempts a bit of polling on every HLT before scheduling
> > itself out via kvm_vcpu_block.
>
> Wouldn't it be better to tune this on a per-VM basis? Think of mixed
> workloads with some latency-sensitive and some standard VMs.

Yes, but:

1) this turned out to be very cheap, so a per-host tunable is not too bad;

2) it also affects only very few workloads (for example network
workloads can already do polling in the guest) so it only affects few
people;

3) long term anyway we want it to auto tune, which is better than tuning
it per-VM.

Paolo

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 19:20   ` Paolo Bonzini
@ 2015-02-05 19:23     ` Rik van Riel
  2015-02-05 20:14       ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Rik van Riel @ 2015-02-05 19:23 UTC (permalink / raw)
  To: Paolo Bonzini, Jan Kiszka, linux-kernel, kvm; +Cc: rkrcmar, mtosatti

On 02/05/2015 02:20 PM, Paolo Bonzini wrote:
> 
> 
> On 05/02/2015 19:55, Jan Kiszka wrote:
>>> This patch introduces a new module parameter for the KVM module; when it
>>> is present, KVM attempts a bit of polling on every HLT before scheduling
>>> itself out via kvm_vcpu_block.
>>
>> Wouldn't it be better to tune this on a per-VM basis? Think of mixed
>> workloads with some latency-sensitive and some standard VMs.
> 
> Yes, but:
> 
> 1) this turned out to be very cheap, so a per-host tunable is not too bad;
> 
> 2) it also affects only very few workloads (for example network
> workloads can already do polling in the guest) so it only affects few
> people;
> 
> 3) long term anyway we want it to auto tune, which is better than tuning
> it per-VM.

We may want to auto tune it per VM.

However, if we make auto tuning work well, I do not
think we want to expose a user visible tunable per
VM, and commit to keeping that kind of interface
around forever.


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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 19:23     ` Rik van Riel
@ 2015-02-05 20:14       ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2015-02-05 20:14 UTC (permalink / raw)
  To: Rik van Riel, Jan Kiszka, linux-kernel, kvm; +Cc: rkrcmar, mtosatti



On 05/02/2015 20:23, Rik van Riel wrote:
>> > 3) long term anyway we want it to auto tune, which is better than tuning
>> > it per-VM.
> We may want to auto tune it per VM.

We may even want to auto tune it per VCPU.

> However, if we make auto tuning work well, I do not
> think we want to expose a user visible tunable per
> VM, and commit to keeping that kind of interface
> around forever.

Exactly.  We probably want module parameters to tune the minimum/maximum
values (which includes the special cases of disabling polling
altogether, and disabling the autotuning while leaving polling enabled),
but committing to a per-VM interface is premature.

Paolo

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 16:05 [PATCH RFC] kvm: x86: add halt_poll module parameter Paolo Bonzini
                   ` (2 preceding siblings ...)
  2015-02-05 18:55 ` Jan Kiszka
@ 2015-02-05 20:39 ` David Matlack
  2015-02-05 21:24   ` Paolo Bonzini
  2015-02-05 23:34 ` Marcelo Tosatti
  4 siblings, 1 reply; 13+ messages in thread
From: David Matlack @ 2015-02-05 20:39 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm list, riel, rkrcmar, Marcelo Tosatti

On Thu, Feb 5, 2015 at 8:05 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This patch introduces a new module parameter for the KVM module; when it
> is present, KVM attempts a bit of polling on every HLT before scheduling
> itself out via kvm_vcpu_block.

Awesome. I have been working on the same feature in parallel so I have
some suggestions :)

>
> This parameter helps a lot for latency-bound workloads---in particular
> I tested it with O_DSYNC writes with a battery-backed disk in the host.
> In this case, writes are fast (because the data doesn't have to go all
> the way to the platters) but they cannot be merged by either the host or
> the guest.  KVM's performance here is usually around 30% of bare metal,
> or 50% if you use cache=directsync or cache=writethrough (these
> parameters avoid that the guest sends pointless flush requests, and
> at the same time they are not slow because of the battery-backed cache).
> The bad performance happens because on every halt the host CPU decides
> to halt itself too.  When the interrupt comes, the vCPU thread is then
> migrated to a new physical CPU, and in general the latency is horrible
> because the vCPU thread has to be scheduled back in.
>
> With this patch performance reaches 60-65% of bare metal and, more
> important, 99% of what you get if you use idle=poll in the guest.  This

I used loopback TCP_RR and loopback memcache as benchmarks for halt
polling. I saw very similar results as you (before: 40% bare metal,
after: 60-65% bare metal and 95% of guest idle=poll).

> means that the tunable gets rid of this particular bottleneck, and more
> work can be done to improve performance in the kernel or QEMU.
>
> Of course there is some price to pay; every time an otherwise idle vCPUs
> is interrupted by an interrupt, it will poll unnecessarily and thus
> impose a little load on the host.  The above results were obtained with
> a mostly random value of the parameter (2000000), and the load was around
> 1.5-2.5% CPU usage on one of the host's core for each idle guest vCPU.
>
> The patch also adds a new stat, /sys/kernel/debug/kvm/halt_successful_poll,
> that can be used to tune the parameter.  It counts how many HLT
> instructions received an interrupt during the polling period; each
> successful poll avoids that Linux schedules the VCPU thread out and back
> in, and may also avoid a likely trip to C1 and back for the physical CPU.
>
> While the VM is idle, a Linux 4 VCPU VM halts around 10 times per second.
> Of these halts, almost all are failed polls.  During the benchmark,
> instead, basically all halts end within the polling period, except a more
> or less constant stream of 50 per second coming from vCPUs that are not
> running the benchmark.  The wasted time is thus very low.  Things may
> be slightly different for Windows VMs, which have a ~10 ms timer tick.
>
> The effect is also visible on Marcelo's recently-introduced latency
> test for the TSC deadline timer.  Though of course a non-RT kernel has
> awful latency bounds, the latency of the timer is around 8000-10000 clock
> cycles compared to 20000-120000 without setting halt_poll.  For the TSC
> deadline timer, thus, the effect is both a smaller average latency and
> a smaller variance.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Reviewed-by: David Matlack <dmatlack@google.com>

>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/x86.c              | 28 ++++++++++++++++++++++++----
>  include/linux/kvm_host.h        |  1 +
>  virt/kvm/kvm_main.c             | 22 +++++++++++++++-------
>  4 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 848947ac6ade..a236e39cc385 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -655,6 +655,7 @@ struct kvm_vcpu_stat {
>         u32 irq_window_exits;
>         u32 nmi_window_exits;
>         u32 halt_exits;
> +       u32 halt_successful_poll;
>         u32 halt_wakeup;
>         u32 request_irq_exits;
>         u32 irq_exits;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1373e04e1f19..b7b20828f01c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -96,6 +96,9 @@ EXPORT_SYMBOL_GPL(kvm_x86_ops);
>  static bool ignore_msrs = 0;
>  module_param(ignore_msrs, bool, S_IRUGO | S_IWUSR);
>
> +unsigned int halt_poll = 0;
> +module_param(halt_poll, uint, S_IRUGO | S_IWUSR);

Suggest encoding the units in the name. "halt_poll_cycles" in this case.

> +
>  unsigned int min_timer_period_us = 500;
>  module_param(min_timer_period_us, uint, S_IRUGO | S_IWUSR);
>
> @@ -145,6 +148,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>         { "irq_window", VCPU_STAT(irq_window_exits) },
>         { "nmi_window", VCPU_STAT(nmi_window_exits) },
>         { "halt_exits", VCPU_STAT(halt_exits) },
> +       { "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
>         { "halt_wakeup", VCPU_STAT(halt_wakeup) },
>         { "hypercalls", VCPU_STAT(hypercalls) },
>         { "request_irq", VCPU_STAT(request_irq_exits) },
> @@ -5819,13 +5823,29 @@ void kvm_arch_exit(void)
>  int kvm_emulate_halt(struct kvm_vcpu *vcpu)
>  {
>         ++vcpu->stat.halt_exits;
> -       if (irqchip_in_kernel(vcpu->kvm)) {
> -               vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
> -               return 1;
> -       } else {
> +       if (!irqchip_in_kernel(vcpu->kvm)) {
>                 vcpu->run->exit_reason = KVM_EXIT_HLT;
>                 return 0;
>         }
> +
> +       vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
> +       if (halt_poll) {

Would it be useful to poll in kvm_vcpu_block() for the benefit of all
arch's?

> +               u64 start, curr;
> +               rdtscll(start);

Why cycles instead of time?

> +               do {
> +                       /*
> +                        * This sets KVM_REQ_UNHALT if an interrupt
> +                        * arrives.
> +                        */
> +                       if (kvm_vcpu_check_block(vcpu) < 0) {
> +                               ++vcpu->stat.halt_successful_poll;
> +                               break;
> +                       }
> +                       rdtscll(curr);
> +               } while(!need_resched() && curr - start < halt_poll);

I found that using need_resched() was not sufficient at preventing
VCPUs from delaying their own progress. To test this try running with
and without polling on a 2 VCPU VM, confined to 1 PCPU, that is running
loopback TCP_RR in the VM. The problem goes away if you stop polling as
soon as there are runnable threads on your cpu. (e.g. use
"single_task_running()" instead of "!need_resched()"
http://lxr.free-electrons.com/source/kernel/sched/core.c#L2398 ). This
also guarantees polling only delays the idle thread.

> +       }
> +
> +       return 1;
>  }
>  EXPORT_SYMBOL_GPL(kvm_emulate_halt);
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 8a82838034f1..1519d48d956f 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -584,6 +584,7 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
>  unsigned long kvm_host_page_size(struct kvm *kvm, gfn_t gfn);
>  void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
>
> +int kvm_vcpu_check_block(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_block(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
>  int kvm_vcpu_yield_to(struct kvm_vcpu *target);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 0c281760a1c5..825fc3ec0509 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1813,6 +1813,20 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
>  }
>  EXPORT_SYMBOL_GPL(mark_page_dirty);
>
> +int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)
> +{
> +       if (kvm_arch_vcpu_runnable(vcpu)) {
> +               kvm_make_request(KVM_REQ_UNHALT, vcpu);
> +               return -EINTR;
> +       }
> +       if (kvm_cpu_has_pending_timer(vcpu))
> +               return -EINTR;
> +       if (signal_pending(current))
> +               return -EINTR;
> +
> +       return 0;
> +}
> +
>  /*
>   * The vCPU has executed a HLT instruction with in-kernel mode enabled.
>   */
> @@ -1823,13 +1837,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
>         for (;;) {
>                 prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
>
> -               if (kvm_arch_vcpu_runnable(vcpu)) {
> -                       kvm_make_request(KVM_REQ_UNHALT, vcpu);
> -                       break;
> -               }
> -               if (kvm_cpu_has_pending_timer(vcpu))
> -                       break;
> -               if (signal_pending(current))
> +               if (kvm_vcpu_check_block(vcpu) < 0)
>                         break;
>
>                 schedule();
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 20:39 ` David Matlack
@ 2015-02-05 21:24   ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2015-02-05 21:24 UTC (permalink / raw)
  To: David Matlack; +Cc: linux-kernel, kvm list, riel, rkrcmar, Marcelo Tosatti



On 05/02/2015 21:39, David Matlack wrote:
>> This parameter helps a lot for latency-bound workloads [...]
>> KVM's performance here is usually around 30% of bare metal,
>> or 50% if you use cache=directsync or cache=writethrough.
>> With this patch performance reaches 60-65% of bare metal and, more
>> important, 99% of what you get if you use idle=poll in the guest.
> 
> I used loopback TCP_RR and loopback memcache as benchmarks for halt
> polling. I saw very similar results as you (before: 40% bare metal,
> after: 60-65% bare metal and 95% of guest idle=poll).

Good that it also works for network!

> Reviewed-by: David Matlack <dmatlack@google.com>
> 
>>  arch/x86/include/asm/kvm_host.h |  1 +
>>  arch/x86/kvm/x86.c              | 28 ++++++++++++++++++++++++----
>>  include/linux/kvm_host.h        |  1 +
>>  virt/kvm/kvm_main.c             | 22 +++++++++++++++-------
>>  4 files changed, 41 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index 848947ac6ade..a236e39cc385 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -655,6 +655,7 @@ struct kvm_vcpu_stat {
>>         u32 irq_window_exits;
>>         u32 nmi_window_exits;
>>         u32 halt_exits;
>> +       u32 halt_successful_poll;
>>         u32 halt_wakeup;
>>         u32 request_irq_exits;
>>         u32 irq_exits;
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 1373e04e1f19..b7b20828f01c 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -96,6 +96,9 @@ EXPORT_SYMBOL_GPL(kvm_x86_ops);
>>  static bool ignore_msrs = 0;
>>  module_param(ignore_msrs, bool, S_IRUGO | S_IWUSR);
>>
>> +unsigned int halt_poll = 0;
>> +module_param(halt_poll, uint, S_IRUGO | S_IWUSR);
> 
> Suggest encoding the units in the name. "halt_poll_cycles" in this case.

I left it out because of the parallel with ple_window/ple_gap.  But I
will call it "halt_poll_ns" in the next version.

>> +
>>  unsigned int min_timer_period_us = 500;
>>  module_param(min_timer_period_us, uint, S_IRUGO | S_IWUSR);
>>
>> @@ -145,6 +148,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>>         { "irq_window", VCPU_STAT(irq_window_exits) },
>>         { "nmi_window", VCPU_STAT(nmi_window_exits) },
>>         { "halt_exits", VCPU_STAT(halt_exits) },
>> +       { "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
>>         { "halt_wakeup", VCPU_STAT(halt_wakeup) },
>>         { "hypercalls", VCPU_STAT(hypercalls) },
>>         { "request_irq", VCPU_STAT(request_irq_exits) },
>> @@ -5819,13 +5823,29 @@ void kvm_arch_exit(void)
>>  int kvm_emulate_halt(struct kvm_vcpu *vcpu)
>>  {
>>         ++vcpu->stat.halt_exits;
>> -       if (irqchip_in_kernel(vcpu->kvm)) {
>> -               vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
>> -               return 1;
>> -       } else {
>> +       if (!irqchip_in_kernel(vcpu->kvm)) {
>>                 vcpu->run->exit_reason = KVM_EXIT_HLT;
>>                 return 0;
>>         }
>> +
>> +       vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
>> +       if (halt_poll) {
> 
> Would it be useful to poll in kvm_vcpu_block() for the benefit of all
> arch's?

Sure.  Especially if I use time instead of cycles.

>> +               u64 start, curr;
>> +               rdtscll(start);
> 
> Why cycles instead of time?

People's love for rdtsc grows every time you tell them it's wrong...

>> +               do {
>> +                       /*
>> +                        * This sets KVM_REQ_UNHALT if an interrupt
>> +                        * arrives.
>> +                        */
>> +                       if (kvm_vcpu_check_block(vcpu) < 0) {
>> +                               ++vcpu->stat.halt_successful_poll;
>> +                               break;
>> +                       }
>> +                       rdtscll(curr);
>> +               } while(!need_resched() && curr - start < halt_poll);
> 
> I found that using need_resched() was not sufficient at preventing
> VCPUs from delaying their own progress. To test this try running with
> and without polling on a 2 VCPU VM, confined to 1 PCPU, that is running
> loopback TCP_RR in the VM. The problem goes away if you stop polling as
> soon as there are runnable threads on your cpu. (e.g. use
> "single_task_running()" instead of "!need_resched()"
> http://lxr.free-electrons.com/source/kernel/sched/core.c#L2398 ). This
> also guarantees polling only delays the idle thread.

Great, I'll include all of your suggestions in the next version of the
patch.

Paolo

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 16:05 [PATCH RFC] kvm: x86: add halt_poll module parameter Paolo Bonzini
                   ` (3 preceding siblings ...)
  2015-02-05 20:39 ` David Matlack
@ 2015-02-05 23:34 ` Marcelo Tosatti
  2015-02-05 23:47   ` Marcelo Tosatti
  2015-02-06  7:50   ` Paolo Bonzini
  4 siblings, 2 replies; 13+ messages in thread
From: Marcelo Tosatti @ 2015-02-05 23:34 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, riel, rkrcmar

On Thu, Feb 05, 2015 at 05:05:25PM +0100, Paolo Bonzini wrote:
> This patch introduces a new module parameter for the KVM module; when it
> is present, KVM attempts a bit of polling on every HLT before scheduling
> itself out via kvm_vcpu_block.
> 
> This parameter helps a lot for latency-bound workloads---in particular
> I tested it with O_DSYNC writes with a battery-backed disk in the host.
> In this case, writes are fast (because the data doesn't have to go all
> the way to the platters) but they cannot be merged by either the host or
> the guest.  KVM's performance here is usually around 30% of bare metal,
> or 50% if you use cache=directsync or cache=writethrough (these
> parameters avoid that the guest sends pointless flush requests, and
> at the same time they are not slow because of the battery-backed cache).
> The bad performance happens because on every halt the host CPU decides
> to halt itself too.  When the interrupt comes, the vCPU thread is then
> migrated to a new physical CPU, and in general the latency is horrible
> because the vCPU thread has to be scheduled back in.
> 
> With this patch performance reaches 60-65% of bare metal and, more
> important, 99% of what you get if you use idle=poll in the guest.  This
> means that the tunable gets rid of this particular bottleneck, and more
> work can be done to improve performance in the kernel or QEMU.
> 
> Of course there is some price to pay; every time an otherwise idle vCPUs
> is interrupted by an interrupt, it will poll unnecessarily and thus
> impose a little load on the host.  The above results were obtained with
> a mostly random value of the parameter (2000000), and the load was around
> 1.5-2.5% CPU usage on one of the host's core for each idle guest vCPU.
> 
> The patch also adds a new stat, /sys/kernel/debug/kvm/halt_successful_poll,
> that can be used to tune the parameter.  It counts how many HLT
> instructions received an interrupt during the polling period; each
> successful poll avoids that Linux schedules the VCPU thread out and back
> in, and may also avoid a likely trip to C1 and back for the physical CPU.
> 
> While the VM is idle, a Linux 4 VCPU VM halts around 10 times per second.
> Of these halts, almost all are failed polls.  During the benchmark,
> instead, basically all halts end within the polling period, except a more
> or less constant stream of 50 per second coming from vCPUs that are not
> running the benchmark.  The wasted time is thus very low.  Things may
> be slightly different for Windows VMs, which have a ~10 ms timer tick.
> 
> The effect is also visible on Marcelo's recently-introduced latency
> test for the TSC deadline timer.  Though of course a non-RT kernel has
> awful latency bounds, the latency of the timer is around 8000-10000 clock
> cycles compared to 20000-120000 without setting halt_poll.  For the TSC
> deadline timer, thus, the effect is both a smaller average latency and
> a smaller variance.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/x86.c              | 28 ++++++++++++++++++++++++----
>  include/linux/kvm_host.h        |  1 +
>  virt/kvm/kvm_main.c             | 22 +++++++++++++++-------
>  4 files changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 848947ac6ade..a236e39cc385 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -655,6 +655,7 @@ struct kvm_vcpu_stat {
>  	u32 irq_window_exits;
>  	u32 nmi_window_exits;
>  	u32 halt_exits;
> +	u32 halt_successful_poll;
>  	u32 halt_wakeup;
>  	u32 request_irq_exits;
>  	u32 irq_exits;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1373e04e1f19..b7b20828f01c 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -96,6 +96,9 @@ EXPORT_SYMBOL_GPL(kvm_x86_ops);
>  static bool ignore_msrs = 0;
>  module_param(ignore_msrs, bool, S_IRUGO | S_IWUSR);
>  
> +unsigned int halt_poll = 0;
> +module_param(halt_poll, uint, S_IRUGO | S_IWUSR);
> +
>  unsigned int min_timer_period_us = 500;
>  module_param(min_timer_period_us, uint, S_IRUGO | S_IWUSR);
>  
> @@ -145,6 +148,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>  	{ "irq_window", VCPU_STAT(irq_window_exits) },
>  	{ "nmi_window", VCPU_STAT(nmi_window_exits) },
>  	{ "halt_exits", VCPU_STAT(halt_exits) },
> +	{ "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
>  	{ "halt_wakeup", VCPU_STAT(halt_wakeup) },
>  	{ "hypercalls", VCPU_STAT(hypercalls) },
>  	{ "request_irq", VCPU_STAT(request_irq_exits) },
> @@ -5819,13 +5823,29 @@ void kvm_arch_exit(void)
>  int kvm_emulate_halt(struct kvm_vcpu *vcpu)
>  {
>  	++vcpu->stat.halt_exits;
> -	if (irqchip_in_kernel(vcpu->kvm)) {
> -		vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
> -		return 1;
> -	} else {
> +	if (!irqchip_in_kernel(vcpu->kvm)) {
>  		vcpu->run->exit_reason = KVM_EXIT_HLT;
>  		return 0;
>  	}
> +
> +	vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
> +	if (halt_poll) {
> +		u64 start, curr;
> +		rdtscll(start);
> +		do {
> +			/*
> +			 * This sets KVM_REQ_UNHALT if an interrupt
> +			 * arrives.
> +			 */
> +			if (kvm_vcpu_check_block(vcpu) < 0) {
> +				++vcpu->stat.halt_successful_poll;
> +				break;
> +			}
> +			rdtscll(curr);
> +		} while(!need_resched() && curr - start < halt_poll);
> +	}
> +
> +	return 1;
>  }

You want at least a basic procedure to estimate a value
(its a function of the device after all).

Rather than halt_successful_poll's, i suppose the optimum 
can be estimated from a dataset containing entries
in the form:

interrupt time - hlt time

Then choose a given value from that table.

You can get the same out of halt_successful_poll, 
but requires multiple runs of the test:

Set halt_poll, run test, record halt_successful_poll.
Set halt_poll, run test, record halt_successful_poll.
Set halt_poll, run test, record halt_successful_poll.
...

A crude histogram also works, to avoid recording all "interrupt time -
hlt" entries and processing them in userspace.


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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 23:34 ` Marcelo Tosatti
@ 2015-02-05 23:47   ` Marcelo Tosatti
  2015-02-06  7:50   ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Marcelo Tosatti @ 2015-02-05 23:47 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, riel, rkrcmar

On Thu, Feb 05, 2015 at 09:34:06PM -0200, Marcelo Tosatti wrote:
> On Thu, Feb 05, 2015 at 05:05:25PM +0100, Paolo Bonzini wrote:
> > This patch introduces a new module parameter for the KVM module; when it
> > is present, KVM attempts a bit of polling on every HLT before scheduling
> > itself out via kvm_vcpu_block.
> > 
> > This parameter helps a lot for latency-bound workloads---in particular
> > I tested it with O_DSYNC writes with a battery-backed disk in the host.
> > In this case, writes are fast (because the data doesn't have to go all
> > the way to the platters) but they cannot be merged by either the host or
> > the guest.  KVM's performance here is usually around 30% of bare metal,
> > or 50% if you use cache=directsync or cache=writethrough (these
> > parameters avoid that the guest sends pointless flush requests, and
> > at the same time they are not slow because of the battery-backed cache).
> > The bad performance happens because on every halt the host CPU decides
> > to halt itself too.  When the interrupt comes, the vCPU thread is then
> > migrated to a new physical CPU, and in general the latency is horrible
> > because the vCPU thread has to be scheduled back in.
> > 
> > With this patch performance reaches 60-65% of bare metal and, more
> > important, 99% of what you get if you use idle=poll in the guest.  This
> > means that the tunable gets rid of this particular bottleneck, and more
> > work can be done to improve performance in the kernel or QEMU.
> > 
> > Of course there is some price to pay; every time an otherwise idle vCPUs
> > is interrupted by an interrupt, it will poll unnecessarily and thus
> > impose a little load on the host.  The above results were obtained with
> > a mostly random value of the parameter (2000000), and the load was around
> > 1.5-2.5% CPU usage on one of the host's core for each idle guest vCPU.
> > 
> > The patch also adds a new stat, /sys/kernel/debug/kvm/halt_successful_poll,
> > that can be used to tune the parameter.  It counts how many HLT
> > instructions received an interrupt during the polling period; each
> > successful poll avoids that Linux schedules the VCPU thread out and back
> > in, and may also avoid a likely trip to C1 and back for the physical CPU.
> > 
> > While the VM is idle, a Linux 4 VCPU VM halts around 10 times per second.
> > Of these halts, almost all are failed polls.  During the benchmark,
> > instead, basically all halts end within the polling period, except a more
> > or less constant stream of 50 per second coming from vCPUs that are not
> > running the benchmark.  The wasted time is thus very low.  Things may
> > be slightly different for Windows VMs, which have a ~10 ms timer tick.
> > 
> > The effect is also visible on Marcelo's recently-introduced latency
> > test for the TSC deadline timer.  Though of course a non-RT kernel has
> > awful latency bounds, the latency of the timer is around 8000-10000 clock
> > cycles compared to 20000-120000 without setting halt_poll.  For the TSC
> > deadline timer, thus, the effect is both a smaller average latency and
> > a smaller variance.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h |  1 +
> >  arch/x86/kvm/x86.c              | 28 ++++++++++++++++++++++++----
> >  include/linux/kvm_host.h        |  1 +
> >  virt/kvm/kvm_main.c             | 22 +++++++++++++++-------
> >  4 files changed, 41 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 848947ac6ade..a236e39cc385 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -655,6 +655,7 @@ struct kvm_vcpu_stat {
> >  	u32 irq_window_exits;
> >  	u32 nmi_window_exits;
> >  	u32 halt_exits;
> > +	u32 halt_successful_poll;
> >  	u32 halt_wakeup;
> >  	u32 request_irq_exits;
> >  	u32 irq_exits;
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 1373e04e1f19..b7b20828f01c 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -96,6 +96,9 @@ EXPORT_SYMBOL_GPL(kvm_x86_ops);
> >  static bool ignore_msrs = 0;
> >  module_param(ignore_msrs, bool, S_IRUGO | S_IWUSR);
> >  
> > +unsigned int halt_poll = 0;
> > +module_param(halt_poll, uint, S_IRUGO | S_IWUSR);
> > +
> >  unsigned int min_timer_period_us = 500;
> >  module_param(min_timer_period_us, uint, S_IRUGO | S_IWUSR);
> >  
> > @@ -145,6 +148,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
> >  	{ "irq_window", VCPU_STAT(irq_window_exits) },
> >  	{ "nmi_window", VCPU_STAT(nmi_window_exits) },
> >  	{ "halt_exits", VCPU_STAT(halt_exits) },
> > +	{ "halt_successful_poll", VCPU_STAT(halt_successful_poll) },
> >  	{ "halt_wakeup", VCPU_STAT(halt_wakeup) },
> >  	{ "hypercalls", VCPU_STAT(hypercalls) },
> >  	{ "request_irq", VCPU_STAT(request_irq_exits) },
> > @@ -5819,13 +5823,29 @@ void kvm_arch_exit(void)
> >  int kvm_emulate_halt(struct kvm_vcpu *vcpu)
> >  {
> >  	++vcpu->stat.halt_exits;
> > -	if (irqchip_in_kernel(vcpu->kvm)) {
> > -		vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
> > -		return 1;
> > -	} else {
> > +	if (!irqchip_in_kernel(vcpu->kvm)) {
> >  		vcpu->run->exit_reason = KVM_EXIT_HLT;
> >  		return 0;
> >  	}
> > +
> > +	vcpu->arch.mp_state = KVM_MP_STATE_HALTED;
> > +	if (halt_poll) {
> > +		u64 start, curr;
> > +		rdtscll(start);
> > +		do {
> > +			/*
> > +			 * This sets KVM_REQ_UNHALT if an interrupt
> > +			 * arrives.
> > +			 */
> > +			if (kvm_vcpu_check_block(vcpu) < 0) {
> > +				++vcpu->stat.halt_successful_poll;
> > +				break;
> > +			}
> > +			rdtscll(curr);
> > +		} while(!need_resched() && curr - start < halt_poll);
> > +	}
> > +
> > +	return 1;
> >  }
> 
> You want at least a basic procedure to estimate a value
> (its a function of the device after all).
> 
> Rather than halt_successful_poll's, i suppose the optimum 
> can be estimated from a dataset containing entries
> in the form:
> 
> interrupt time - hlt time
> 
> Then choose a given value from that table.

I meant a given percentile, with stats compiled from that table.

> You can get the same out of halt_successful_poll, 
> but requires multiple runs of the test:
> 
> Set halt_poll, run test, record halt_successful_poll.
> Set halt_poll, run test, record halt_successful_poll.
> Set halt_poll, run test, record halt_successful_poll.
> ...
> 
> A crude histogram also works, to avoid recording all "interrupt time -
> hlt" entries and processing them in userspace.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH RFC] kvm: x86: add halt_poll module parameter
  2015-02-05 23:34 ` Marcelo Tosatti
  2015-02-05 23:47   ` Marcelo Tosatti
@ 2015-02-06  7:50   ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2015-02-06  7:50 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: linux-kernel, kvm, riel, rkrcmar



On 06/02/2015 00:34, Marcelo Tosatti wrote:
> You want at least a basic procedure to estimate a value
> (its a function of the device after all).

I will add a tracepoint.

> Rather than halt_successful_poll's, i suppose the optimum 
> can be estimated from a dataset containing entries
> in the form:
> 
> interrupt time - hlt time
> 
> Then choose a given value from that table.

Yes.

Paolo

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

end of thread, other threads:[~2015-02-06  7:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-05 16:05 [PATCH RFC] kvm: x86: add halt_poll module parameter Paolo Bonzini
2015-02-05 17:46 ` Rik van Riel
2015-02-05 17:53 ` Radim Krčmář
2015-02-05 19:18   ` Paolo Bonzini
2015-02-05 18:55 ` Jan Kiszka
2015-02-05 19:20   ` Paolo Bonzini
2015-02-05 19:23     ` Rik van Riel
2015-02-05 20:14       ` Paolo Bonzini
2015-02-05 20:39 ` David Matlack
2015-02-05 21:24   ` Paolo Bonzini
2015-02-05 23:34 ` Marcelo Tosatti
2015-02-05 23:47   ` Marcelo Tosatti
2015-02-06  7:50   ` Paolo Bonzini

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