incorrect usage of callout_reset in vbox 4.2.4 ?

[Bernhard Fröhlich]

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?

-- 
Bernhard Fröhlich
http://www.bluelife.at/