From owner-freebsd-emulation@FreeBSD.ORG Sun Dec 23 10:33:43 2012 Return-Path: Delivered-To: freebsd-emulation@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id C5FB6939; Sun, 23 Dec 2012 10:33:43 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id D9F598FC0C; Sun, 23 Dec 2012 10:33:42 +0000 (UTC) Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua [212.40.38.100]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id MAA23619; Sun, 23 Dec 2012 12:33:40 +0200 (EET) (envelope-from avg@FreeBSD.org) Received: from localhost ([127.0.0.1]) by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD)) id 1TmisF-000FQd-Sq; Sun, 23 Dec 2012 12:33:39 +0200 Message-ID: <50D6DE03.50707@FreeBSD.org> Date: Sun, 23 Dec 2012 12:33:39 +0200 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: =?ISO-8859-1?Q?Bernhard_Fr=F6hlich?= Subject: Re: incorrect usage of callout_reset in vbox 4.2.4 ? References: <50C9D369.6040204@FreeBSD.org> In-Reply-To: X-Enigmail-Version: 1.4.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit Cc: freebsd-emulation@FreeBSD.org, vbox@FreeBSD.org X-BeenThere: freebsd-emulation@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Development of Emulators of other operating systems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 23 Dec 2012 10:33:43 -0000 on 20/12/2012 13:14 Bernhard Fröhlich said the following: > On Thu, Dec 13, 2012 at 2:08 PM, Andriy Gapon 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