From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: KVM: x86: add module parameter to disable periodic kvmclock sync Date: Thu, 13 Nov 2014 18:46:59 +0100 Message-ID: <20141113174658.GA6801@potion.brq.redhat.com> References: <20141113024439.GA7402@amt.cnet> <20141113084040.GA3193@hawk.usersys.redhat.com> <20141113104401.GA4325@hawk.usersys.redhat.com> <20141113113219.GA5233@hawk.usersys.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marcelo Tosatti , Paolo Bonzini , kvm-devel To: Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:45459 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933296AbaKMRrD (ORCPT ); Thu, 13 Nov 2014 12:47:03 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sADHl2Yg016118 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 13 Nov 2014 12:47:02 -0500 Content-Disposition: inline In-Reply-To: <20141113113219.GA5233@hawk.usersys.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 2014-11-13 12:32+0100, Andrew Jones: > On Thu, Nov 13, 2014 at 11:44:02AM +0100, Andrew Jones wrote: > > On Thu, Nov 13, 2014 at 09:40:41AM +0100, Andrew Jones wrote: > > > On Thu, Nov 13, 2014 at 12:44:39AM -0200, Marcelo Tosatti wrote: > > > > The periodic kvmclock sync can be an undesired source of latencies. [...] > > > > +static bool kvmclock_periodic_sync = 1; > > > > > > Using 'true' would look nicer. > > > > Ahh, disregard this comment. 1 matches what the user would input. The output is always Y|N and the user can input that as well. (bool = {true, false}, so I'd prefer 'true'.) > @@ -1717,7 +1717,8 @@ static void kvmclock_sync_fn(struct work_struct *work) > kvmclock_sync_work); > struct kvm *kvm = container_of(ka, struct kvm, arch); > > - schedule_delayed_work(&kvm->arch.kvmclock_update_work, 0); > + if (kvmclock_periodic_sync) > + schedule_delayed_work(&kvm->arch.kvmclock_update_work, 0); > schedule_delayed_work(&kvm->arch.kvmclock_sync_work, > KVMCLOCK_SYNC_PERIOD); Yes, or add callbacks to sysfs writes that would schedule/cancel this work. (But having a for_every_vm loop is quite ugly.) I'd be happy with a 'const kvmclock_periodic_sync'. (Having useless timers is weird if we care about latencies.)