From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755527AbcLZTow (ORCPT ); Mon, 26 Dec 2016 14:44:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36264 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755151AbcLZTot (ORCPT ); Mon, 26 Dec 2016 14:44:49 -0500 Date: Mon, 26 Dec 2016 17:44:25 -0200 From: Marcelo Tosatti To: Chen Yu Cc: kvm@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, "Rafael J. Wysocki" , Len Brown , Paolo Bonzini , Radim Krcmar , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Wanpeng Li Subject: Re: [PATCH][RFC] VM: x86: Return ealier if clocksource has not changed Message-ID: <20161226194422.GA30796@amt.cnet> References: <1482482513-24151-1-git-send-email-yu.c.chen@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1482482513-24151-1-git-send-email-yu.c.chen@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Mon, 26 Dec 2016 19:44:49 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Dec 23, 2016 at 04:41:53PM +0800, Chen Yu wrote: > Currently the notifier of pvclock_gtod_notify() get invoked > frequently due to the periodic update_wall_time(). This might > slow down the system a little bit as there might be redundant > execution code path and unnecessary lock contention > in update_pvclock_gtod(), which was found when I was doing > suspend/resume speed testings. As pvclock_gtod_notify() > should be invoked only when clocksource has changed, according to > Commit 16e8d74d2da9 ("KVM: x86: notifier for clocksource changes") > , either we can add a new notifier for clocksource switch, > or we can simply bypass the following code in pvclock_gtod_notify() > earlier if there is no clocksource switch. > > Cc: Paolo Bonzini > Cc: "Radim Krcmar" > Cc: Marcelo Tosatti > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: Wanpeng Li > Signed-off-by: Chen Yu > --- > arch/x86/kvm/x86.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 445c51b..54aa32d 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5961,13 +5961,14 @@ static int pvclock_gtod_notify(struct notifier_block *nb, unsigned long unused, > struct pvclock_gtod_data *gtod = &pvclock_gtod_data; > struct timekeeper *tk = priv; > > + if (likely(gtod->clock.vclock_mode == VCLOCK_TSC)) > + return 0; I think this is only safe if any of the values in "struct pvclock_gtod_data" are unchanged. Otherwise the local (KVM) copy is kept incorrect. > update_pvclock_gtod(tk); > > /* disable master clock if host does not trust, or does not > * use, TSC clocksource > */ > - if (gtod->clock.vclock_mode != VCLOCK_TSC && > - atomic_read(&kvm_guest_has_master_clock) != 0) > + if (atomic_read(&kvm_guest_has_master_clock) != 0) > queue_work(system_long_wq, &pvclock_gtod_work); > > return 0; > -- > 2.7.4