All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: LAPIC: Loose fluctuation filter for auto tune lapic_timer_advance_ns
@ 2019-09-25  5:47 Wanpeng Li
  2019-09-25 19:29 ` Sean Christopherson
  0 siblings, 1 reply; 3+ messages in thread
From: Wanpeng Li @ 2019-09-25  5:47 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Paolo Bonzini, Radim Krčmář,
	Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel

From: Wanpeng Li <wanpengli@tencent.com>

5000 guest cycles delta is easy to encounter on desktop, per-vCPU 
lapic_timer_advance_ns always keeps at 1000ns initial value, lets 
loose fluctuation filter a bit to make auto tune can make some 
progress.

Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
 arch/x86/kvm/lapic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 3a3a685..258407e 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -67,7 +67,7 @@
 
 static bool lapic_timer_advance_dynamic __read_mostly;
 #define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100
-#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000
+#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 10000
 #define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000
 /* step-by-step approximation to mitigate fluctuation */
 #define LAPIC_TIMER_ADVANCE_ADJUST_STEP 8
@@ -1504,7 +1504,7 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
 		timer_advance_ns += ns/LAPIC_TIMER_ADVANCE_ADJUST_STEP;
 	}
 
-	if (unlikely(timer_advance_ns > LAPIC_TIMER_ADVANCE_ADJUST_MAX))
+	if (unlikely(timer_advance_ns > LAPIC_TIMER_ADVANCE_ADJUST_MAX/2))
 		timer_advance_ns = LAPIC_TIMER_ADVANCE_ADJUST_INIT;
 	apic->lapic_timer.timer_advance_ns = timer_advance_ns;
 }
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: LAPIC: Loose fluctuation filter for auto tune lapic_timer_advance_ns
  2019-09-25  5:47 [PATCH] KVM: LAPIC: Loose fluctuation filter for auto tune lapic_timer_advance_ns Wanpeng Li
@ 2019-09-25 19:29 ` Sean Christopherson
  2019-09-26  0:19   ` Wanpeng Li
  0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2019-09-25 19:29 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: linux-kernel, kvm, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel

On Wed, Sep 25, 2019 at 01:47:04PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> 5000 guest cycles delta is easy to encounter on desktop, per-vCPU 
> lapic_timer_advance_ns always keeps at 1000ns initial value, lets 
> loose fluctuation filter a bit to make auto tune can make some 
> progress.
> 
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>  arch/x86/kvm/lapic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 3a3a685..258407e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -67,7 +67,7 @@
>  
>  static bool lapic_timer_advance_dynamic __read_mostly;
>  #define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100
> -#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000
> +#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 10000
>  #define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000
>  /* step-by-step approximation to mitigate fluctuation */
>  #define LAPIC_TIMER_ADVANCE_ADJUST_STEP 8
> @@ -1504,7 +1504,7 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
>  		timer_advance_ns += ns/LAPIC_TIMER_ADVANCE_ADJUST_STEP;
>  	}
>  
> -	if (unlikely(timer_advance_ns > LAPIC_TIMER_ADVANCE_ADJUST_MAX))
> +	if (unlikely(timer_advance_ns > LAPIC_TIMER_ADVANCE_ADJUST_MAX/2))

Doh, missed that these are two different time domains in the original
review, i.e. ns vs. cycles.

We should use separate defines for the filter since that check is done
in cycles.  Not sure what names to use to keep things somewhat clear.

Maybe s/ADJUST/EXPIRE for the cycles, and s/ADJUST/NS for the ns ones?
E.g.:

#define LAPIC_TIMER_ADVANCE_EXPIRE_MIN	100
#define LAPIC_TIMER_ADVANCE_EXPIRE_MAX	10000
#define LAPIC_TIMER_ADVANCE_NS_MAX	5000
#define LAPIC_TIMER_ADVANCE_NS_INIT	1000

>  		timer_advance_ns = LAPIC_TIMER_ADVANCE_ADJUST_INIT;
>  	apic->lapic_timer.timer_advance_ns = timer_advance_ns;
>  }
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] KVM: LAPIC: Loose fluctuation filter for auto tune lapic_timer_advance_ns
  2019-09-25 19:29 ` Sean Christopherson
@ 2019-09-26  0:19   ` Wanpeng Li
  0 siblings, 0 replies; 3+ messages in thread
From: Wanpeng Li @ 2019-09-26  0:19 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: LKML, kvm, Paolo Bonzini, Radim Krčmář,
	Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel

On Thu, 26 Sep 2019 at 03:29, Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Wed, Sep 25, 2019 at 01:47:04PM +0800, Wanpeng Li wrote:
> > From: Wanpeng Li <wanpengli@tencent.com>
> >
> > 5000 guest cycles delta is easy to encounter on desktop, per-vCPU
> > lapic_timer_advance_ns always keeps at 1000ns initial value, lets
> > loose fluctuation filter a bit to make auto tune can make some
> > progress.
> >
> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> > ---
> >  arch/x86/kvm/lapic.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 3a3a685..258407e 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -67,7 +67,7 @@
> >
> >  static bool lapic_timer_advance_dynamic __read_mostly;
> >  #define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100
> > -#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000
> > +#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 10000
> >  #define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000
> >  /* step-by-step approximation to mitigate fluctuation */
> >  #define LAPIC_TIMER_ADVANCE_ADJUST_STEP 8
> > @@ -1504,7 +1504,7 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
> >               timer_advance_ns += ns/LAPIC_TIMER_ADVANCE_ADJUST_STEP;
> >       }
> >
> > -     if (unlikely(timer_advance_ns > LAPIC_TIMER_ADVANCE_ADJUST_MAX))
> > +     if (unlikely(timer_advance_ns > LAPIC_TIMER_ADVANCE_ADJUST_MAX/2))
>
> Doh, missed that these are two different time domains in the original
> review, i.e. ns vs. cycles.

I try to save one #define in this patch, will fold below in next version.

    Wanpeng

>
> We should use separate defines for the filter since that check is done
> in cycles.  Not sure what names to use to keep things somewhat clear.
>
> Maybe s/ADJUST/EXPIRE for the cycles, and s/ADJUST/NS for the ns ones?
> E.g.:
>
> #define LAPIC_TIMER_ADVANCE_EXPIRE_MIN  100
> #define LAPIC_TIMER_ADVANCE_EXPIRE_MAX  10000
> #define LAPIC_TIMER_ADVANCE_NS_MAX      5000
> #define LAPIC_TIMER_ADVANCE_NS_INIT     1000
>
> >               timer_advance_ns = LAPIC_TIMER_ADVANCE_ADJUST_INIT;
> >       apic->lapic_timer.timer_advance_ns = timer_advance_ns;
> >  }
> > --
> > 2.7.4
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-09-26  0:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25  5:47 [PATCH] KVM: LAPIC: Loose fluctuation filter for auto tune lapic_timer_advance_ns Wanpeng Li
2019-09-25 19:29 ` Sean Christopherson
2019-09-26  0:19   ` Wanpeng Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.