On 17/09/19 10:16, Wanpeng Li wrote: > From: Wanpeng Li <wanpengli@xxxxxxxxxxx> > > Filter out drastic fluctuation and random fluctuation, remove > timer_advance_adjust_done altogether, the adjustment would be > continuous. > > Signed-off-by: Wanpeng Li <wanpengli@xxxxxxxxxxx> Queued, thanks (I renamed the new variable to lapic_timer_advance_dynamic). Thanks, Paolo > --- > arch/x86/kvm/lapic.c | 28 ++++++++++++++-------------- > arch/x86/kvm/lapic.h | 1 - > 2 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index dbbe478..323bdca 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -65,7 +65,9 @@ > #define APIC_BROADCAST 0xFF > #define X2APIC_BROADCAST 0xFFFFFFFFul > > -#define LAPIC_TIMER_ADVANCE_ADJUST_DONE 100 > +static bool dynamically_adjust_timer_advance __read_mostly; > +#define LAPIC_TIMER_ADVANCE_ADJUST_MIN 100 > +#define LAPIC_TIMER_ADVANCE_ADJUST_MAX 5000 > #define LAPIC_TIMER_ADVANCE_ADJUST_INIT 1000 > /* step-by-step approximation to mitigate fluctuation */ > #define LAPIC_TIMER_ADVANCE_ADJUST_STEP 8 > @@ -1485,26 +1487,25 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu, > u32 timer_advance_ns = apic->lapic_timer.timer_advance_ns; > u64 ns; > > + /* Do not adjust for tiny fluctuations or large random spikes. */ > + if (abs(advance_expire_delta) > LAPIC_TIMER_ADVANCE_ADJUST_MAX || > + abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_MIN) > + return; > + > /* too early */ > if (advance_expire_delta < 0) { > ns = -advance_expire_delta * 1000000ULL; > do_div(ns, vcpu->arch.virtual_tsc_khz); > - timer_advance_ns -= min((u32)ns, > - timer_advance_ns / LAPIC_TIMER_ADVANCE_ADJUST_STEP); > + timer_advance_ns -= ns/LAPIC_TIMER_ADVANCE_ADJUST_STEP; > } else { > /* too late */ > ns = advance_expire_delta * 1000000ULL; > do_div(ns, vcpu->arch.virtual_tsc_khz); > - timer_advance_ns += min((u32)ns, > - timer_advance_ns / LAPIC_TIMER_ADVANCE_ADJUST_STEP); > + timer_advance_ns += ns/LAPIC_TIMER_ADVANCE_ADJUST_STEP; > } > > - if (abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_DONE) > - apic->lapic_timer.timer_advance_adjust_done = true; > - if (unlikely(timer_advance_ns > 5000)) { > + if (unlikely(timer_advance_ns > LAPIC_TIMER_ADVANCE_ADJUST_MAX)) > timer_advance_ns = LAPIC_TIMER_ADVANCE_ADJUST_INIT; > - apic->lapic_timer.timer_advance_adjust_done = false; > - } > apic->lapic_timer.timer_advance_ns = timer_advance_ns; > } > > @@ -1524,7 +1525,7 @@ static void __kvm_wait_lapic_expire(struct kvm_vcpu *vcpu) > if (guest_tsc < tsc_deadline) > __wait_lapic_expire(vcpu, tsc_deadline - guest_tsc); > > - if (unlikely(!apic->lapic_timer.timer_advance_adjust_done)) > + if (dynamically_adjust_timer_advance) > adjust_lapic_timer_advance(vcpu, apic->lapic_timer.advance_expire_delta); > } > > @@ -2302,13 +2303,12 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns) > apic->lapic_timer.timer.function = apic_timer_fn; > if (timer_advance_ns == -1) { > apic->lapic_timer.timer_advance_ns = LAPIC_TIMER_ADVANCE_ADJUST_INIT; > - apic->lapic_timer.timer_advance_adjust_done = false; > + dynamically_adjust_timer_advance = true; > } else { > apic->lapic_timer.timer_advance_ns = timer_advance_ns; > - apic->lapic_timer.timer_advance_adjust_done = true; > + dynamically_adjust_timer_advance = false; > } > > - > /* > * APIC is created enabled. This will prevent kvm_lapic_set_base from > * thinking that APIC state has changed. > diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h > index 50053d2..2aad7e2 100644 > --- a/arch/x86/kvm/lapic.h > +++ b/arch/x86/kvm/lapic.h > @@ -35,7 +35,6 @@ struct kvm_timer { > s64 advance_expire_delta; > atomic_t pending; /* accumulated triggered timers */ > bool hv_timer_in_use; > - bool timer_advance_adjust_done; > }; > > struct kvm_lapic { >