incorrect usage of callout_reset in vbox 4.2.4 ?

[Andriy Gapon]

on 20/12/2012 13:14 Bernhard Fröhlich said the following:
> On Thu, Dec 13, 2012 at 2:08 PM, Andriy Gapon <avg at freebsd.org> wrote:
>>
>> It looks like in timer-r0drv-freebsd.c the code tries to pass absolute time as a
>> timeout parameter to callout_reset while that function actually expects relative
>> time (period).
>>
>> I am testing the following patch, but I am sure that the code can be made more
>> efficient.
>>
>> --- timer-r0drv-freebsd.c.orig  2012-12-12 20:13:27.623202784 +0200
>> +++ timer-r0drv-freebsd.c       2012-12-12 20:19:43.368202795 +0200
>> @@ -172,15 +172,16 @@
>>      /*
>>       * Calc when it should start firing.
>>       */
>> -    u64First += RTTimeNanoTS();
>> +    const uint64_t u64Now = RTTimeNanoTS();
>> +    u64First += u64Now;
>>
>>      pTimer->fSuspended = false;
>>      pTimer->iTick = 0;
>>      pTimer->u64StartTS = u64First;
>>      pTimer->u64NextTS = u64First;
>>
>> -    tv.tv_sec  =  u64First / 1000000000;
>> -    tv.tv_usec = (u64First % 1000000000) / 1000;
>> +    tv.tv_sec  =  (u64First - u64Now) / 1000000000;
>> +    tv.tv_usec = ((u64First - u64Now) % 1000000000) / 1000;
>>      callout_reset(&pTimer->Callout, tvtohz(&tv), rtTimerFreeBSDCallback, pTimer);
>>
>>      return VINF_SUCCESS;
>> @@ -247,8 +248,8 @@
>>          if (pTimer->u64NextTS < u64NanoTS)
>>              pTimer->u64NextTS = u64NanoTS + RTTimerGetSystemGranularity() / 2;
>>
>> -        tv.tv_sec = pTimer->u64NextTS / 1000000000;
>> -        tv.tv_usec = (pTimer->u64NextTS % 1000000000) / 1000;
>> +        tv.tv_sec = (pTimer->u64NextTS - u64NanoTS) / 1000000000;
>> +        tv.tv_usec = ((pTimer->u64NextTS - u64NanoTS) % 1000000000) / 1000;
>>          callout_reset(&pTimer->Callout, tvtohz(&tv), rtTimerFreeBSDCallback, pTimer);
>>      }
> 
> What is your results from that tests? Is the patch correct so should we include
> it into the port?

I believe that the patch is correct.
I haven't seen any regressions nor any visible improvements from it.

-- 
Andriy Gapon