[Top][All Lists]

[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:26:44 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0

On 11/03/2016 13:12, Christian Borntraeger wrote:
> On 03/11/2016 01:07 PM, Paolo Bonzini wrote:
>> 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
>> reasons.
> So in essence the patch is ok and we should try to get rid of the "tick" word 
> in future
> patches?

Not necessarily.  The patch stops overloading the word "tick", so that
"tick" means "whatever the timer device counts".

In fact, you and I were both confused by the appearance of the word
"tick" in get_ticks_per_sec().  In the above line of code, the unit of
measure of delta is nanoseconds, because it is computed like this on the
line before:

    int64_t delta = qemu_clock_get_ns(rtc_clock)
                        - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);

therefore s->tick_offset_vmstate is *seconds*

       delta   /  get_ticks_per_sec()
       ^^^^^      ^^^^^^^^^^^^^^^^^^^
    nanosecond / (nanosecond / second)  = second

The code after Rutu's patch is easier to understand because "tick" now
means uniformly "whatever a PL031 counts" -- which is seconds.  Before,
"tick" on the left of the plus meant seconds and tick on the right of
the plus meant nanoseconds.  And I got confused *despite being the
author of that line* (commit b0f2663, "pl031: switch clock base to
rtc_clock", 2012-03-30).


reply via email to

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