From owner-freebsd-current@FreeBSD.ORG Fri Sep 11 16:57:17 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 71CF1106568D; Fri, 11 Sep 2009 16:57:17 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 32F018FC0C; Fri, 11 Sep 2009 16:57:16 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id 9C7FD730DA; Fri, 11 Sep 2009 19:03:17 +0200 (CEST) Date: Fri, 11 Sep 2009 19:03:17 +0200 From: Luigi Rizzo To: John Baldwin Message-ID: <20090911170317.GA33232@onelab2.iet.unipi.it> References: <4A93BF0C.8040601@web.de> <20090910174640.GA30706@triton8.kn-bremen.de> <20090910190800.GA14191@onelab2.iet.unipi.it> <200909111123.00257.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200909111123.00257.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i 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 16:57:17 -0000 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. cheers luigi