From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH RFC] kvm: x86: add halt_poll module parameter Date: Thu, 5 Feb 2015 18:53:44 +0100 Message-ID: <20150205175344.GB14367@potion.redhat.com> References: <1423152325-5094-1-git-send-email-pbonzini@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, riel@redhat.com, mtosatti@redhat.com To: Paolo Bonzini Return-path: Content-Disposition: inline In-Reply-To: <1423152325-5094-1-git-send-email-pbonzini@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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 schedul= ing > itself out via kvm_vcpu_block. >=20 > This parameter helps a lot for latency-bound workloads---in particula= r > I tested it with O_DSYNC writes with a battery-backed disk in the hos= t. > In this case, writes are fast (because the data doesn't have to go al= l > 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 meta= l, > or 50% if you use cache=3Ddirectsync or cache=3Dwritethrough (these > parameters avoid that the guest sends pointless flush requests, and > at the same time they are not slow because of the battery-backed cach= e). > The bad performance happens because on every halt the host CPU decide= s > to halt itself too. > When the interrupt comes, the vCPU thread is the= n > migrated to a new physical CPU, Unrelated: are drawbacks of migrating vs waking up evaluated correctly= ? > and in general the latency is horribl= e > because the vCPU thread has to be scheduled back in. >=20 > With this patch performance reaches 60-65% of bare metal and, more > important, > 99% of what you get if you use idle=3Dpoll in the guest. (Hm, I would have thought that this can outperform idle=3Dpoll ...) > Th= is > means that the tunable gets rid of this particular bottleneck, and mo= re > work can be done to improve performance in the kernel or QEMU. >=20 > Of course there is some price to pay; every time an otherwise idle vC= PUs > is interrupted by an interrupt, it will poll unnecessarily and thus > impose a little load on the host. The above results were obtained wi= th > a mostly random value of the parameter (2000000) (I guess that translated to about 0.66 ms.) > and the load was ar= ound > 1.5-2.5% CPU usage on one of the host's core for each idle guest vCPU= =2E (10 exits_per_second / 4 VCPUs * 0.0066 second_per_exit =3D 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 b= ack > in, and may also avoid a likely trip to C1 and back for the physical = CPU. >=20 > While the VM is idle, a Linux 4 VCPU VM halts around 10 times per sec= ond. (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 n= ot > 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= =2E (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 ha= s > awful latency bounds, the latency of the timer is around 8000-10000 c= lock > cycles compared to 20000-120000 without setting halt_poll. For the T= SC > deadline timer, thus, the effect is both a smaller average latency an= d > a smaller variance. >=20 > Signed-off-by: Paolo Bonzini > --- 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=C4=8Dm=C3=A1=C5=99 > +++ 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 gf= n) > } > EXPORT_SYMBOL_GPL(mark_page_dirty); > =20 > +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; > +}