From owner-freebsd-current@FreeBSD.ORG Fri Sep 11 19:14:47 2009 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0C669106568B for ; Fri, 11 Sep 2009 19:14:47 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id CDA438FC12 for ; Fri, 11 Sep 2009 19:14:46 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 7C32146B2A; Fri, 11 Sep 2009 15:14:46 -0400 (EDT) Received: from jhbbsd.hudson-trading.com (unknown [209.249.190.8]) by bigwig.baldwin.cx (Postfix) with ESMTPA id AE15D8A027; Fri, 11 Sep 2009 15:14:45 -0400 (EDT) From: John Baldwin To: Luigi Rizzo Date: Fri, 11 Sep 2009 13:01:54 -0400 User-Agent: KMail/1.9.7 References: <4A93BF0C.8040601@web.de> <200909111123.00257.jhb@freebsd.org> <20090911170317.GA33232@onelab2.iet.unipi.it> In-Reply-To: <20090911170317.GA33232@onelab2.iet.unipi.it> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200909111301.55692.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Fri, 11 Sep 2009 15:14:45 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Juergen Lock , Avi Kivity , qemu-devel@nongnu.org, freebsd-current@freebsd.org, Jan Kiszka , Mohammed Gamal Subject: Re: FreeBSD timing issues and qemu (was: Re: [Qemu-devel] Re: Breakage with local APIC routing) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Sep 2009 19:14:47 -0000 On Friday 11 September 2009 1:03:17 pm Luigi Rizzo wrote: > On Fri, Sep 11, 2009 at 11:22:59AM -0400, John Baldwin wrote: > > On Thursday 10 September 2009 3:08:00 pm Luigi Rizzo wrote: > ... > > > > Index: sys/kern/kern_timeout.c > > > > @@ -323,7 +323,7 @@ softclock(void *arg) > > > > steps = 0; > > > > cc = (struct callout_cpu *)arg; > > > > CC_LOCK(cc); > > > > - while (cc->cc_softticks != ticks) { > > > > + while (cc->cc_softticks-1 != ticks) { > > > > /* > > > > * cc_softticks may be modified by hard clock, so cache > > > > * it while we work on a given bucket. > > > > > > > > > > as mentioned in the followup message in that thread, > > > you also need this change in callout_tick() > > > > > > mtx_lock_spin_flags(&cc->cc_lock, MTX_QUIET); > > > - for (; (cc->cc_softticks - ticks) < 0; cc->cc_softticks++) { > > > + for (; (cc->cc_softticks - ticks) <= 0; cc->cc_softticks++) { > > > bucket = cc->cc_softticks & callwheelmask; > > > > I would fix the style in the first hunk (spaces around '-') but I think you > > should commit this and get it into 8.0. I think a per-CPU ticks might prove > > very problematic as 'ticks' is rather widely used (though I would find that > > cleaner perhaps). > > i will ask permission to re -- i was hoping to get some feedback > on the thread on -current but no response so far :( > > Note that the per-cpu ticks i was proposing were only visible to the > timing wheels, which don't use absolute timeouts anyways. > So i think the mechanism would be quite safe: right now, when you > request a callout after x ticks, the code first picks a CPU > (with some criteria), then puts the request in the timer wheel for > that CPU using (now) the global 'ticks'. Replacing ticks with cc->cc_ticks, > would completely remove the races in insertion and removal. > > I actually find the per-cpu ticks even less intrusive than this change. Well, it depends. If TCP ever started using per-CPU callouts (i.e. callout_reset_on()) it would probably need to start using the per-CPU ticks instead of the global ticks, etc. You could have 'ticks' just be == to CPU 0's ticks perhaps. -- John Baldwin