Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 23 Dec 2012 12:33:39 +0200
From:      Andriy Gapon <avg@FreeBSD.org>
To:        =?ISO-8859-1?Q?Bernhard_Fr=F6hlich?= <decke@bluelife.at>
Cc:        freebsd-emulation@FreeBSD.org, vbox@FreeBSD.org
Subject:   Re: incorrect usage of callout_reset in vbox 4.2.4 ?
Message-ID:  <50D6DE03.50707@FreeBSD.org>
In-Reply-To: <CAE-m3X2QzsGvw6ekGdssL1MMBQucRy9ve99M4P1BhJ4=tQBh0w@mail.gmail.com>
References:  <50C9D369.6040204@FreeBSD.org> <CAE-m3X2QzsGvw6ekGdssL1MMBQucRy9ve99M4P1BhJ4=tQBh0w@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
on 20/12/2012 13:14 Bernhard Fröhlich said the following:
> On Thu, Dec 13, 2012 at 2:08 PM, Andriy Gapon <avg@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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50D6DE03.50707>