All of lore.kernel.org
 help / color / mirror / Atom feed
From: Aurelien Jarno <aurelien@aurel32.net>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/3] use nanoseconds everywhere for timeout computation
Date: Tue, 1 Feb 2011 19:47:18 +0100	[thread overview]
Message-ID: <20110201184718.GF7112@hall.aurel32.net> (raw)
In-Reply-To: <4D473510.7020209@codemonkey.ws>

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<pbonzini@redhat.com>
>>    
>
> 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

  reply	other threads:[~2011-02-01 18:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-31 21:51 [Qemu-devel] [PATCH 0/3] Simplify and fix alarm deadline computation Paolo Bonzini
2011-01-31 21:51 ` [Qemu-devel] [PATCH 1/3] use nanoseconds everywhere for timeout computation Paolo Bonzini
2011-01-31 22:17   ` Anthony Liguori
2011-02-01 18:47     ` Aurelien Jarno [this message]
2011-02-02  8:04       ` [Qemu-devel] " Paolo Bonzini
2011-02-01 18:38   ` [Qemu-devel] " Aurelien Jarno
2011-01-31 21:51 ` [Qemu-devel] [PATCH 2/3] Correct alarm deadline computation Paolo Bonzini
2011-02-01 13:01   ` [Qemu-devel] " Jan Kiszka
2011-02-01 13:04     ` Paolo Bonzini
2011-02-01 13:07       ` Jan Kiszka
2011-01-31 21:51 ` [Qemu-devel] [PATCH 3/3] Unify " Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110201184718.GF7112@hall.aurel32.net \
    --to=aurelien@aurel32.net \
    --cc=anthony@codemonkey.ws \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.