Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 10 Jun 2019 18:51:27 +0200
From:   Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
To:     Wanpeng Li <kernellwp@gmail.com>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v2 2/3] KVM: LAPIC: lapic timer interrupt is injected by
 posted interrupt
Message-ID: <20190610165127.GA8389@flask>
References: <1559799086-13912-1-git-send-email-wanpengli@tencent.com>
 <1559799086-13912-3-git-send-email-wanpengli@tencent.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1559799086-13912-3-git-send-email-wanpengli@tencent.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

2019-06-06 13:31+0800, Wanpeng Li:
> 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%~5% redis performance benefit can be observed on Skylake server.
> 
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>  arch/x86/kvm/lapic.c | 32 +++++++++++++++++++++++++-------
>  arch/x86/kvm/x86.h   |  5 +++++
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 09b7387..c08e5a8 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -133,6 +133,12 @@ static inline bool posted_interrupt_inject_timer_enabled(struct kvm_vcpu *vcpu)
>  		kvm_mwait_in_guest(vcpu->kvm);
>  }
>  
> +static inline bool can_posted_interrupt_inject_timer(struct kvm_vcpu *vcpu)
> +{
> +	return posted_interrupt_inject_timer_enabled(vcpu) &&
> +		!vcpu_halt_in_guest(vcpu);

It would make more sense to have a condition for general blocking in
KVM, but keep in mind that we're not running on the same cpu anymore, so
any code like that has to be properly protected against VM entries under
our hands.  (The VCPU could appear halted here, but before we get make
the timer pending, the VCPU would enter and potentially never check the
interrupt.)

I think we should be able to simply do

  if (posted_interrupt_inject_timer_enabled(vcpu))
  	kvm_inject_apic_timer_irqs();

directly in the apic_timer_expired() as the injection will wake up the
target if necessary.  It's going to be a bit slow for timer callback in
those (too slow to warrant special handling?), but there hopefully
aren't any context restrictions in place.


Thanks.