freebsd 10 kqueue timer regression

[John Baldwin]

On Friday, October 03, 2014 10:16:56 AM Ian Lepore wrote:
> On Fri, 2014-10-03 at 08:50 -0400, John Baldwin wrote:
> > That said, I think one of the reasons the old code worked was that the
> > previous callout had the equivalent of the C_HARDCLOCK flag set.  Thus,
> > when the timer interrupt fires and we rescheuled for N ticks, it was
> > actually N ticks - <time since the timer interrupt that triggered this
> > callout> (if that makes sense?).  Now I think what is happening is that
> > that the
> > callout_reset_sbt() is doing 'delta time t + <time since the timer
> > interrupt>' and that that accounts for your numbers.
> 
> I'm lost with this paragraph.  I don't know what you mean by "old
> code" (pre-eventtimer?  pre-my-changes?).

Pre-eventtimer.  My suggestion here is that because the old code fired at 
hardclock intervals and because the second value (<time since the timer 
interrupt fired (or was scheduled)>) is much smaller than 1/hz, the old code 
was not affected by the phase shift.

> > In that case, I wonder if the reason
> > you are seeing such large gaps is that you have Cx states enabled
> > (including C1E) and that those are making that second factor large enough
> > to account for the skew.  Try setting machdep.idle=spin as a test (this
> > is a better test than ntpd/ptpd I think).
> 
> I'm testing on ARM hardware with no power management enabled other than
> the idle loop executing a wait-for-interrupt instruction that does not
> stop the eventtimer and timecounter clocks, and the latency for coming
> out of that state is on the order of 1uS.

Ok.  Certainly on x86 boxes the latency for coming out of Cx states could be 
much larger (80-100 us IIRC) and that would give a phase shift comparable to 
what you see.

> > The reason I don't like using C_ABSOLUTE is that in theory that will
> > result in longer or shorter intervals if time is adjusted via ntp/ptp,
> > whereas historically EVFILT_TIMER has been CLOCK_UPTIME based instead of
> > CLOCK_REALTIME.
> 
> I don't see anything in the code that implies C_ABSOLUTE uses a
> different clock.  In fact, it's even documented as being absolute time
> since boot.

Ah, I'm used to absolute timeouts being walltime for some reason.

> In callout_reset_sbt_on() if the absolute flag is set the
> sbt you pass in is used as-is.  If the flag is not set, the sbt you pass
> in is added to either the current uptime or the last tc tick uptime.
> 
> For getting a truly periodic timer event, this absolute time technique
> is required.  Adding a delta to something that isn't the time the prior
> was *scheduled* to occur will result in a phase shift in the event
> occurrances over time (phase relative to uptime top-of-second).

I agree, I was only worrying about ntp time stepping somehow skewing this.
However, since C_ABSOLUTE shouldn't be affected by that, I think your
patch is correct.

-- 
John Baldwin