Date: Fri, 11 Sep 2009 19:03:17 +0200 From: Luigi Rizzo <rizzo@iet.unipi.it> To: John Baldwin <jhb@freebsd.org> Cc: Juergen Lock <nox@jelal.kn-bremen.de>, Avi Kivity <avi@qumranet.com>, qemu-devel@nongnu.org, freebsd-current@freebsd.org, Jan Kiszka <jan.kiszka@web.de>, Mohammed Gamal <m.gamal005@gmail.com> Subject: Re: FreeBSD timing issues and qemu (was: Re: [Qemu-devel] Re: Breakage with local APIC routing) Message-ID: <20090911170317.GA33232@onelab2.iet.unipi.it> In-Reply-To: <200909111123.00257.jhb@freebsd.org> References: <4A93BF0C.8040601@web.de> <20090910174640.GA30706@triton8.kn-bremen.de> <20090910190800.GA14191@onelab2.iet.unipi.it> <200909111123.00257.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090911170317.GA33232>