Re: [PATCH V5 5/5] powerpc/kvm/stats: Implement existing and add new halt polling vcpu stats

[David Matlack via Linuxppc-dev]

On Thu, Jul 21, 2016 at 8:41 PM, Suraj Jitindar Singh
 wrote:
> vcpu stats are used to collect information about a vcpu which can be viewed
> in the debugfs. For example halt_attempted_poll and halt_successful_poll
> are used to keep track of the number of times the vcpu attempts to and
> successfully polls. These stats are currently not used on powerpc.
>
> Implement incrementation of the halt_attempted_poll and
> halt_successful_poll vcpu stats for powerpc. Since these stats are summed
> over all the vcpus for all running guests it doesn't matter which vcpu
> they are attributed to, thus we choose the current runner vcpu of the
> vcore.
>
> Also add new vcpu stats: halt_poll_success_ns, halt_poll_fail_ns and
> halt_wait_ns to be used to accumulate the total time spend polling
> successfully, polling unsuccessfully and waiting respectively, and
> halt_successful_wait to accumulate the number of times the vcpu waits.
> Given that halt_poll_success_ns, halt_poll_fail_ns and halt_wait_ns are
> expressed in nanoseconds it is necessary to represent these as 64-bit
> quantities, otherwise they would overflow after only about 4 seconds.
>
> Given that the total time spend either polling or waiting will be known and
> the number of times that each was done, it will be possible to determine
> the average poll and wait times. This will give the ability to tune the kvm
> module parameters based on the calculated average wait and poll times.
>
> Signed-off-by: Suraj Jitindar Singh 
> Reviewed-by: David Matlack 
>
> ---
> Change Log:
>
> V3 -> V4:
> - Instead of accounting just wait and poll time, separate these
>   into successful_poll_time, failed_poll_time and wait_time.
> V4 -> V5:
> - Add single_task_running() check to polling loop

I was expecting to see this in PATCH 3/5 with the halt-polling
implementation. But otherwise, looks good, and the net effect is the
same.

> ---
>  arch/powerpc/include/asm/kvm_host.h |  4 
>  arch/powerpc/kvm/book3s.c   |  4 
>  arch/powerpc/kvm/book3s_hv.c| 38 
> +++--
>  3 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index f6304c5..f15ffc0 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -114,8 +114,12 @@ struct kvm_vcpu_stat {
> u64 emulated_inst_exits;
> u64 dec_exits;
> u64 ext_intr_exits;
> +   u64 halt_poll_success_ns;
> +   u64 halt_poll_fail_ns;
> +   u64 halt_wait_ns;
> u64 halt_successful_poll;
> u64 halt_attempted_poll;
> +   u64 halt_successful_wait;
> u64 halt_poll_invalid;
> u64 halt_wakeup;
> u64 dbell_exits;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index 47018fc..71eb8f3 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -52,8 +52,12 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
> { "dec", VCPU_STAT(dec_exits) },
> { "ext_intr",VCPU_STAT(ext_intr_exits) },
> { "queue_intr",  VCPU_STAT(queue_intr) },
> +   { "halt_poll_success_ns",   VCPU_STAT(halt_poll_success_ns) },
> +   { "halt_poll_fail_ns",  VCPU_STAT(halt_poll_fail_ns) },
> +   { "halt_wait_ns",   VCPU_STAT(halt_wait_ns) },
> { "halt_successful_poll", VCPU_STAT(halt_successful_poll), },
> { "halt_attempted_poll", VCPU_STAT(halt_attempted_poll), },
> +   { "halt_successful_wait",   VCPU_STAT(halt_successful_wait) },
> { "halt_poll_invalid", VCPU_STAT(halt_poll_invalid) },
> { "halt_wakeup", VCPU_STAT(halt_wakeup) },
> { "pf_storage",  VCPU_STAT(pf_storage) },
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index a9de1d4..b1d9e88 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -2679,15 +2679,16 @@ static int kvmppc_vcore_check_block(struct 
> kvmppc_vcore *vc)
>   */
>  static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
>  {
> +   ktime_t cur, start_poll, start_wait;
> int do_sleep = 1;
> -   ktime_t cur, start;
> u64 block_ns;
> DECLARE_SWAITQUEUE(wait);
>
> /* Poll for pending exceptions and ceded state */
> -   cur = start = ktime_get();
> +   cur = start_poll = ktime_get();
> if (vc->halt_poll_ns) {
> -   ktime_t stop = ktime_add_ns(start, vc->halt_poll_ns);
> +   ktime_t stop = ktime_add_ns(start_poll, vc->halt_poll_ns);
> +   ++vc->runner->stat.halt_attempted_poll;
>
> vc->vcore_state = VCORE_POLLING;
> spin_unlock(>lock);
> @@ -2698,13 +2699,15 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore 
> *vc)
> break;
>   

Re: [PATCH V4 4/5] kvm/stats: Add provisioning for ulong vm stats and u64 vcpu stats

[David Matlack via Linuxppc-dev]

On Tue, Jul 19, 2016 at 1:12 AM, Suraj Jitindar Singh
 wrote:
> vms and vcpus have statistics associated with them which can be viewed
> within the debugfs. Currently it is assumed within the vcpu_stat_get() and
> vm_stat_get() functions that all of these statistics are represented as
> u32s, however the next patch adds some u64 vcpu statistics.
>
> Change all vcpu statistics to u64 and modify vcpu_stat_get() accordingly.
> Since vcpu statistics are per vcpu, they will only be updated by a single
> vcpu at a time so this shouldn't present a problem on 32-bit machines
> which can't atomically increment 64-bit numbers. However vm statistics
> could potentially be updated by multiple vcpus from that vm at a time.
> To avoid the overhead of atomics make all vm statistics ulong such that
> they are 64-bit on 64-bit systems where they can be atomically incremented
> and are 32-bit on 32-bit systems which may not be able to atomically
> increment 64-bit numbers. Modify vm_stat_get() to expect ulongs.
>
> Signed-off-by: Suraj Jitindar Singh 

Looks great, thanks.

Reviewed-by: David Matlack 

>
> ---
> Change Log:
>
> V2 -> V3:
> - Instead of implementing separate u32 and u64 functions keep the
>   generic functions and modify them to expect u64s. Thus update all
>   vm and vcpu statistics to u64s accordingly.
> V3 -> V4:
> - Change vm_stats from u64 to ulong
> ---
>  arch/arm/include/asm/kvm_host.h |  12 ++--
>  arch/arm64/include/asm/kvm_host.h   |  12 ++--
>  arch/mips/include/asm/kvm_host.h|  46 ++---
>  arch/powerpc/include/asm/kvm_host.h |  60 -
>  arch/s390/include/asm/kvm_host.h| 128 
> ++--
>  arch/x86/include/asm/kvm_host.h |  72 ++--
>  virt/kvm/kvm_main.c |   4 +-
>  7 files changed, 167 insertions(+), 167 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 96387d4..c8e55b3b 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -183,15 +183,15 @@ struct kvm_vcpu_arch {
>  };
>
>  struct kvm_vm_stat {
> -   u32 remote_tlb_flush;
> +   ulong remote_tlb_flush;
>  };
>
>  struct kvm_vcpu_stat {
> -   u32 halt_successful_poll;
> -   u32 halt_attempted_poll;
> -   u32 halt_poll_invalid;
> -   u32 halt_wakeup;
> -   u32 hvc_exit_stat;
> +   u64 halt_successful_poll;
> +   u64 halt_attempted_poll;
> +   u64 halt_poll_invalid;
> +   u64 halt_wakeup;
> +   u64 hvc_exit_stat;
> u64 wfe_exit_stat;
> u64 wfi_exit_stat;
> u64 mmio_exit_user;
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index 49095fc..b14c8bc 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -291,15 +291,15 @@ struct kvm_vcpu_arch {
>  #endif
>
>  struct kvm_vm_stat {
> -   u32 remote_tlb_flush;
> +   ulong remote_tlb_flush;
>  };
>
>  struct kvm_vcpu_stat {
> -   u32 halt_successful_poll;
> -   u32 halt_attempted_poll;
> -   u32 halt_poll_invalid;
> -   u32 halt_wakeup;
> -   u32 hvc_exit_stat;
> +   u64 halt_successful_poll;
> +   u64 halt_attempted_poll;
> +   u64 halt_poll_invalid;
> +   u64 halt_wakeup;
> +   u64 hvc_exit_stat;
> u64 wfe_exit_stat;
> u64 wfi_exit_stat;
> u64 mmio_exit_user;
> diff --git a/arch/mips/include/asm/kvm_host.h 
> b/arch/mips/include/asm/kvm_host.h
> index 36a391d..9704888 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -98,32 +98,32 @@ extern void (*kvm_mips_release_pfn_clean)(kvm_pfn_t pfn);
>  extern bool (*kvm_mips_is_error_pfn)(kvm_pfn_t pfn);
>
>  struct kvm_vm_stat {
> -   u32 remote_tlb_flush;
> +   ulong remote_tlb_flush;
>  };
>
>  struct kvm_vcpu_stat {
> -   u32 wait_exits;
> -   u32 cache_exits;
> -   u32 signal_exits;
> -   u32 int_exits;
> -   u32 cop_unusable_exits;
> -   u32 tlbmod_exits;
> -   u32 tlbmiss_ld_exits;
> -   u32 tlbmiss_st_exits;
> -   u32 addrerr_st_exits;
> -   u32 addrerr_ld_exits;
> -   u32 syscall_exits;
> -   u32 resvd_inst_exits;
> -   u32 break_inst_exits;
> -   u32 trap_inst_exits;
> -   u32 msa_fpe_exits;
> -   u32 fpe_exits;
> -   u32 msa_disabled_exits;
> -   u32 flush_dcache_exits;
> -   u32 halt_successful_poll;
> -   u32 halt_attempted_poll;
> -   u32 halt_poll_invalid;
> -   u32 halt_wakeup;
> +   u64 wait_exits;
> +   u64 cache_exits;
> +   u64 signal_exits;
> +   u64 int_exits;
> +   u64 cop_unusable_exits;
> +   u64 tlbmod_exits;
> +   u64 tlbmiss_ld_exits;
> +   u64 tlbmiss_st_exits;
> +   u64 addrerr_st_exits;
> +   u64 addrerr_ld_exits;
> +   u64 

Re: [PATCH V4 3/5] kvm/ppc/book3s_hv: Implement halt polling in the kvm_hv kernel module

[David Matlack via Linuxppc-dev]

On Tue, Jul 19, 2016 at 1:12 AM, Suraj Jitindar Singh
 wrote:
> This patch introduces new halt polling functionality into the kvm_hv kernel
> module. When a vcore is idle it will poll for some period of time before
> scheduling itself out.
>
> When all of the runnable vcpus on a vcore have ceded (and thus the vcore is
> idle) we schedule ourselves out to allow something else to run. In the
> event that we need to wake up very quickly (for example an interrupt
> arrives), we are required to wait until we get scheduled again.
>
> Implement halt polling so that when a vcore is idle, and before scheduling
> ourselves, we poll for vcpus in the runnable_threads list which have
> pending exceptions or which leave the ceded state. If we poll successfully
> then we can get back into the guest very quickly without ever scheduling
> ourselves, otherwise we schedule ourselves out as before.
>
> Testing of this patch with a TCP round robin test between two guests with
> virtio network interfaces has found a decrease in round trip time of ~15us
> on average. A performance gain is only seen when going out of and
> back into the guest often and quickly, otherwise there is no net benefit
> from the polling. The polling interval is adjusted such that when we are
> often scheduled out for long periods of time it is reduced, and when we
> often poll successfully it is increased. The rate at which the polling
> interval increases or decreases, and the maximum polling interval, can
> be set through module parameters.
>
> Based on the implementation in the generic kvm module by Wanpeng Li and
> Paolo Bonzini, and on direction from Paul Mackerras.
>
> Signed-off-by: Suraj Jitindar Singh 
> ---
>  arch/powerpc/include/asm/kvm_book3s.h |   1 +
>  arch/powerpc/include/asm/kvm_host.h   |   1 +
>  arch/powerpc/kvm/book3s_hv.c  | 116 
> ++
>  arch/powerpc/kvm/trace_hv.h   |  22 +++
>  4 files changed, 126 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
> b/arch/powerpc/include/asm/kvm_book3s.h
> index 151f817..c261f52 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -102,6 +102,7 @@ struct kvmppc_vcore {
> ulong pcr;
> ulong dpdes;/* doorbell state (POWER8) */
> ulong conferring_threads;
> +   unsigned int halt_poll_ns;
>  };
>
>  struct kvmppc_vcpu_book3s {
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index 02d06e9..610f393 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -294,6 +294,7 @@ struct kvm_arch {
>  #define VCORE_SLEEPING 3
>  #define VCORE_RUNNING  4
>  #define VCORE_EXITING  5
> +#define VCORE_POLLING  6
>
>  /*
>   * Struct used to manage memory for a virtual processor area
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 3bcf9e6..a9de1d4 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -94,6 +94,23 @@ module_param_cb(h_ipi_redirect, _param_ops, 
> _ipi_redirect,
>  MODULE_PARM_DESC(h_ipi_redirect, "Redirect H_IPI wakeup to a free host 
> core");
>  #endif
>
> +/* Maximum halt poll interval defaults to KVM_HALT_POLL_NS_DEFAULT */
> +static unsigned int halt_poll_max_ns = KVM_HALT_POLL_NS_DEFAULT;
> +module_param(halt_poll_max_ns, uint, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(halt_poll_max_ns, "Maximum halt poll time in ns");
> +
> +/* Factor by which the vcore halt poll interval is grown, default is to 
> double
> + */
> +static unsigned int halt_poll_ns_grow = 2;
> +module_param(halt_poll_ns_grow, int, S_IRUGO);
> +MODULE_PARM_DESC(halt_poll_ns_grow, "Factor halt poll time is grown by");
> +
> +/* Factor by which the vcore halt poll interval is shrunk, default is to 
> reset
> + */
> +static unsigned int halt_poll_ns_shrink;
> +module_param(halt_poll_ns_shrink, int, S_IRUGO);
> +MODULE_PARM_DESC(halt_poll_ns_shrink, "Factor halt poll time is shrunk by");
> +
>  static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
>  static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>
> @@ -2620,32 +2637,82 @@ static void kvmppc_wait_for_exec(struct kvmppc_vcore 
> *vc,
> finish_wait(>arch.cpu_run, );
>  }
>
> +static void grow_halt_poll_ns(struct kvmppc_vcore *vc)
> +{
> +   /* 10us base */
> +   if (vc->halt_poll_ns == 0 && halt_poll_ns_grow)
> +   vc->halt_poll_ns = 1;
> +   else
> +   vc->halt_poll_ns *= halt_poll_ns_grow;
> +
> +   if (vc->halt_poll_ns > halt_poll_max_ns)
> +   vc->halt_poll_ns = halt_poll_max_ns;
> +}
> +
> +static void shrink_halt_poll_ns(struct kvmppc_vcore *vc)
> +{
> +   if (halt_poll_ns_shrink == 0)
> +   vc->halt_poll_ns = 0;
> +   else
> +   vc->halt_poll_ns /= halt_poll_ns_shrink;
> +}
> +
> +/* Check 

Re: [PATCH V2 5/5] powerpc/kvm/stats: Implement existing and add new halt polling vcpu stats

[David Matlack via Linuxppc-dev]

On Tue, Jul 12, 2016 at 11:07 PM, Suraj Jitindar Singh
 wrote:
> On 12/07/16 16:17, Suraj Jitindar Singh wrote:
>> On 12/07/16 02:49, David Matlack wrote:
[snip]
>>> It's possible to poll and wait in one halt, conflating this stat with
>>> polling time. Is it useful to split out a third stat,
>>> halt_poll_fail_ns which counts how long we polled which ended up
>>> sleeping? Then halt_wait_time only counts the time the VCPU spent on
>>> the wait queue. The sum of all 3 is still the total time spent halted.
>>>
>> I see what you're saying. I would say that in the event that you do wait
>> then the most useful number is going to be the total block time (the sum
>> of the wait and poll time) as this is the minimum value you would have to
>> set the halt_poll_max_ns module parameter in order to ensure you poll
>> for long enough (in most circumstances) to avoid waiting, which is the main
>> use case I envision for this statistic. That being said this is definitely
>> a source of ambiguity and splitting this into two statistics would make the
>> distinction clearer without any loss of data, you could simply sum the two
>> stats to get the same number.
>>
>> Either way I don't think it really makes much of a difference, but in the
>> interest of clarity I think I'll split the statistic.
>
> On further though, I really think that splitting this statistic is an
> unnecessary source of ambiguity. In reality the interesting piece of
> information is going to be the average time that you blocked on
> either an unsuccessful poll or a successful poll.
>
> So instead of splitting the statistic I'm going to rename them as:
> halt_poll_time -> halt_block_time_successful_poll
> halt_wait_time -> halt_block_time_waited

The downside of having only these 2 stats is there is no way to see
the total time spent halt-polling. Halt-polling shows up as host
kernel CPU usage on the VCPU thread, despite it really being idle
cycles that could be reclaimed. It's useful to have the total amount
of time spent halt-polling (halt_poll_fail + halt_poll_success) to
feed into provisioning/monitoring systems that look at CPU usage.

FWIW, I have a very similar patch internally. It adds 2 stats,
halt_poll_success_ns and halt_poll_fail_ns, to the halt-polling code
in virt/kvm/kvm_main.c. So if you agree splitting the stats makes
sense, it would be helpful to us if we can adopt the same naming
convention.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V2 4/5] kvm/stats: Add provisioning for 64-bit vcpu statistics

[David Matlack via Linuxppc-dev]

On Mon, Jul 11, 2016 at 12:31 PM, Paolo Bonzini  wrote:
>
>
> On 11/07/2016 19:30, David Matlack wrote:
>> On Mon, Jul 11, 2016 at 10:05 AM, Paolo Bonzini  wrote:
>>>
>>>
>>> On 11/07/2016 18:51, David Matlack wrote:
>> vcpus have statistics associated with them which can be viewed within the
>> debugfs. Currently it is assumed within the vcpu_stat_get() and
>> vcpu_stat_get_per_vm() functions that all of these statistics are
>> represented as 32-bit numbers. The next patch adds some 64-bit 
>> statistics,
>> so add provisioning for the display of 64-bit vcpu statistics.
 Thanks, we need 64-bit stats in other places as well. Can we use this
 opportunity to wholesale upgrade all KVM stats from u32 to u64? Most
 of this patch is duplicated code with "u32" swapped with "u64".

>>>
>>> I'm not sure of what 32-bit architectures would do, but perhaps we could
>>> upgrade them to unsigned long at least.
>>
>> I thought u64 still existed on 32-bit architectures. unsigned long
>> would be fine but with the caveat that certain stats would overflow on
>> 32-bit architectures.
>
> Yes, but not all 32-bit architectures can do atomic read-modify-write
> (e.g. add) operations on 64-bit values.

I think that's ok, none of the stats currently use atomic operations.

>
> Paolo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V2 3/5] kvm/ppc/book3s_hv: Implement halt polling in the kvm_hv kernel module

[David Matlack via Linuxppc-dev]

On Mon, Jul 11, 2016 at 10:07 AM, Paolo Bonzini  wrote:
>
>
> On 11/07/2016 18:57, David Matlack wrote:
>> On Mon, Jul 11, 2016 at 12:08 AM, Suraj Jitindar Singh
>>  wrote:
>> > This patch introduces new halt polling functionality into the kvm_hv kernel
>> > module. When a vcore is idle it will poll for some period of time before
>> > scheduling itself out.
>>
>> Is there any way to reuse the existing halt-polling code? Having two
>> copies risks them diverging over time.
>
> s/risks/guarantees/ :(
>
> Unfortunately, handling of the hardware threads in KVM PPC is a mess,
> and I don't think it's possible to remove the duplication.

Ah, ok. That's a shame.

>
> Paolo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V2 3/5] kvm/ppc/book3s_hv: Implement halt polling in the kvm_hv kernel module

[David Matlack via Linuxppc-dev]

On Mon, Jul 11, 2016 at 12:08 AM, Suraj Jitindar Singh
 wrote:
> This patch introduces new halt polling functionality into the kvm_hv kernel
> module. When a vcore is idle it will poll for some period of time before
> scheduling itself out.

Is there any way to reuse the existing halt-polling code? Having two
copies risks them diverging over time.

>
> When all of the runnable vcpus on a vcore have ceded (and thus the vcore is
> idle) we schedule ourselves out to allow something else to run. In the
> event that we need to wake up very quickly (for example an interrupt
> arrives), we are required to wait until we get scheduled again.
>
> Implement halt polling so that when a vcore is idle, and before scheduling
> ourselves, we poll for vcpus in the runnable_threads list which have
> pending exceptions or which leave the ceded state. If we poll successfully
> then we can get back into the guest very quickly without ever scheduling
> ourselves, otherwise we schedule ourselves out as before.
>
> Testing of this patch with a TCP round robin test between two guests with
> virtio network interfaces has found a decrease in round trip time from
> ~140us to ~115us. A performance gain is only seen when going out of and
> back into the guest often and quickly, otherwise there is no net benefit
> from the polling. The polling interval is adjusted such that when we are
> often scheduled out for long periods of time it is reduced, and when we
> often poll successfully it is increased. The rate at which the polling
> interval increases or decreases, and the maximum polling interval, can
> be set through module parameters.
>
> Based on the implementation in the generic kvm module by Wanpeng Li and
> Paolo Bonzini, and on direction from Paul Mackerras.
>
> ---
> Change Log:
>
> V1 -> V2:
> - Nothing
>
> Signed-off-by: Suraj Jitindar Singh 
> ---
>  arch/powerpc/include/asm/kvm_book3s.h |   1 +
>  arch/powerpc/include/asm/kvm_host.h   |   1 +
>  arch/powerpc/kvm/book3s_hv.c  | 115 
> +-
>  arch/powerpc/kvm/trace_hv.h   |  22 +++
>  4 files changed, 125 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
> b/arch/powerpc/include/asm/kvm_book3s.h
> index 151f817..c261f52 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -102,6 +102,7 @@ struct kvmppc_vcore {
> ulong pcr;
> ulong dpdes;/* doorbell state (POWER8) */
> ulong conferring_threads;
> +   unsigned int halt_poll_ns;
>  };
>
>  struct kvmppc_vcpu_book3s {
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index 02d06e9..610f393 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -294,6 +294,7 @@ struct kvm_arch {
>  #define VCORE_SLEEPING 3
>  #define VCORE_RUNNING  4
>  #define VCORE_EXITING  5
> +#define VCORE_POLLING  6
>
>  /*
>   * Struct used to manage memory for a virtual processor area
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 3bcf9e6..0d8ce14 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -94,6 +94,23 @@ module_param_cb(h_ipi_redirect, _param_ops, 
> _ipi_redirect,
>  MODULE_PARM_DESC(h_ipi_redirect, "Redirect H_IPI wakeup to a free host 
> core");
>  #endif
>
> +/* Maximum halt poll interval defaults to KVM_HALT_POLL_NS_DEFAULT */
> +static unsigned int halt_poll_max_ns = KVM_HALT_POLL_NS_DEFAULT;
> +module_param(halt_poll_max_ns, uint, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(halt_poll_max_ns, "Maximum halt poll time in ns");
> +
> +/* Factor by which the vcore halt poll interval is grown, default is to 
> double
> + */
> +static unsigned int halt_poll_ns_grow = 2;
> +module_param(halt_poll_ns_grow, int, S_IRUGO);
> +MODULE_PARM_DESC(halt_poll_ns_grow, "Factor halt poll time is grown by");
> +
> +/* Factor by which the vcore halt poll interval is shrunk, default is to 
> reset
> + */
> +static unsigned int halt_poll_ns_shrink;
> +module_param(halt_poll_ns_shrink, int, S_IRUGO);
> +MODULE_PARM_DESC(halt_poll_ns_shrink, "Factor halt poll time is shrunk by");
> +
>  static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
>  static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>
> @@ -2620,32 +2637,82 @@ static void kvmppc_wait_for_exec(struct kvmppc_vcore 
> *vc,
> finish_wait(>arch.cpu_run, );
>  }
>
> +static void grow_halt_poll_ns(struct kvmppc_vcore *vc)
> +{
> +   /* 10us base */
> +   if (vc->halt_poll_ns == 0 && halt_poll_ns_grow)
> +   vc->halt_poll_ns = 1;
> +   else
> +   vc->halt_poll_ns *= halt_poll_ns_grow;
> +
> +   if (vc->halt_poll_ns > halt_poll_max_ns)
> +   vc->halt_poll_ns = halt_poll_max_ns;
> +}
> +
> +static void shrink_halt_poll_ns(struct kvmppc_vcore *vc)
> +{
> + 

Re: [PATCH V2 5/5] powerpc/kvm/stats: Implement existing and add new halt polling vcpu stats

[David Matlack via Linuxppc-dev]

On Mon, Jul 11, 2016 at 12:08 AM, Suraj Jitindar Singh
 wrote:
> vcpu stats are used to collect information about a vcpu which can be viewed
> in the debugfs. For example halt_attempted_poll and halt_successful_poll
> are used to keep track of the number of times the vcpu attempts to and
> successfully polls. These stats are currently not used on powerpc.
>
> Implement incrementation of the halt_attempted_poll and
> halt_successful_poll vcpu stats for powerpc. Since these stats are summed
> over all the vcpus for all running guests it doesn't matter which vcpu
> they are attributed to, thus we choose the current runner vcpu of the
> vcore.
>
> Also add new vcpu stats: halt_poll_time and halt_wait_time to be used to
> accumulate the total time spend polling and waiting respectively, and
> halt_successful_wait to accumulate the number of times the vcpu waits.
> Given that halt_poll_time and halt_wait_time are expressed in nanoseconds
> it is necessary to represent these as 64-bit quantities, otherwise they
> would overflow after only about 4 seconds.
>
> Given that the total time spend either polling or waiting will be known and
> the number of times that each was done, it will be possible to determine
> the average poll and wait times. This will give the ability to tune the kvm
> module parameters based on the calculated average wait and poll times.
>
> ---
> Change Log:
>
> V1 -> V2:
> - Nothing
>
> Signed-off-by: Suraj Jitindar Singh 
> ---
>  arch/powerpc/include/asm/kvm_host.h |  3 +++
>  arch/powerpc/kvm/book3s.c   |  3 +++
>  arch/powerpc/kvm/book3s_hv.c| 14 +-
>  3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index 610f393..66a7198 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -114,8 +114,11 @@ struct kvm_vcpu_stat {
> u32 emulated_inst_exits;
> u32 dec_exits;
> u32 ext_intr_exits;
> +   u64 halt_poll_time;
> +   u64 halt_wait_time;
> u32 halt_successful_poll;
> u32 halt_attempted_poll;
> +   u32 halt_successful_wait;
> u32 halt_poll_invalid;
> u32 halt_wakeup;
> u32 dbell_exits;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index ed9132b..6217bea 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -53,8 +53,11 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
> { "dec", VCPU_STAT(dec_exits) },
> { "ext_intr",VCPU_STAT(ext_intr_exits) },
> { "queue_intr",  VCPU_STAT(queue_intr) },
> +   { "halt_poll_time_ns",  VCPU_STAT_U64(halt_poll_time) },
> +   { "halt_wait_time_ns",  VCPU_STAT_U64(halt_wait_time) },
> { "halt_successful_poll", VCPU_STAT(halt_successful_poll), },
> { "halt_attempted_poll", VCPU_STAT(halt_attempted_poll), },
> +   { "halt_successful_wait",   VCPU_STAT(halt_successful_wait) },
> { "halt_poll_invalid", VCPU_STAT(halt_poll_invalid) },
> { "halt_wakeup", VCPU_STAT(halt_wakeup) },
> { "pf_storage",  VCPU_STAT(pf_storage) },
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 0d8ce14..a0dae63 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -2688,6 +2688,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore 
> *vc)
> cur = start = ktime_get();
> if (vc->halt_poll_ns) {
> ktime_t stop = ktime_add_ns(start, vc->halt_poll_ns);
> +   ++vc->runner->stat.halt_attempted_poll;
>
> vc->vcore_state = VCORE_POLLING;
> spin_unlock(>lock);
> @@ -2703,8 +2704,10 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore 
> *vc)
> spin_lock(>lock);
> vc->vcore_state = VCORE_INACTIVE;
>
> -   if (!do_sleep)
> +   if (!do_sleep) {
> +   ++vc->runner->stat.halt_successful_poll;
> goto out;
> +   }
> }
>
> prepare_to_swait(>wq, , TASK_INTERRUPTIBLE);
> @@ -2712,6 +2715,9 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore 
> *vc)
> if (kvmppc_vcore_check_block(vc)) {
> finish_swait(>wq, );
> do_sleep = 0;
> +   /* If we polled, count this as a successful poll */
> +   if (vc->halt_poll_ns)
> +   ++vc->runner->stat.halt_successful_poll;
> goto out;
> }
>
> @@ -2723,12 +2729,18 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore 
> *vc)
> spin_lock(>lock);
> vc->vcore_state = VCORE_INACTIVE;
> trace_kvmppc_vcore_blocked(vc, 1);
> +   ++vc->runner->stat.halt_successful_wait;
>
> cur = ktime_get();
>
>  out:
>