From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=43642 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PkLGB-0005Lk-Ub for qemu-devel@nongnu.org; Tue, 01 Feb 2011 13:47:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PkLG5-0004av-9K for qemu-devel@nongnu.org; Tue, 01 Feb 2011 13:47:22 -0500 Received: from hall.aurel32.net ([88.191.126.93]:55334) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PkLG5-0004an-3N for qemu-devel@nongnu.org; Tue, 01 Feb 2011 13:47:21 -0500 Date: Tue, 1 Feb 2011 19:47:18 +0100 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH 1/3] use nanoseconds everywhere for timeout computation Message-ID: <20110201184718.GF7112@hall.aurel32.net> References: <1296510679-12268-1-git-send-email-pbonzini@redhat.com> <1296510679-12268-2-git-send-email-pbonzini@redhat.com> <4D473510.7020209@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <4D473510.7020209@codemonkey.ws> Sender: Aurelien Jarno List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Paolo Bonzini , qemu-devel@nongnu.org On Mon, Jan 31, 2011 at 04:17:52PM -0600, Anthony Liguori wrote: > On 01/31/2011 03:51 PM, Paolo Bonzini wrote: >> Suggested by Aurelien Jarno. >> >> Signed-off-by: Paolo Bonzini >> > > Something I've found is that we have a lot of bugs that are the result > of unit conversions when the unit can't be mapped directly to base 10. > > This happens in both the PIT and RTC and probably every other fixed bus > speed based clock we support. > > I think it would be better to have a Unit argument to qemu_get_clock to > specify the desired units. That way, we could request NS as the time > unit or something like PC_FREQ0 which would map to the bus speed of the > PIT. It's more or less what is already done with the two versions of the functions, qemu_get_clock() which can return different unit depending on what you ask (and in my opinion should simply disappear because it's the best way to have bugs), and qemu_get_clock_ns() that always return it in nanoseconds. What you suggests is a generalization of this second function to different units than ns. That's an option, but we should pay attention that given we work in integer, a conversion decrease the precision, so it should usually be done at the last moment to keep the precision correct (assuming ns is precise enough, which I guess is true). In any case I think this patch series should be applied as it fixes a real bug. It's already a step in the right direction, as it removes useless conversions, as all the involved functions use ns in fine. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net