diff mbox series

[v3,2/4] KVM: LAPIC: lapic timer interrupt is injected by posted interrupt

Message ID 1560255429-7105-3-git-send-email-wanpengli@tencent.com (mailing list archive)
State New, archived
Headers show
Series KVM: LAPIC: Implement Exitless Timer | expand

Commit Message

Wanpeng Li June 11, 2019, 12:17 p.m. UTC 
From: Wanpeng Li <wanpengli@tencent.com>

Dedicated instances are currently disturbed by unnecessary jitter due 
to the emulated lapic timers fire on the same pCPUs which vCPUs resident.
There is no hardware virtual timer on Intel for guest like ARM. Both 
programming timer in guest and the emulated timer fires incur vmexits.
This patch tries to avoid vmexit which is incurred by the emulated 
timer fires in dedicated instance scenario. 

When nohz_full is enabled in dedicated instances scenario, the emulated 
timers can be offload to the nearest busy housekeeping cpus since APICv 
is really common in recent years. The guest timer interrupt is injected 
by posted-interrupt which is delivered by housekeeping cpu once the emulated 
timer fires. 

~3% redis performance benefit can be observed on Skylake server.

w/o patch:

            VM-EXIT  Samples  Samples%  Time%   Min Time  Max Time   Avg time

EXTERNAL_INTERRUPT    42916    49.43%   39.30%   0.47us   106.09us   0.71us ( +-   1.09% )

w/ patch:

            VM-EXIT  Samples  Samples%  Time%   Min Time  Max Time         Avg time

EXTERNAL_INTERRUPT    6871     9.29%     2.96%   0.44us    57.88us   0.72us ( +-   4.02% )

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 arch/x86/kvm/lapic.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

Comments

Marcelo Tosatti June 11, 2019, 8:18 p.m. UTC | #1 
On Tue, Jun 11, 2019 at 08:17:07PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> Dedicated instances are currently disturbed by unnecessary jitter due 
> to the emulated lapic timers fire on the same pCPUs which vCPUs resident.
> There is no hardware virtual timer on Intel for guest like ARM. Both 
> programming timer in guest and the emulated timer fires incur vmexits.
> This patch tries to avoid vmexit which is incurred by the emulated 
> timer fires in dedicated instance scenario. 
> 
> When nohz_full is enabled in dedicated instances scenario, the emulated 
> timers can be offload to the nearest busy housekeeping cpus since APICv 
> is really common in recent years. The guest timer interrupt is injected 
> by posted-interrupt which is delivered by housekeeping cpu once the emulated 
> timer fires. 
> 
> ~3% redis performance benefit can be observed on Skylake server.
> 
> w/o patch:
> 
>             VM-EXIT  Samples  Samples%  Time%   Min Time  Max Time   Avg time
> 
> EXTERNAL_INTERRUPT    42916    49.43%   39.30%   0.47us   106.09us   0.71us ( +-   1.09% )
> 
> w/ patch:
> 
>             VM-EXIT  Samples  Samples%  Time%   Min Time  Max Time         Avg time
> 
> EXTERNAL_INTERRUPT    6871     9.29%     2.96%   0.44us    57.88us   0.72us ( +-   4.02% )
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>  arch/x86/kvm/lapic.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index e57eeba..020599f 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -133,6 +133,12 @@ inline bool posted_interrupt_inject_timer_enabled(struct kvm_vcpu *vcpu)
>  }
>  EXPORT_SYMBOL_GPL(posted_interrupt_inject_timer_enabled);
>  
> +static inline bool can_posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> +{
> +	return posted_interrupt_inject_timer_enabled(vcpu) &&
> +		kvm_hlt_in_guest(vcpu->kvm);
> +}

Hi Li,

Don't think its necessary to depend on kvm_hlt_in_guest: Can also use
exitless injection if the guest is running (think DPDK style workloads
that busy-spin on network card).
Wanpeng Li June 12, 2019, 1:48 a.m. UTC | #2 
On Wed, 12 Jun 2019 at 04:39, Marcelo Tosatti <mtosatti@redhat.com> wrote:
>
> On Tue, Jun 11, 2019 at 08:17:07PM +0800, Wanpeng Li wrote:
> > From: Wanpeng Li <wanpengli@tencent.com>
> >
> > Dedicated instances are currently disturbed by unnecessary jitter due
> > to the emulated lapic timers fire on the same pCPUs which vCPUs resident.
> > There is no hardware virtual timer on Intel for guest like ARM. Both
> > programming timer in guest and the emulated timer fires incur vmexits.
> > This patch tries to avoid vmexit which is incurred by the emulated
> > timer fires in dedicated instance scenario.
> >
> > When nohz_full is enabled in dedicated instances scenario, the emulated
> > timers can be offload to the nearest busy housekeeping cpus since APICv
> > is really common in recent years. The guest timer interrupt is injected
> > by posted-interrupt which is delivered by housekeeping cpu once the emulated
> > timer fires.
> >
> > ~3% redis performance benefit can be observed on Skylake server.
> >
> > w/o patch:
> >
> >             VM-EXIT  Samples  Samples%  Time%   Min Time  Max Time   Avg time
> >
> > EXTERNAL_INTERRUPT    42916    49.43%   39.30%   0.47us   106.09us   0.71us ( +-   1.09% )
> >
> > w/ patch:
> >
> >             VM-EXIT  Samples  Samples%  Time%   Min Time  Max Time         Avg time
> >
> > EXTERNAL_INTERRUPT    6871     9.29%     2.96%   0.44us    57.88us   0.72us ( +-   4.02% )
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> > ---
> >  arch/x86/kvm/lapic.c | 32 +++++++++++++++++++++++++-------
> >  1 file changed, 25 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index e57eeba..020599f 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -133,6 +133,12 @@ inline bool posted_interrupt_inject_timer_enabled(struct kvm_vcpu *vcpu)
> >  }
> >  EXPORT_SYMBOL_GPL(posted_interrupt_inject_timer_enabled);
> >
> > +static inline bool can_posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> > +{
> > +     return posted_interrupt_inject_timer_enabled(vcpu) &&
> > +             kvm_hlt_in_guest(vcpu->kvm);
> > +}
>
> Hi Li,

Hi Marcelo,

>
> Don't think its necessary to depend on kvm_hlt_in_guest: Can also use
> exitless injection if the guest is running (think DPDK style workloads
> that busy-spin on network card).
>

There are some discussions here.

https://lkml.org/lkml/2019/6/11/424
https://lkml.org/lkml/2019/6/5/436

Regards,
Wanpeng Li
Radim Krčmář June 12, 2019, 3:22 p.m. UTC | #3 
2019-06-12 09:48+0800, Wanpeng Li:
> On Wed, 12 Jun 2019 at 04:39, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > On Tue, Jun 11, 2019 at 08:17:07PM +0800, Wanpeng Li wrote:
> > > From: Wanpeng Li <wanpengli@tencent.com>
> > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > > @@ -133,6 +133,12 @@ inline bool posted_interrupt_inject_timer_enabled(struct kvm_vcpu *vcpu)
> > >  }
> > >  EXPORT_SYMBOL_GPL(posted_interrupt_inject_timer_enabled);
> > >
> > > +static inline bool can_posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> > > +{
> > > +     return posted_interrupt_inject_timer_enabled(vcpu) &&
> > > +             kvm_hlt_in_guest(vcpu->kvm);
> > > +}
> >
> > Hi Li,
> 
> Hi Marcelo,
> 
> >
> > Don't think its necessary to depend on kvm_hlt_in_guest: Can also use
> > exitless injection if the guest is running (think DPDK style workloads
> > that busy-spin on network card).

I agree.

> There are some discussions here.
> 
> https://lkml.org/lkml/2019/6/11/424
> https://lkml.org/lkml/2019/6/5/436

Paolo wants to disable the APF synthetic halt first, which I think is
unrelated to the timer implementation.
The synthetic halt happens when the VCPU cannot progress because the
host swapped out its memory and any asynchronous event should unhalt it,
because we assume that the interrupt path wasn't swapped out.

The posted interrupt does a swake_up_one (part of vcpu kick), which is
everything what the non-posted path does after setting a KVM request --
it's a bug if we later handle the PIR differently from the KVM request,
so the guest is going to be woken up on any halt blocking in KVM (even
synthetic APF halt).

Paolo, have I missed the point?

Thanks.
Marcelo Tosatti June 12, 2019, 9:51 p.m. UTC | #4 
On Wed, Jun 12, 2019 at 05:22:31PM +0200, Radim Krčmář wrote:
> 2019-06-12 09:48+0800, Wanpeng Li:
> > On Wed, 12 Jun 2019 at 04:39, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > > On Tue, Jun 11, 2019 at 08:17:07PM +0800, Wanpeng Li wrote:
> > > > From: Wanpeng Li <wanpengli@tencent.com>
> > > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > > > @@ -133,6 +133,12 @@ inline bool posted_interrupt_inject_timer_enabled(struct kvm_vcpu *vcpu)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(posted_interrupt_inject_timer_enabled);
> > > >
> > > > +static inline bool can_posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +     return posted_interrupt_inject_timer_enabled(vcpu) &&
> > > > +             kvm_hlt_in_guest(vcpu->kvm);
> > > > +}
> > >
> > > Hi Li,
> > 
> > Hi Marcelo,
> > 
> > >
> > > Don't think its necessary to depend on kvm_hlt_in_guest: Can also use
> > > exitless injection if the guest is running (think DPDK style workloads
> > > that busy-spin on network card).
> 
> I agree.
> 
> > There are some discussions here.
> > 
> > https://lkml.org/lkml/2019/6/11/424
> > https://lkml.org/lkml/2019/6/5/436
> 
> Paolo wants to disable the APF synthetic halt first, which I think is
> unrelated to the timer implementation.
> The synthetic halt happens when the VCPU cannot progress because the
> host swapped out its memory and any asynchronous event should unhalt it,
> because we assume that the interrupt path wasn't swapped out.
> 
> The posted interrupt does a swake_up_one (part of vcpu kick), which is
> everything what the non-posted path does after setting a KVM request --
> it's a bug if we later handle the PIR differently from the KVM request,
> so the guest is going to be woken up on any halt blocking in KVM (even
> synthetic APF halt).
> 
> Paolo, have I missed the point?
> 
> Thanks.

"Here you need to check kvm_halt_in_guest, not kvm_mwait_in_guest,
because you need to go through kvm_apic_expired if the guest needs to be
woken up from kvm_vcpu_block."

Note: VMX preemption timer is disabled by Li's patch.
diff mbox series

Patch

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e57eeba..020599f 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -133,6 +133,12 @@  inline bool posted_interrupt_inject_timer_enabled(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(posted_interrupt_inject_timer_enabled);
 
+static inline bool can_posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
+{
+	return posted_interrupt_inject_timer_enabled(vcpu) &&
+		kvm_hlt_in_guest(vcpu->kvm);
+}
+
 static inline bool kvm_apic_map_get_logical_dest(struct kvm_apic_map *map,
 		u32 dest_id, struct kvm_lapic ***cluster, u16 *mask) {
 	switch (map->mode) {
@@ -1441,6 +1447,19 @@  static void apic_update_lvtt(struct kvm_lapic *apic)
 	}
 }
 
+static void kvm_apic_inject_pending_timer_irqs(struct kvm_lapic *apic)
+{
+	struct kvm_timer *ktimer = &apic->lapic_timer;
+
+	kvm_apic_local_deliver(apic, APIC_LVTT);
+	if (apic_lvtt_tscdeadline(apic))
+		ktimer->tscdeadline = 0;
+	if (apic_lvtt_oneshot(apic)) {
+		ktimer->tscdeadline = 0;
+		ktimer->target_expiration = 0;
+	}
+}
+
 static void apic_timer_expired(struct kvm_lapic *apic)
 {
 	struct kvm_vcpu *vcpu = apic->vcpu;
@@ -1450,6 +1469,11 @@  static void apic_timer_expired(struct kvm_lapic *apic)
 	if (atomic_read(&apic->lapic_timer.pending))
 		return;
 
+	if (can_posted_interrupt_inject_timer(apic->vcpu)) {
+		kvm_apic_inject_pending_timer_irqs(apic);
+		return;
+	}
+
 	atomic_inc(&apic->lapic_timer.pending);
 	kvm_set_pending_timer(vcpu);
 
@@ -2386,13 +2410,7 @@  void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
 	if (atomic_read(&apic->lapic_timer.pending) > 0) {
-		kvm_apic_local_deliver(apic, APIC_LVTT);
-		if (apic_lvtt_tscdeadline(apic))
-			apic->lapic_timer.tscdeadline = 0;
-		if (apic_lvtt_oneshot(apic)) {
-			apic->lapic_timer.tscdeadline = 0;
-			apic->lapic_timer.target_expiration = 0;
-		}
+		kvm_apic_inject_pending_timer_irqs(apic);
 		atomic_set(&apic->lapic_timer.pending, 0);
 	}
 }