* [PATCH] KVM: Don't use vcpu->requests for steal time accounting
@ 2012-12-14 10:37 Takuya Yoshikawa
2012-12-14 11:28 ` Gleb Natapov
0 siblings, 1 reply; 4+ messages in thread
From: Takuya Yoshikawa @ 2012-12-14 10:37 UTC (permalink / raw)
To: mtosatti, gleb; +Cc: kvm
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.
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: Don't use vcpu->requests for steal time accounting
2012-12-14 10:37 [PATCH] KVM: Don't use vcpu->requests for steal time accounting Takuya Yoshikawa
@ 2012-12-14 11:28 ` Gleb Natapov
2012-12-14 15:12 ` Takuya Yoshikawa
0 siblings, 1 reply; 4+ messages in thread
From: Gleb Natapov @ 2012-12-14 11:28 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: mtosatti, kvm
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: Don't use vcpu->requests for steal time accounting
2012-12-14 11:28 ` Gleb Natapov
@ 2012-12-14 15:12 ` Takuya Yoshikawa
2012-12-14 15:41 ` Gleb Natapov
0 siblings, 1 reply; 4+ messages in thread
From: Takuya Yoshikawa @ 2012-12-14 15:12 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Takuya Yoshikawa, mtosatti, kvm
On Fri, 14 Dec 2012 13:28:15 +0200
Gleb Natapov <gleb@redhat.com> wrote:
> 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.
Yes, +1 "if" for the case we have nothing in requests.
I'm not sure if setting and clearing a bit for that minor
optimization is worth it.
Thanks,
Takuya
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: Don't use vcpu->requests for steal time accounting
2012-12-14 15:12 ` Takuya Yoshikawa
@ 2012-12-14 15:41 ` Gleb Natapov
0 siblings, 0 replies; 4+ messages in thread
From: Gleb Natapov @ 2012-12-14 15:41 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: Takuya Yoshikawa, mtosatti, kvm
On Sat, Dec 15, 2012 at 12:12:08AM +0900, Takuya Yoshikawa wrote:
> On Fri, 14 Dec 2012 13:28:15 +0200
> Gleb Natapov <gleb@redhat.com> wrote:
>
> > 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.
>
> Yes, +1 "if" for the case we have nothing in requests.
>
Almost any bit in requests can be replaced by one "if". Those
if's add up.
> I'm not sure if setting and clearing a bit for that minor
> optimization is worth it.
>
Setting/clearing it should be much more rare than guest entry.
--
Gleb.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-12-14 15:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-14 10:37 [PATCH] KVM: Don't use vcpu->requests for steal time accounting Takuya Yoshikawa
2012-12-14 11:28 ` Gleb Natapov
2012-12-14 15:12 ` Takuya Yoshikawa
2012-12-14 15:41 ` Gleb Natapov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox