[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Replacing (and removing) get_ticks_per_sec() fu
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [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 /
>> > NANOSECONDS_PER_SECOND;
> [...]
>
> 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
reasons.
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
10^9.
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
timer_mod(fdctrl->result_timer,
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.
Paolo
Re: [Qemu-devel] [PATCH] Replacing (and removing) get_ticks_per_sec() function with NANOSECONDS_PER_SECOND Signed-off-by: Rutuja Shah <address@hidden>, rutuja shah, 2016/03/12
Re: [Qemu-devel] [Qemu-ppc] [PATCH] Replacing (and removing) get_ticks_per_sec() function with NANOSECONDS_PER_SECOND Signed-off-by: Rutuja Shah <address@hidden>, Laurent Vivier, 2016/03/15