[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH] Replacing (and removing) get_ticks_per_sec() fu

From: Paolo Bonzini
Subject: Re: [Qemu-block] [PATCH] Replacing (and removing) get_ticks_per_sec() function with NANOSECONDS_PER_SECOND Signed-off-by: Rutuja Shah <address@hidden>
Date: Fri, 11 Mar 2016 13:07:06 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 11/03/2016 12:44, Christian Borntraeger wrote:
>> -    s->tick_offset_vmstate = s->tick_offset + delta / get_ticks_per_sec();
>> > +    s->tick_offset_vmstate = s->tick_offset + delta / 
> [...]
> While technically correct, I do not like these changes. The interfaces expect 
> "ticks",
> and the fact that this happens to be a nanosecond does not help regarding 
> readability.

Actually, I think usage of "tick" in this file is just for historical

A long time ago QEMU had a timer that ticked every millisecond and the
timers were not in nanosecond precision; rather they used
cpu_get_ticks() which was tied to the TSC.  This was changed in 2006,
and since then the number of ticks per second was changed to a constant

Usage of "tick" to represent nanoseconds is definitely the exception.
It's much more common to have code like:

            now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
                ticks = muldiv64(now, 32768, get_ticks_per_sec());

(hw/arm/omap1.c) that converts nanoseconds to 32768 Hz "ticks", and
using get_ticks_per_sec() is very confusing in the latter example.  You
would think that get_ticks_per_sec() is in the numerator (second
argument of muldiv64), not in the denominator!

Most of the time, get_ticks_per_sec()'s result end up being massaged in
some formula and passed to timer_mod which expects nanoseconds, such as

              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
              (get_ticks_per_sec() / 50));

where you have nanoseconds on the left of the plus sign and "ticks" on
the right.  NANOSECONDS_PER_SECOND makes it obvious that the timer will
expire in 1/50th of a second.

> If - for some reason - we want to replace a function with a MACRO, then
> please introduce TICKS_PER_SECOND which just feels better when reading the 
> code.

This would be wrong, for the reason mentioned above.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]