From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 3/3] alleviate time drift with HPET periodic timers Date: Tue, 22 Mar 2011 11:08:10 +0100 Message-ID: <4D88750A.4050304@siemens.com> References: <996058219.455940.1300788200730.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm , gcosta@redhat.com, Anthony Liguori , aliguori@us.ibm.com To: Ulrich Obergfell Return-path: Received: from goliath.siemens.de ([192.35.17.28]:20296 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753406Ab1CVKIW (ORCPT ); Tue, 22 Mar 2011 06:08:22 -0400 In-Reply-To: <996058219.455940.1300788200730.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-03-22 11:03, Ulrich Obergfell wrote: > >>> Part 3 of the patch implements the following options for the >>> 'configure' script. >>> >>> --disable-hpet-driftfix >>> --enable-hpet-driftfix >> >> I see no benefit in this configurability. Just make the driftfix >> unconditionally available, runtime-disabled by default for now until it >> matured and there is no downside in enabling it all the time. > > > Many Thanks Jan, > > I enclosed the code in '#ifdef CONFIG_HPET_DRIFTFIX ... #endif' > so that it can be easily identified (and removed if the generic API > would be implemented some day). Since the ifdef's are already there > I added the configuration option for convenience. As you don't see > any benefit in this option, I can remove that part of the patch. > However, I'd suggest to keep the ifdef's and do the following: > > - Rename to '#ifdef HPET_DRIFTFIX ... #endif' to make it clear that > this is not controlled via a configuration option. > > - Add '#define HPET_DRIFTFIX' to hw/hpet_emul.h. > > Do you agree ? Thanks to versioning control and feature-oriented commits, it's not very hard to identify what code changes relate to which feature additions. So I still don't see a need for that. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41221 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q1yVb-0004UE-DT for qemu-devel@nongnu.org; Tue, 22 Mar 2011 06:08:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q1yVa-0003hO-CH for qemu-devel@nongnu.org; Tue, 22 Mar 2011 06:08:15 -0400 Received: from goliath.siemens.de ([192.35.17.28]:22253) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q1yVa-0003hE-1C for qemu-devel@nongnu.org; Tue, 22 Mar 2011 06:08:14 -0400 Message-ID: <4D88750A.4050304@siemens.com> Date: Tue, 22 Mar 2011 11:08:10 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <996058219.455940.1300788200730.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> In-Reply-To: <996058219.455940.1300788200730.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 3/3] alleviate time drift with HPET periodic timers List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ulrich Obergfell Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, kvm , gcosta@redhat.com On 2011-03-22 11:03, Ulrich Obergfell wrote: > >>> Part 3 of the patch implements the following options for the >>> 'configure' script. >>> >>> --disable-hpet-driftfix >>> --enable-hpet-driftfix >> >> I see no benefit in this configurability. Just make the driftfix >> unconditionally available, runtime-disabled by default for now until it >> matured and there is no downside in enabling it all the time. > > > Many Thanks Jan, > > I enclosed the code in '#ifdef CONFIG_HPET_DRIFTFIX ... #endif' > so that it can be easily identified (and removed if the generic API > would be implemented some day). Since the ifdef's are already there > I added the configuration option for convenience. As you don't see > any benefit in this option, I can remove that part of the patch. > However, I'd suggest to keep the ifdef's and do the following: > > - Rename to '#ifdef HPET_DRIFTFIX ... #endif' to make it clear that > this is not controlled via a configuration option. > > - Add '#define HPET_DRIFTFIX' to hw/hpet_emul.h. > > Do you agree ? Thanks to versioning control and feature-oriented commits, it's not very hard to identify what code changes relate to which feature additions. So I still don't see a need for that. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux