| Message ID | 20210925005528.1145584-4-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Halt-polling fixes, cleanups and a new stat | expand |
On Fri, Sep 24, 2021 at 05:55:17PM -0700, Sean Christopherson wrote: > Add a comment to document that halt-polling is considered successful even > if the polling loop itself didn't detect a wake event, i.e. if a wake > event was detect in the final kvm_vcpu_check_block(). Invert the param > to the update helper so that the helper is a dumb function that is "told" > whether or not polling was successful, as opposed to having it determinine > success/failure based on blocking behavior. > > Opportunistically tweak the params to the update helper to reduce the > line length for the call site so that it fits on a single line, and so > that the prototype conforms to the more traditional kernel style. > > No functional change intended. > > Signed-off-by: Sean Christopherson <seanjc@google.com> Reviewed-by: David Matlack <dmatlack@google.com> > --- > virt/kvm/kvm_main.c | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 8b33f5045b4d..12fe91a0a4c8 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3199,13 +3199,15 @@ static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu) > return ret; > } > > -static inline void > -update_halt_poll_stats(struct kvm_vcpu *vcpu, u64 poll_ns, bool waited) > +static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start, > + ktime_t end, bool success) > { > - if (waited) > - vcpu->stat.generic.halt_poll_fail_ns += poll_ns; > - else > + u64 poll_ns = ktime_to_ns(ktime_sub(end, start)); > + > + if (success) > vcpu->stat.generic.halt_poll_success_ns += poll_ns; > + else > + vcpu->stat.generic.halt_poll_fail_ns += poll_ns; > } > > /* > @@ -3274,9 +3276,13 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) > kvm_arch_vcpu_unblocking(vcpu); > block_ns = ktime_to_ns(cur) - ktime_to_ns(start); > > + /* > + * Note, halt-polling is considered successful so long as the vCPU was > + * never actually scheduled out, i.e. even if the wake event arrived > + * after of the halt-polling loop itself, but before the full wait. > + */ > if (do_halt_poll) > - update_halt_poll_stats( > - vcpu, ktime_to_ns(ktime_sub(poll_end, start)), waited); > + update_halt_poll_stats(vcpu, start, poll_end, !waited); > > if (halt_poll_allowed) { > if (!vcpu_valid_wakeup(vcpu)) { > -- > 2.33.0.685.g46640cef36-goog >
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 8b33f5045b4d..12fe91a0a4c8 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3199,13 +3199,15 @@ static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu) return ret; } -static inline void -update_halt_poll_stats(struct kvm_vcpu *vcpu, u64 poll_ns, bool waited) +static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start, + ktime_t end, bool success) { - if (waited) - vcpu->stat.generic.halt_poll_fail_ns += poll_ns; - else + u64 poll_ns = ktime_to_ns(ktime_sub(end, start)); + + if (success) vcpu->stat.generic.halt_poll_success_ns += poll_ns; + else + vcpu->stat.generic.halt_poll_fail_ns += poll_ns; } /* @@ -3274,9 +3276,13 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) kvm_arch_vcpu_unblocking(vcpu); block_ns = ktime_to_ns(cur) - ktime_to_ns(start); + /* + * Note, halt-polling is considered successful so long as the vCPU was + * never actually scheduled out, i.e. even if the wake event arrived + * after of the halt-polling loop itself, but before the full wait. + */ if (do_halt_poll) - update_halt_poll_stats( - vcpu, ktime_to_ns(ktime_sub(poll_end, start)), waited); + update_halt_poll_stats(vcpu, start, poll_end, !waited); if (halt_poll_allowed) { if (!vcpu_valid_wakeup(vcpu)) {
Add a comment to document that halt-polling is considered successful even if the polling loop itself didn't detect a wake event, i.e. if a wake event was detect in the final kvm_vcpu_check_block(). Invert the param to the update helper so that the helper is a dumb function that is "told" whether or not polling was successful, as opposed to having it determinine success/failure based on blocking behavior. Opportunistically tweak the params to the update helper to reduce the line length for the call site so that it fits on a single line, and so that the prototype conforms to the more traditional kernel style. No functional change intended. Signed-off-by: Sean Christopherson <seanjc@google.com> --- virt/kvm/kvm_main.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)