Re: [PATCH] KVM: Don't use vcpu->requests for steal time accounting

Gleb Natapov
2012-12-14T11:28:30Z

On Fri, Dec 14, 2012 at 07:37:18PM +0900, Takuya Yoshikawa wrote:
> We can check if accum_steal has any positive value instead of using
> KVM_REQ_STEAL_UPDATE bit in vcpu->requests; and this is the way we
> usually do for accounting for something in the kernel.
>
Now you added check that will be done on each guest entry, requests
mechanism prevents that.

> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
> ---
> arch/x86/kvm/x86.c | 11 +++++------
> include/linux/kvm_host.h | 41 ++++++++++++++++++++---------------------
> 2 files changed, 25 insertions(+), 27 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 57c76e8..fab4c3e 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1857,6 +1857,9 @@ static void accumulate_steal_time(struct kvm_vcpu *vcpu)
>
> static void record_steal_time(struct kvm_vcpu *vcpu)
> {
> + if (!vcpu->arch.st.accum_steal)
> + return;
> +
> if (!(vcpu->arch.st.msr_val & KVM_MSR_ENABLED))
> return;
>
> @@ -1992,9 +1995,6 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> preempt_disable();
> accumulate_steal_time(vcpu);
> preempt_enable();
> -
> - kvm_make_request(KVM_REQ_STEAL_UPDATE, vcpu);
> -
> break;
> case MSR_KVM_PV_EOI_EN:
> if (kvm_lapic_enable_pv_eoi(vcpu, data))
> @@ -2668,7 +2668,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> }
>
> accumulate_steal_time(vcpu);
> - kvm_make_request(KVM_REQ_STEAL_UPDATE, vcpu);
> }
>
> void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> @@ -5645,8 +5644,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> r = 1;
> goto out;
> }
> - if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu))
> - record_steal_time(vcpu);
> if (kvm_check_request(KVM_REQ_NMI, vcpu))
> process_nmi(vcpu);
> req_immediate_exit =
> @@ -5672,6 +5669,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> }
> }
>
> + record_steal_time(vcpu);
> +
> r = kvm_mmu_reload(vcpu);
> if (unlikely(r)) {
> goto cancel_injection;
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 91ae127..5476ffc 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -112,27 +112,26 @@ static inline bool is_error_page(struct page *page)
> /*
> * vcpu->requests bit members
> */
> -#define KVM_REQ_TLB_FLUSH 0
> -#define KVM_REQ_MIGRATE_TIMER 1
> -#define KVM_REQ_REPORT_TPR_ACCESS 2
> -#define KVM_REQ_MMU_RELOAD 3
> -#define KVM_REQ_TRIPLE_FAULT 4
> -#define KVM_REQ_PENDING_TIMER 5
> -#define KVM_REQ_UNHALT 6
> -#define KVM_REQ_MMU_SYNC 7
> -#define KVM_REQ_CLOCK_UPDATE 8
> -#define KVM_REQ_KICK 9
> -#define KVM_REQ_DEACTIVATE_FPU 10
> -#define KVM_REQ_EVENT 11
> -#define KVM_REQ_APF_HALT 12
> -#define KVM_REQ_STEAL_UPDATE 13
> -#define KVM_REQ_NMI 14
> -#define KVM_REQ_IMMEDIATE_EXIT 15
> -#define KVM_REQ_PMU 16
> -#define KVM_REQ_PMI 17
> -#define KVM_REQ_WATCHDOG 18
> -#define KVM_REQ_MASTERCLOCK_UPDATE 19
> -#define KVM_REQ_MCLOCK_INPROGRESS 20
> +#define KVM_REQ_TLB_FLUSH 0
> +#define KVM_REQ_MIGRATE_TIMER 1
> +#define KVM_REQ_REPORT_TPR_ACCESS 2
> +#define KVM_REQ_MMU_RELOAD 3
> +#define KVM_REQ_TRIPLE_FAULT 4
> +#define KVM_REQ_PENDING_TIMER 5
> +#define KVM_REQ_UNHALT 6
> +#define KVM_REQ_MMU_SYNC 7
> +#define KVM_REQ_CLOCK_UPDATE 8
> +#define KVM_REQ_KICK 9
> +#define KVM_REQ_DEACTIVATE_FPU 10
> +#define KVM_REQ_EVENT 11
> +#define KVM_REQ_APF_HALT 12
> +#define KVM_REQ_NMI 13
> +#define KVM_REQ_IMMEDIATE_EXIT 14
> +#define KVM_REQ_PMU 15
> +#define KVM_REQ_PMI 16
> +#define KVM_REQ_WATCHDOG 17
> +#define KVM_REQ_MASTERCLOCK_UPDATE 18
> +#define KVM_REQ_MCLOCK_INPROGRESS 19
>
> #define KVM_USERSPACE_IRQ_SOURCE_ID 0
> #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID 1
> --
> 1.7.5.4

--
Gleb.