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 > > > _at__at_ -323,7 +323,7 _at__at_ 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 luigiReceived on Fri Sep 11 2009 - 14:57:17 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:55 UTC