Message ID | 20211009021236.4122790-17-seanjc@google.com |
---|---|
State | New |
Headers | show |
Series | KVM: Halt-polling and x86 APICv overhaul | expand |
On 09/10/21 04:12, Sean Christopherson wrote: > Calculate the halt-polling "stop" time using "cur" instead of redoing > ktime_get(). In the happy case where hardware correctly predicts > do_halt_poll, "cur" is only a few cycles old. And if the branch is > mispredicted, arguably that extra latency should count toward the > halt-polling time. > > In all likelihood, the numbers involved are in the noise and either > approach is perfectly ok. Using "start" makes the change even more obvious, so: Calculate the halt-polling "stop" time using "start" instead of redoing ktime_get(). In practice, the numbers involved are in the noise (e.g., in the happy case where hardware correctly predicts do_halt_poll and there are no interrupts, "start" is probably only a few cycles old) and either approach is perfectly ok. But it's more precise to count any extra latency toward the halt-polling time. Paolo
On Mon, 2021-10-25 at 16:26 +0200, Paolo Bonzini wrote: > On 09/10/21 04:12, Sean Christopherson wrote: > > Calculate the halt-polling "stop" time using "cur" instead of redoing > > ktime_get(). In the happy case where hardware correctly predicts > > do_halt_poll, "cur" is only a few cycles old. And if the branch is > > mispredicted, arguably that extra latency should count toward the > > halt-polling time. > > > > In all likelihood, the numbers involved are in the noise and either > > approach is perfectly ok. > > Using "start" makes the change even more obvious, so: > > Calculate the halt-polling "stop" time using "start" instead of redoing > ktime_get(). In practice, the numbers involved are in the noise (e.g., > in the happy case where hardware correctly predicts do_halt_poll and > there are no interrupts, "start" is probably only a few cycles old) > and either approach is perfectly ok. But it's more precise to count > any extra latency toward the halt-polling time. > > Paolo > Agreed. Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index a36ccdc93a72..481e8178b43d 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3272,7 +3272,7 @@ void kvm_vcpu_halt(struct kvm_vcpu *vcpu) start = cur = poll_end = ktime_get(); if (do_halt_poll) { - ktime_t stop = ktime_add_ns(ktime_get(), vcpu->halt_poll_ns); + ktime_t stop = ktime_add_ns(cur, vcpu->halt_poll_ns); do { /*