| Message ID | 20210706180350.2838127-5-jingzhangos@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Linear and Logarithmic histogram statistics | expand |
On Tue, Jul 06, 2021 at 06:03:50PM +0000, Jing Zhang wrote: > Add simple stats halt_wait_ns to record the time a VCPU has spent on > waiting for all architectures (not just powerpc). > Add three log histogram stats to record the distribution of time spent > on successful polling, failed polling and VCPU wait. > halt_poll_success_hist: Distribution of time spent before a successful > polling. > halt_poll_fail_hist: Distribution of time spent before a failed polling. > halt_wait_hist: Distribution of time a VCPU has spent on waiting. > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > --- > arch/powerpc/include/asm/kvm_host.h | 1 - > arch/powerpc/kvm/book3s.c | 1 - > arch/powerpc/kvm/book3s_hv.c | 20 +++++++++++++++++--- > arch/powerpc/kvm/booke.c | 1 - > include/linux/kvm_host.h | 9 ++++++++- > include/linux/kvm_types.h | 4 ++++ > virt/kvm/kvm_main.c | 19 +++++++++++++++++++ > 7 files changed, 48 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > index 9f52f282b1aa..4931d03e5799 100644 > --- a/arch/powerpc/include/asm/kvm_host.h > +++ b/arch/powerpc/include/asm/kvm_host.h > @@ -103,7 +103,6 @@ struct kvm_vcpu_stat { > u64 emulated_inst_exits; > u64 dec_exits; > u64 ext_intr_exits; > - u64 halt_wait_ns; The halt_wait_ns refactor should be a separate patch. > u64 halt_successful_wait; > u64 dbell_exits; > u64 gdbell_exits; > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > index 5cc6e90095b0..b785f6772391 100644 > --- a/arch/powerpc/kvm/book3s.c > +++ b/arch/powerpc/kvm/book3s.c > @@ -69,7 +69,6 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = { > STATS_DESC_COUNTER(VCPU, emulated_inst_exits), > STATS_DESC_COUNTER(VCPU, dec_exits), > STATS_DESC_COUNTER(VCPU, ext_intr_exits), > - STATS_DESC_TIME_NSEC(VCPU, halt_wait_ns), > STATS_DESC_COUNTER(VCPU, halt_successful_wait), > STATS_DESC_COUNTER(VCPU, dbell_exits), > STATS_DESC_COUNTER(VCPU, gdbell_exits), > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > index cd544a46183e..103f998cee75 100644 > --- a/arch/powerpc/kvm/book3s_hv.c > +++ b/arch/powerpc/kvm/book3s_hv.c > @@ -4144,19 +4144,33 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc) > > /* Attribute wait time */ > if (do_sleep) { > - vc->runner->stat.halt_wait_ns += > + vc->runner->stat.generic.halt_wait_ns += > ktime_to_ns(cur) - ktime_to_ns(start_wait); > + kvm_stats_log_hist_update( > + vc->runner->stat.generic.halt_wait_hist, > + LOGHIST_SIZE_LARGE, > + ktime_to_ns(cur) - ktime_to_ns(start_wait)); > /* Attribute failed poll time */ > - if (vc->halt_poll_ns) > + if (vc->halt_poll_ns) { > vc->runner->stat.generic.halt_poll_fail_ns += > ktime_to_ns(start_wait) - > ktime_to_ns(start_poll); > + kvm_stats_log_hist_update( > + vc->runner->stat.generic.halt_poll_fail_hist, > + LOGHIST_SIZE_LARGE, ktime_to_ns(start_wait) - > + ktime_to_ns(start_poll)); > + } > } else { > /* Attribute successful poll time */ > - if (vc->halt_poll_ns) > + if (vc->halt_poll_ns) { > vc->runner->stat.generic.halt_poll_success_ns += > ktime_to_ns(cur) - > ktime_to_ns(start_poll); > + kvm_stats_log_hist_update( > + vc->runner->stat.generic.halt_poll_success_hist, > + LOGHIST_SIZE_LARGE, > + ktime_to_ns(cur) - ktime_to_ns(start_poll)); > + } > } > > /* Adjust poll time */ > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 5ed6c235e059..977801c83aff 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -67,7 +67,6 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = { > STATS_DESC_COUNTER(VCPU, emulated_inst_exits), > STATS_DESC_COUNTER(VCPU, dec_exits), > STATS_DESC_COUNTER(VCPU, ext_intr_exits), > - STATS_DESC_TIME_NSEC(VCPU, halt_wait_ns), > STATS_DESC_COUNTER(VCPU, halt_successful_wait), > STATS_DESC_COUNTER(VCPU, dbell_exits), > STATS_DESC_COUNTER(VCPU, gdbell_exits), > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 356af173114d..268a0ccc9c5f 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -1369,7 +1369,14 @@ struct _kvm_stats_desc { > STATS_DESC_COUNTER(VCPU_GENERIC, halt_poll_invalid), \ > STATS_DESC_COUNTER(VCPU_GENERIC, halt_wakeup), \ > STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_success_ns), \ > - STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_ns) > + STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_ns), \ > + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_poll_success_hist, \ > + LOGHIST_SIZE_LARGE), \ This should probably be a new macro rather than using LOGHIST_SIZE_LARGE everywhere. #define HALT_POLL_HIST_SIZE LOGHIST_SIZE_LARGE > + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_hist, \ > + LOGHIST_SIZE_LARGE), \ > + STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_wait_ns), \ > + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_wait_hist, \ > + LOGHIST_SIZE_LARGE) > > extern struct dentry *kvm_debugfs_dir; > > diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h > index cc88cd676775..7838a42932c8 100644 > --- a/include/linux/kvm_types.h > +++ b/include/linux/kvm_types.h > @@ -103,6 +103,10 @@ struct kvm_vcpu_stat_generic { > u64 halt_wakeup; > u64 halt_poll_success_ns; > u64 halt_poll_fail_ns; > + u64 halt_poll_success_hist[LOGHIST_SIZE_LARGE]; > + u64 halt_poll_fail_hist[LOGHIST_SIZE_LARGE]; > + u64 halt_wait_ns; > + u64 halt_wait_hist[LOGHIST_SIZE_LARGE]; > }; > > #define KVM_STATS_NAME_SIZE 48 > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 3dcc2abbfc60..840b5bece080 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3093,12 +3093,24 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) > ++vcpu->stat.generic.halt_successful_poll; > if (!vcpu_valid_wakeup(vcpu)) > ++vcpu->stat.generic.halt_poll_invalid; > + > + kvm_stats_log_hist_update( > + vcpu->stat.generic.halt_poll_success_hist, > + LOGHIST_SIZE_LARGE, > + ktime_to_ns(ktime_get()) - > + ktime_to_ns(start)); > goto out; > } > poll_end = cur = ktime_get(); > } while (kvm_vcpu_can_poll(cur, stop)); > + > + kvm_stats_log_hist_update( > + vcpu->stat.generic.halt_poll_fail_hist, > + LOGHIST_SIZE_LARGE, > + ktime_to_ns(ktime_get()) - ktime_to_ns(start)); nit: Consider creating a wrapper for kvm_stats_log_hist_update() since there are so many call sites. You can save a line at every call site by avoiding passing in the histogram size. void halt_poll_hist_update(u64 *histogram, ktime_t time) { kvm_stats_log_hist_update(histogram, LOGHIST_SIZE_LARGE, time); } > } > > + > prepare_to_rcuwait(&vcpu->wait); > for (;;) { > set_current_state(TASK_INTERRUPTIBLE); > @@ -3111,6 +3123,13 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) > } > finish_rcuwait(&vcpu->wait); > cur = ktime_get(); > + if (waited) { > + vcpu->stat.generic.halt_wait_ns += > + ktime_to_ns(cur) - ktime_to_ns(poll_end); > + kvm_stats_log_hist_update(vcpu->stat.generic.halt_wait_hist, > + LOGHIST_SIZE_LARGE, > + ktime_to_ns(cur) - ktime_to_ns(poll_end)); > + } > out: > kvm_arch_vcpu_unblocking(vcpu); > block_ns = ktime_to_ns(cur) - ktime_to_ns(start); > -- > 2.32.0.93.g670b81a890-goog >
On Thu, Jul 8, 2021 at 4:42 PM David Matlack <dmatlack@google.com> wrote: > > On Tue, Jul 06, 2021 at 06:03:50PM +0000, Jing Zhang wrote: > > Add simple stats halt_wait_ns to record the time a VCPU has spent on > > waiting for all architectures (not just powerpc). > > Add three log histogram stats to record the distribution of time spent > > on successful polling, failed polling and VCPU wait. > > halt_poll_success_hist: Distribution of time spent before a successful > > polling. > > halt_poll_fail_hist: Distribution of time spent before a failed polling. > > halt_wait_hist: Distribution of time a VCPU has spent on waiting. > > > > Signed-off-by: Jing Zhang <jingzhangos@google.com> > > --- > > arch/powerpc/include/asm/kvm_host.h | 1 - > > arch/powerpc/kvm/book3s.c | 1 - > > arch/powerpc/kvm/book3s_hv.c | 20 +++++++++++++++++--- > > arch/powerpc/kvm/booke.c | 1 - > > include/linux/kvm_host.h | 9 ++++++++- > > include/linux/kvm_types.h | 4 ++++ > > virt/kvm/kvm_main.c | 19 +++++++++++++++++++ > > 7 files changed, 48 insertions(+), 7 deletions(-) > > > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > > index 9f52f282b1aa..4931d03e5799 100644 > > --- a/arch/powerpc/include/asm/kvm_host.h > > +++ b/arch/powerpc/include/asm/kvm_host.h > > @@ -103,7 +103,6 @@ struct kvm_vcpu_stat { > > u64 emulated_inst_exits; > > u64 dec_exits; > > u64 ext_intr_exits; > > - u64 halt_wait_ns; > > The halt_wait_ns refactor should be a separate patch. > How about putting it into a separate commit? Just want to keep all halt related change in the same patch series. > > u64 halt_successful_wait; > > u64 dbell_exits; > > u64 gdbell_exits; > > diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c > > index 5cc6e90095b0..b785f6772391 100644 > > --- a/arch/powerpc/kvm/book3s.c > > +++ b/arch/powerpc/kvm/book3s.c > > @@ -69,7 +69,6 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = { > > STATS_DESC_COUNTER(VCPU, emulated_inst_exits), > > STATS_DESC_COUNTER(VCPU, dec_exits), > > STATS_DESC_COUNTER(VCPU, ext_intr_exits), > > - STATS_DESC_TIME_NSEC(VCPU, halt_wait_ns), > > STATS_DESC_COUNTER(VCPU, halt_successful_wait), > > STATS_DESC_COUNTER(VCPU, dbell_exits), > > STATS_DESC_COUNTER(VCPU, gdbell_exits), > > diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > > index cd544a46183e..103f998cee75 100644 > > --- a/arch/powerpc/kvm/book3s_hv.c > > +++ b/arch/powerpc/kvm/book3s_hv.c > > @@ -4144,19 +4144,33 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc) > > > > /* Attribute wait time */ > > if (do_sleep) { > > - vc->runner->stat.halt_wait_ns += > > + vc->runner->stat.generic.halt_wait_ns += > > ktime_to_ns(cur) - ktime_to_ns(start_wait); > > + kvm_stats_log_hist_update( > > + vc->runner->stat.generic.halt_wait_hist, > > + LOGHIST_SIZE_LARGE, > > + ktime_to_ns(cur) - ktime_to_ns(start_wait)); > > /* Attribute failed poll time */ > > - if (vc->halt_poll_ns) > > + if (vc->halt_poll_ns) { > > vc->runner->stat.generic.halt_poll_fail_ns += > > ktime_to_ns(start_wait) - > > ktime_to_ns(start_poll); > > + kvm_stats_log_hist_update( > > + vc->runner->stat.generic.halt_poll_fail_hist, > > + LOGHIST_SIZE_LARGE, ktime_to_ns(start_wait) - > > + ktime_to_ns(start_poll)); > > + } > > } else { > > /* Attribute successful poll time */ > > - if (vc->halt_poll_ns) > > + if (vc->halt_poll_ns) { > > vc->runner->stat.generic.halt_poll_success_ns += > > ktime_to_ns(cur) - > > ktime_to_ns(start_poll); > > + kvm_stats_log_hist_update( > > + vc->runner->stat.generic.halt_poll_success_hist, > > + LOGHIST_SIZE_LARGE, > > + ktime_to_ns(cur) - ktime_to_ns(start_poll)); > > + } > > } > > > > /* Adjust poll time */ > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > > index 5ed6c235e059..977801c83aff 100644 > > --- a/arch/powerpc/kvm/booke.c > > +++ b/arch/powerpc/kvm/booke.c > > @@ -67,7 +67,6 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = { > > STATS_DESC_COUNTER(VCPU, emulated_inst_exits), > > STATS_DESC_COUNTER(VCPU, dec_exits), > > STATS_DESC_COUNTER(VCPU, ext_intr_exits), > > - STATS_DESC_TIME_NSEC(VCPU, halt_wait_ns), > > STATS_DESC_COUNTER(VCPU, halt_successful_wait), > > STATS_DESC_COUNTER(VCPU, dbell_exits), > > STATS_DESC_COUNTER(VCPU, gdbell_exits), > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > > index 356af173114d..268a0ccc9c5f 100644 > > --- a/include/linux/kvm_host.h > > +++ b/include/linux/kvm_host.h > > @@ -1369,7 +1369,14 @@ struct _kvm_stats_desc { > > STATS_DESC_COUNTER(VCPU_GENERIC, halt_poll_invalid), \ > > STATS_DESC_COUNTER(VCPU_GENERIC, halt_wakeup), \ > > STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_success_ns), \ > > - STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_ns) > > + STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_ns), \ > > + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_poll_success_hist, \ > > + LOGHIST_SIZE_LARGE), \ > > This should probably be a new macro rather than using LOGHIST_SIZE_LARGE > everywhere. > > #define HALT_POLL_HIST_SIZE LOGHIST_SIZE_LARGE > Will double check the new halt polling stats with this idea. > > + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_hist, \ > > + LOGHIST_SIZE_LARGE), \ > > + STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_wait_ns), \ > > + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_wait_hist, \ > > + LOGHIST_SIZE_LARGE) > > > > extern struct dentry *kvm_debugfs_dir; > > > > diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h > > index cc88cd676775..7838a42932c8 100644 > > --- a/include/linux/kvm_types.h > > +++ b/include/linux/kvm_types.h > > @@ -103,6 +103,10 @@ struct kvm_vcpu_stat_generic { > > u64 halt_wakeup; > > u64 halt_poll_success_ns; > > u64 halt_poll_fail_ns; > > + u64 halt_poll_success_hist[LOGHIST_SIZE_LARGE]; > > + u64 halt_poll_fail_hist[LOGHIST_SIZE_LARGE]; > > + u64 halt_wait_ns; > > + u64 halt_wait_hist[LOGHIST_SIZE_LARGE]; > > }; > > > > #define KVM_STATS_NAME_SIZE 48 > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > > index 3dcc2abbfc60..840b5bece080 100644 > > --- a/virt/kvm/kvm_main.c > > +++ b/virt/kvm/kvm_main.c > > @@ -3093,12 +3093,24 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) > > ++vcpu->stat.generic.halt_successful_poll; > > if (!vcpu_valid_wakeup(vcpu)) > > ++vcpu->stat.generic.halt_poll_invalid; > > + > > + kvm_stats_log_hist_update( > > + vcpu->stat.generic.halt_poll_success_hist, > > + LOGHIST_SIZE_LARGE, > > + ktime_to_ns(ktime_get()) - > > + ktime_to_ns(start)); > > goto out; > > } > > poll_end = cur = ktime_get(); > > } while (kvm_vcpu_can_poll(cur, stop)); > > + > > + kvm_stats_log_hist_update( > > + vcpu->stat.generic.halt_poll_fail_hist, > > + LOGHIST_SIZE_LARGE, > > + ktime_to_ns(ktime_get()) - ktime_to_ns(start)); > > nit: Consider creating a wrapper for kvm_stats_log_hist_update() since > there are so many call sites. You can save a line at every call site by > avoiding passing in the histogram size. > > void halt_poll_hist_update(u64 *histogram, ktime_t time) > { > kvm_stats_log_hist_update(histogram, LOGHIST_SIZE_LARGE, time); > } > > Will double check the new halt polling stats with this idea. > > } > > > > + > > prepare_to_rcuwait(&vcpu->wait); > > for (;;) { > > set_current_state(TASK_INTERRUPTIBLE); > > @@ -3111,6 +3123,13 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) > > } > > finish_rcuwait(&vcpu->wait); > > cur = ktime_get(); > > + if (waited) { > > + vcpu->stat.generic.halt_wait_ns += > > + ktime_to_ns(cur) - ktime_to_ns(poll_end); > > + kvm_stats_log_hist_update(vcpu->stat.generic.halt_wait_hist, > > + LOGHIST_SIZE_LARGE, > > + ktime_to_ns(cur) - ktime_to_ns(poll_end)); > > + } > > out: > > kvm_arch_vcpu_unblocking(vcpu); > > block_ns = ktime_to_ns(cur) - ktime_to_ns(start); > > -- > > 2.32.0.93.g670b81a890-goog > > Thanks, Jing
On Fri, Jul 09, 2021 at 10:18:20AM -0500, Jing Zhang wrote: > On Thu, Jul 8, 2021 at 4:42 PM David Matlack <dmatlack@google.com> wrote: > > > > On Tue, Jul 06, 2021 at 06:03:50PM +0000, Jing Zhang wrote: > > > > > > diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h > > > index 9f52f282b1aa..4931d03e5799 100644 > > > --- a/arch/powerpc/include/asm/kvm_host.h > > > +++ b/arch/powerpc/include/asm/kvm_host.h > > > @@ -103,7 +103,6 @@ struct kvm_vcpu_stat { > > > u64 emulated_inst_exits; > > > u64 dec_exits; > > > u64 ext_intr_exits; > > > - u64 halt_wait_ns; > > > > The halt_wait_ns refactor should be a separate patch. > > > How about putting it into a separate commit? Just want to keep all > halt related change in the > same patch series. I think we're on the same page: move the halt_wait_ns changes to a separate patch but still part of this series.
On 06/07/21 20:03, Jing Zhang wrote: > + kvm_stats_log_hist_update( > + vc->runner->stat.generic.halt_wait_hist, > + LOGHIST_SIZE_LARGE, > + ktime_to_ns(cur) - ktime_to_ns(start_wait)); Instead of passing the size to the function, perhaps you can wrap it with a macro #define KVM_STATS_LOG_HIST_UPDATE(array, value) \ kvm_stats_log_hist_update(array, ARRAY_SIZE(array), value) Paolo
On Wed, Jul 28, 2021 at 5:45 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 06/07/21 20:03, Jing Zhang wrote: > > + kvm_stats_log_hist_update( > > + vc->runner->stat.generic.halt_wait_hist, > > + LOGHIST_SIZE_LARGE, > > + ktime_to_ns(cur) - ktime_to_ns(start_wait)); > > Instead of passing the size to the function, perhaps you can wrap it > with a macro > > #define KVM_STATS_LOG_HIST_UPDATE(array, value) \ > kvm_stats_log_hist_update(array, ARRAY_SIZE(array), value) That's nice! Will do that. > > Paolo > Thanks, Jing
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h index 9f52f282b1aa..4931d03e5799 100644 --- a/arch/powerpc/include/asm/kvm_host.h +++ b/arch/powerpc/include/asm/kvm_host.h @@ -103,7 +103,6 @@ struct kvm_vcpu_stat { u64 emulated_inst_exits; u64 dec_exits; u64 ext_intr_exits; - u64 halt_wait_ns; u64 halt_successful_wait; u64 dbell_exits; u64 gdbell_exits; diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c index 5cc6e90095b0..b785f6772391 100644 --- a/arch/powerpc/kvm/book3s.c +++ b/arch/powerpc/kvm/book3s.c @@ -69,7 +69,6 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = { STATS_DESC_COUNTER(VCPU, emulated_inst_exits), STATS_DESC_COUNTER(VCPU, dec_exits), STATS_DESC_COUNTER(VCPU, ext_intr_exits), - STATS_DESC_TIME_NSEC(VCPU, halt_wait_ns), STATS_DESC_COUNTER(VCPU, halt_successful_wait), STATS_DESC_COUNTER(VCPU, dbell_exits), STATS_DESC_COUNTER(VCPU, gdbell_exits), diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index cd544a46183e..103f998cee75 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -4144,19 +4144,33 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc) /* Attribute wait time */ if (do_sleep) { - vc->runner->stat.halt_wait_ns += + vc->runner->stat.generic.halt_wait_ns += ktime_to_ns(cur) - ktime_to_ns(start_wait); + kvm_stats_log_hist_update( + vc->runner->stat.generic.halt_wait_hist, + LOGHIST_SIZE_LARGE, + ktime_to_ns(cur) - ktime_to_ns(start_wait)); /* Attribute failed poll time */ - if (vc->halt_poll_ns) + if (vc->halt_poll_ns) { vc->runner->stat.generic.halt_poll_fail_ns += ktime_to_ns(start_wait) - ktime_to_ns(start_poll); + kvm_stats_log_hist_update( + vc->runner->stat.generic.halt_poll_fail_hist, + LOGHIST_SIZE_LARGE, ktime_to_ns(start_wait) - + ktime_to_ns(start_poll)); + } } else { /* Attribute successful poll time */ - if (vc->halt_poll_ns) + if (vc->halt_poll_ns) { vc->runner->stat.generic.halt_poll_success_ns += ktime_to_ns(cur) - ktime_to_ns(start_poll); + kvm_stats_log_hist_update( + vc->runner->stat.generic.halt_poll_success_hist, + LOGHIST_SIZE_LARGE, + ktime_to_ns(cur) - ktime_to_ns(start_poll)); + } } /* Adjust poll time */ diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 5ed6c235e059..977801c83aff 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -67,7 +67,6 @@ const struct _kvm_stats_desc kvm_vcpu_stats_desc[] = { STATS_DESC_COUNTER(VCPU, emulated_inst_exits), STATS_DESC_COUNTER(VCPU, dec_exits), STATS_DESC_COUNTER(VCPU, ext_intr_exits), - STATS_DESC_TIME_NSEC(VCPU, halt_wait_ns), STATS_DESC_COUNTER(VCPU, halt_successful_wait), STATS_DESC_COUNTER(VCPU, dbell_exits), STATS_DESC_COUNTER(VCPU, gdbell_exits), diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 356af173114d..268a0ccc9c5f 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1369,7 +1369,14 @@ struct _kvm_stats_desc { STATS_DESC_COUNTER(VCPU_GENERIC, halt_poll_invalid), \ STATS_DESC_COUNTER(VCPU_GENERIC, halt_wakeup), \ STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_success_ns), \ - STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_ns) + STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_ns), \ + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_poll_success_hist, \ + LOGHIST_SIZE_LARGE), \ + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_poll_fail_hist, \ + LOGHIST_SIZE_LARGE), \ + STATS_DESC_TIME_NSEC(VCPU_GENERIC, halt_wait_ns), \ + STATS_DESC_LOGHIST_TIME_NSEC(VCPU_GENERIC, halt_wait_hist, \ + LOGHIST_SIZE_LARGE) extern struct dentry *kvm_debugfs_dir; diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h index cc88cd676775..7838a42932c8 100644 --- a/include/linux/kvm_types.h +++ b/include/linux/kvm_types.h @@ -103,6 +103,10 @@ struct kvm_vcpu_stat_generic { u64 halt_wakeup; u64 halt_poll_success_ns; u64 halt_poll_fail_ns; + u64 halt_poll_success_hist[LOGHIST_SIZE_LARGE]; + u64 halt_poll_fail_hist[LOGHIST_SIZE_LARGE]; + u64 halt_wait_ns; + u64 halt_wait_hist[LOGHIST_SIZE_LARGE]; }; #define KVM_STATS_NAME_SIZE 48 diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 3dcc2abbfc60..840b5bece080 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3093,12 +3093,24 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) ++vcpu->stat.generic.halt_successful_poll; if (!vcpu_valid_wakeup(vcpu)) ++vcpu->stat.generic.halt_poll_invalid; + + kvm_stats_log_hist_update( + vcpu->stat.generic.halt_poll_success_hist, + LOGHIST_SIZE_LARGE, + ktime_to_ns(ktime_get()) - + ktime_to_ns(start)); goto out; } poll_end = cur = ktime_get(); } while (kvm_vcpu_can_poll(cur, stop)); + + kvm_stats_log_hist_update( + vcpu->stat.generic.halt_poll_fail_hist, + LOGHIST_SIZE_LARGE, + ktime_to_ns(ktime_get()) - ktime_to_ns(start)); } + prepare_to_rcuwait(&vcpu->wait); for (;;) { set_current_state(TASK_INTERRUPTIBLE); @@ -3111,6 +3123,13 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu) } finish_rcuwait(&vcpu->wait); cur = ktime_get(); + if (waited) { + vcpu->stat.generic.halt_wait_ns += + ktime_to_ns(cur) - ktime_to_ns(poll_end); + kvm_stats_log_hist_update(vcpu->stat.generic.halt_wait_hist, + LOGHIST_SIZE_LARGE, + ktime_to_ns(cur) - ktime_to_ns(poll_end)); + } out: kvm_arch_vcpu_unblocking(vcpu); block_ns = ktime_to_ns(cur) - ktime_to_ns(start);
Add simple stats halt_wait_ns to record the time a VCPU has spent on waiting for all architectures (not just powerpc). Add three log histogram stats to record the distribution of time spent on successful polling, failed polling and VCPU wait. halt_poll_success_hist: Distribution of time spent before a successful polling. halt_poll_fail_hist: Distribution of time spent before a failed polling. halt_wait_hist: Distribution of time a VCPU has spent on waiting. Signed-off-by: Jing Zhang <jingzhangos@google.com> --- arch/powerpc/include/asm/kvm_host.h | 1 - arch/powerpc/kvm/book3s.c | 1 - arch/powerpc/kvm/book3s_hv.c | 20 +++++++++++++++++--- arch/powerpc/kvm/booke.c | 1 - include/linux/kvm_host.h | 9 ++++++++- include/linux/kvm_types.h | 4 ++++ virt/kvm/kvm_main.c | 19 +++++++++++++++++++ 7 files changed, 48 insertions(+), 7 deletions(-)