From owner-freebsd-hackers@FreeBSD.ORG  Fri Oct  3 20:36:13 2014
Return-Path: <owner-freebsd-hackers@FreeBSD.ORG>
Delivered-To: freebsd-hackers@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115])
 (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by hub.freebsd.org (Postfix) with ESMTPS id 97B06D78;
 Fri,  3 Oct 2014 20:36:13 +0000 (UTC)
Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1])
 (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits))
 (Client did not present a certificate)
 by mx1.freebsd.org (Postfix) with ESMTPS id 7058E223;
 Fri,  3 Oct 2014 20:36:13 +0000 (UTC)
Received: from ralph.baldwin.cx (pool-173-70-85-31.nwrknj.fios.verizon.net
 [173.70.85.31])
 by bigwig.baldwin.cx (Postfix) with ESMTPSA id EA3BAB995;
 Fri,  3 Oct 2014 16:36:11 -0400 (EDT)
From: John Baldwin <jhb@freebsd.org>
To: Ian Lepore <ian@freebsd.org>
Subject: Re: freebsd 10 kqueue timer regression
Date: Fri, 03 Oct 2014 16:35:39 -0400
Message-ID: <3432803.0HkSRzvHj6@ralph.baldwin.cx>
User-Agent: KMail/4.12.5 (FreeBSD/10.1-BETA2; KDE/4.12.5; amd64; ; )
In-Reply-To: <1412353016.12052.82.camel@revolution.hippie.lan>
References: <8ABC0977-FB8F-45E7-ACCC-BFA92EE22E1C@glccom.com>
 <2499075.KMdpQjyIZI@ralph.baldwin.cx>
 <1412353016.12052.82.camel@revolution.hippie.lan>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7
 (bigwig.baldwin.cx); Fri, 03 Oct 2014 16:36:12 -0400 (EDT)
Cc: freebsd-hackers@freebsd.org, Adrian Chadd <adrian@freebsd.org>,
 Paul Albrecht <palbrecht@glccom.com>
X-BeenThere: freebsd-hackers@freebsd.org
X-Mailman-Version: 2.1.18-1
Precedence: list
List-Id: Technical Discussions relating to FreeBSD
 <freebsd-hackers.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-hackers>, 
 <mailto:freebsd-hackers-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-hackers/>
List-Post: <mailto:freebsd-hackers@freebsd.org>
List-Help: <mailto:freebsd-hackers-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>,
 <mailto:freebsd-hackers-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 03 Oct 2014 20:36:13 -0000

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