Bug 4112 - PPSCLOCK kernel diffs
Summary: PPSCLOCK kernel diffs
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 2.2.2-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: FreeBSD bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 1997-07-18 04:00 UTC by Craig Leres
Modified: 1998-05-04 12:01 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Leres freebsd_committer 1997-07-18 04:00:01 UTC
	Appended are context diffs to the 2.2.2-RELEASE kernel that
	add the PPSCLOCK option. Although the system already has
	the ability to capture DCD timestamps, the existing code
	suffers from several problems. First, the timestamp is not
	taken as soon as possible; measurements show that 2-3
	microseconds are lost on a really fast system like a 200
	MHz pentium pro. 2nd, the lack of a event serial number
	makes it impossible to detect extra or missing pulses.

	In addition to the standard PPSCLOCK functionality provided
	by the original LBL ppsclock package, the new code included
	in this patch gives the user the ability to capture on the
	leading or trailing edge.

	The current layout has a new include file, sys/ppsclock.h.
	It might be a better idea to move the contents of this file
	into sys/ttycom.h and rename the two new ioctls so they
	fit in with the other TIO* ioctls.

Fix: ------ i386/isa/sio.c diffs

RCS file: RCS/sio.c,v
retrieving revision 1.1


------ sys/ppsclock.h

/* @(#) $Header: ppsclock.h,v 1.2 97/07/11 22:29:42 leres Exp $ (LBL) */
/*
 * This software was developed by the Computer Systems Engineering group
 * at Lawrence Berkeley Laboratory under DARPA contract BG 91-66.
 *
 * Copyright (c) 1997 The Regents of the University of California.
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 * 3. All advertising materials mentioning features or use of this software
 *    must display the following acknowledgement:
 *	This product includes software developed by the University of
 *	California, Lawrence Berkeley Laboratory.
 * 4. The name of the University may not be used to endorse or promote
 *    products derived from this software without specific prior
 *    written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */

struct ppsclockev {
	struct timeval tv;
	u_int serial;
};

#if defined(__STDC__)
#define CIOGETEV _IOR('t', 244, struct ppsclockev)	/* get last pps event *
/
#define CIOSETTET _IOW('t', 245, int)			/* trailing edge trig *
/
#else
#define CIOGETEV _IOR(t, 244, struct ppsclockev)	/* get last pps event *
/
#define CIOSETTET _IOW(t, 245, int)			/* trailing edge trig *
/
#endif

------- End of Forwarded Message--yWQYGyn8l72Xmczb26SWQfs6GXlI9n3KbamdwB4QZ2C7bMT7
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

diff -c -r1.1 sio.c
*** /tmp/,RCSt1005653	Thu Jul 17 18:54:38 1997
--- sio.c	Fri Jul 11 22:30:05 1997
***************
*** 64,69 ****
--- 64,72 ----
  #ifdef DEVFS
  #include <sys/devfsext.h>
  #endif
+ #ifdef PPSCLOCK
+ #include <sys/ppsclock.h>
+ #endif
  
  #include <machine/clock.h>
  
***************
*** 245,252 ****
--- 248,262 ----
  
  	bool_t	do_timestamp;
  	bool_t	do_dcd_timestamp;
+ #ifdef PPSCLOCK
+ 	bool_t	do_pps_timestamp;
+ 	bool_t	pps_tet;	/* trailing edge trigger */
+ #endif
  	struct timeval	timestamp;
  	struct timeval	dcd_timestamp;
+ #ifdef PPSCLOCK
+ 	struct ppsclockev ppsclock;
+ #endif
  
  	u_long	bytes_in;	/* statistics */
  	u_long	bytes_out;
***************
*** 1298,1303 ****
--- 1308,1317 ----
  	com->poll_output = FALSE;
  	com->do_timestamp = FALSE;
  	com->do_dcd_timestamp = FALSE;
+ #ifdef PPSCLOCK
+ 	com->do_pps_timestamp = FALSE;
+ 	com->pps_tet = FALSE;
+ #endif
  	outb(iobase + com_cfcr, com->cfcr_image &= ~CFCR_SBREAK);
  	{
  		outb(iobase + com_ier, 0);
***************
*** 1468,1473 ****
--- 1482,1505 ----
  	u_char	recv_data;
  
  	while (TRUE) {
+ #ifdef PPSCLOCK
+ 		/*
+ 		 * Check the modem status now so we get a better
+ 		 * timestamp (On a 200 MHZ Pentium Pro this makes
+ 		 * a difference of about 2 or 3 microseconds...)
+ 		 *
+ 		 * If data carrier detect changed and it was the
+ 		 * one we are waiting for (either leading or trailing
+ 		 * edge) then grab the timestamp.
+ 		 */
+ 		modem_status = inb(com->modem_status_port);
+ 		if (com->do_pps_timestamp &&
+ 		    ((modem_status ^ com->last_modem_status) & MSR_DCD) != 0 &&
+ 		    com->pps_tet ^ ((modem_status & MSR_DCD) != 0)) {
+ 			microtime(&com->ppsclock.tv);
+ 			++com->ppsclock.serial;
+ 		}
+ #endif
  		line_status = inb(com->line_status_port);
  
  #if NCRD > 0
***************
*** 1559,1565 ****
--- 1591,1600 ----
  		}
  
  		/* modem status change? (always check before doing output) */
+ #ifndef PPSCLOCK
+ 		/* If PPSCLOCK, we already did this */
  		modem_status = inb(com->modem_status_port);
+ #endif
  		if (modem_status != com->last_modem_status) {
  			if (com->do_dcd_timestamp
  			    && !(com->last_modem_status & MSR_DCD)
***************
*** 1813,1818 ****
--- 1848,1864 ----
  		com->do_dcd_timestamp = TRUE;
  		*(struct timeval *)data = com->dcd_timestamp;
  		break;
+ #ifdef PPSCLOCK
+ 	case CIOGETEV:
+ 		com->do_pps_timestamp = TRUE;
+ 		*(struct ppsclockev *)data = com->ppsclock;
+ 		break;
+ 
+ 	case CIOSETTET:
+ 		com->pps_tet = *(int *)data;
+ 		break;
+ #endif
+ 
  	default:
  		splx(s);
  		return (ENOTTY);
Comment 1 bde 1997-07-28 09:18:29 UTC
>	Appended are context diffs to the 2.2.2-RELEASE kernel that
>	add the PPSCLOCK option. Although the system already has
>	the ability to capture DCD timestamps, the existing code
>	suffers from several problems. First, the timestamp is not
>	taken as soon as possible; measurements show that 2-3
>	microseconds are lost on a really fast system like a 200
>	MHz pentium pro.

The latency is presumably because serial i/o or just checking for it
takes some time, and DCD is checked after input.  This time doesn't
depend much on the CPU - the (lack of) speed of i/o instructions
depends more on the bus than on the CPU.  I don't think it is
reasonable to attempy to support a latency of 2-3 microseconds in a
general purpose driver or system.  While the "fast interrupt handler"
interface used by the sio driver gives it a chance of reaching such a
latency, there is no chance of reaching it consistently unless there
is only one active device with a fast interrupt handler in the whole
system.  Another active 16550A device (even on the same interrupt)
would give up to about 75 usec of jitter, because it takes up to about
60 i/o instructions to handle one interrupt (more for the COM_MULTIPORT
configuration), and each i/o instruction takes up to about 1.25 usec.
The jitter can be reduced to about 10 usec per extra active 16550A
device by not using the FIFO.

I have some improvements to the interrupt handler that would get
in the way of polling the modem status first.  They depend on the
UART actually working, so that the modem status doesn't need to be
polled unless it has changed.  The UART doesn't tell you that the
modem status register should be read until you have processed what
the UART considers to be higher priority events.

>	2nd, the lack of a event serial number
>	makes it impossible to detect extra or missing pulses.

OK.

>	The current layout has a new include file, sys/ppsclock.h.
>	It might be a better idea to move the contents of this file
>	into sys/ttycom.h and rename the two new ioctls so they
>	fit in with the other TIO* ioctls.

Do you want the whole LBL copyright on the merged version?

>+ #ifdef PPSCLOCK
>+ 		/*
>+ 		 * Check the modem status now so we get a better
>+ 		 * timestamp (On a 200 MHZ Pentium Pro this makes
>+ 		 * a difference of about 2 or 3 microseconds...)
>+ 		 *
>+ 		 * If data carrier detect changed and it was the
>+ 		 * one we are waiting for (either leading or trailing
>+ 		 * edge) then grab the timestamp.
>+ 		 */
>+ 		modem_status = inb(com->modem_status_port);
>+ 		if (com->do_pps_timestamp &&
>+ 		    ((modem_status ^ com->last_modem_status) & MSR_DCD) != 0 &&
>+ 		    com->pps_tet ^ ((modem_status & MSR_DCD) != 0)) {
>+ 			microtime(&com->ppsclock.tv);
>+ 			++com->ppsclock.serial;
>+ 		}
>+ #endif

This inherits a bug from the old code.  I somehow convinced myself that it's
OK to call microtime() from here.  It's not.  microtime() can only be called
from ordinary interrupt handlers.  Calling it from fast interrupt handlers
may fail because of the following problems:

1. If microtime() uses the i8254 counter and not the "i586" TSC, then a
   clock overflow heuristic is used.  The heuristic is fine-tuned to
   work at an i8254 clock interrupt period of 62.5 usec, and won't
   work if clock interrupts have been disabled for more than about
   TIMER0_LATCH_COUNT = 20 i8254 cycles (17 usec).  Fast interrupt
   handlers often break this by keeping all interrupts disabled for
   100 usec or so.

2. `time' is not valid in fast interrupt handlers, since the fast interrupt
   handler may have interrupted hardclock().  Therefore, the accesses to
   `time' in microtime() are invalid if microtime() is called from
   a fast interrupt handler.  Note that the feature of fast interrupt
   handlers that causes this problem - that they can interrupt any normal
   interrupt handler - is the same feature that gives you a chance of
   getting very accurate timestamps.  Also, the complications for the
   kernel PLL and PPSCLOCK in hardclock() make this nontrivial to fix
   - for a simple adjustment to `time' it would only be necessary to
   disable fast interrupts while doing the adjustment and to somehow
   keep track of the before and after states so that microtime() knows
   which one to use.  In fact, the simple adjustment to `time' is
   already handled correctly for the TSC case in cpu_clockupdate().
   Perhaps this case already works, because other adjustments are
   smaller than interrupt jitter.

John Hay tried calling hardpps() from siointr1().  I don't like this,
because hardpps() is probably too slow on slow machines.

Bruce
Comment 2 Craig Leres freebsd_committer 1997-07-28 11:18:16 UTC
Running a time server on a Unix box is basically a hack. To do a good
job, you need a dedicated system that is doing nothing else but time
keeping. But we've found that if you do dedicate a system to doing time
keeping, give it a good time source (like a GPS) and are careful how
your write the kernel code, you can achieve pretty nice results.

> >	Appended are context diffs to the 2.2.2-RELEASE kernel that
> >	add the PPSCLOCK option. Although the system already has
> >	the ability to capture DCD timestamps, the existing code
> >	suffers from several problems. First, the timestamp is not
> >	taken as soon as possible; measurements show that 2-3
> >	microseconds are lost on a really fast system like a 200
> >	MHz pentium pro.
> 
> The latency is presumably because serial i/o or just checking for it
> takes some time, and DCD is checked after input.  This time doesn't
> depend much on the CPU - the (lack of) speed of i/o instructions
> depends more on the bus than on the CPU.  I don't think it is
> reasonable to attempy to support a latency of 2-3 microseconds in a
> general purpose driver or system.  While the "fast interrupt handler"
> interface used by the sio driver gives it a chance of reaching such a
> latency, there is no chance of reaching it consistently unless there
> is only one active device with a fast interrupt handler in the whole
> system.  Another active 16550A device (even on the same interrupt)
> would give up to about 75 usec of jitter, because it takes up to about
> 60 i/o instructions to handle one interrupt (more for the COM_MULTIPORT
> configuration), and each i/o instruction takes up to about 1.25 usec.
> The jitter can be reduced to about 10 usec per extra active 16550A
> device by not using the FIFO.

As far as time keeping goes, it is critical to capture the unix
timestamp as soon after the DCD pulse as possible. The sooner you do it
(and the smaller the jitter due to differing lengths of the code paths)
the better the time keeping code will work. Without designing custom
hardware to grab a timestamp when the pulse occurs, the best you can do
is put the timestamp code as close to the interrupt as possible.

The 2-3 microseconds I measured are due to the "input event" code that
normally gets done before the modem status is checked. I think this is
directly proportional to the cpu clock so it will be worse for slower
systems. (We're basically counting instructions from the time the
interrupt happens until we get the timestamp.)

I think it's reasonable to require people who want to run time servers
to build a kernel with option PPSCLOCK; so it's also reasonable to
restructure the code to read the modem status as soon as possible, but
only when PPSCLOCK is defined. By default, the system is "general
purpose" but if you turn on PPSCLOCK and actually use it, then it is
capable of accurate time recovery.

Don't worry about running a 2nd "fast interrupt" device, if you're
doing accurate time capture with a Unix box, it will be dedicated to
the task and certainly shouldn't have any other serial I/O happening.

> I have some improvements to the interrupt handler that would get
> in the way of polling the modem status first.  They depend on the
> UART actually working, so that the modem status doesn't need to be
> polled unless it has changed.  The UART doesn't tell you that the
> modem status register should be read until you have processed what
> the UART considers to be higher priority events.

Do you object to checking the modem status immediately, if PPSCLOCK is
defined *and* if we're actually doing time recovery? If these
conditions are met, then all we're using the serial port for is time
recovery and we should try to do the best job we can, right?

(I really liked the way the current DCD code latches on the first time
you ask for a timestamp, I wish I had thought of that when I wrote the
orignal PPSCLOCK hack for SunOS.)

> Do you want the whole LBL copyright on the merged version?

It's not really necessary, especially since it's such a tiny piece of
code. What I would really like to see is a future release of FreeBSD
where you could build a kernel with "options PPSCLOCK" and hook up a
GPS and end up with a accurate time server.

It would be nice if the ioctl used the same ppsclockev structure
layout. But it doesn't matter if the struct is defined in a seperate
include or in the standard tty ioctl include. And the ioctl defines
don't need to be exactly the same as we defined them when we originally
wrote this code (for SunOS 4...)

BTW, does the "trailing edge trigger" option seem reasonable?

> >+ #ifdef PPSCLOCK
> >+ 		/*
> >+ 		 * Check the modem status now so we get a better
> >+ 		 * timestamp (On a 200 MHZ Pentium Pro this makes
> >+ 		 * a difference of about 2 or 3 microseconds...)
> >+ 		 *
> >+ 		 * If data carrier detect changed and it was the
> >+ 		 * one we are waiting for (either leading or trailing
> >+ 		 * edge) then grab the timestamp.
> >+ 		 */
> >+ 		modem_status = inb(com->modem_status_port);
> >+ 		if (com->do_pps_timestamp &&
> >+ 		    ((modem_status ^ com->last_modem_status) & MSR_DCD) != 0 &&
> >+ 		    com->pps_tet ^ ((modem_status & MSR_DCD) != 0)) {
> >+ 			microtime(&com->ppsclock.tv);
> >+ 			++com->ppsclock.serial;
> >+ 		}
> >+ #endif
> 
> This inherits a bug from the old code.  I somehow convinced myself that it's
> OK to call microtime() from here.  It's not.  microtime() can only be called
> from ordinary interrupt handlers.  Calling it from fast interrupt handlers
> may fail because of the following problems:

The current code has two other calls to microtime() so I assumed it was ok.

> 1. If microtime() uses the i8254 counter and not the "i586" TSC, then a
>    clock overflow heuristic is used.  The heuristic is fine-tuned to
>    work at an i8254 clock interrupt period of 62.5 usec, and won't
>    work if clock interrupts have been disabled for more than about
>    TIMER0_LATCH_COUNT = 20 i8254 cycles (17 usec).  Fast interrupt
>    handlers often break this by keeping all interrupts disabled for
>    100 usec or so.
> 
> 2. `time' is not valid in fast interrupt handlers, since the fast interrupt
>    handler may have interrupted hardclock().  Therefore, the accesses to
>    `time' in microtime() are invalid if microtime() is called from
>    a fast interrupt handler.  Note that the feature of fast interrupt
>    handlers that causes this problem - that they can interrupt any normal
>    interrupt handler - is the same feature that gives you a chance of
>    getting very accurate timestamps.  Also, the complications for the
>    kernel PLL and PPSCLOCK in hardclock() make this nontrivial to fix
>    - for a simple adjustment to `time' it would only be necessary to
>    disable fast interrupts while doing the adjustment and to somehow
>    keep track of the before and after states so that microtime() knows
>    which one to use.  In fact, the simple adjustment to `time' is
>    already handled correctly for the TSC case in cpu_clockupdate().
>    Perhaps this case already works, because other adjustments are
>    smaller than interrupt jitter.
> 
> John Hay tried calling hardpps() from siointr1().  I don't like this,
> because hardpps() is probably too slow on slow machines.

Wouldn't it be possible to rewrite microtime() to not need to block
interrupts? (This is what we did for the sparc.) In addition, I believe
it would be possible for hardclock() to cooperate with microtime() (and
the kernel PLL) in such a way that it would always give a correct
timestamp. If you're interested in pursuing this type of change, we're
motivated and have several systems we can use for testing.

Thanks for the thoughtful feedback.

		Craig
Comment 3 Poul-Henning Kamp freebsd_committer 1998-05-04 12:00:16 UTC
State Changed
From-To: open->closed

Some work is underway to standardize PPS interfaces, 
see IETF draft draft-mogul-pps-api-01.txt and contact 
me if you're interested in this.