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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
