188

Unix/Linux 网络时间协议版本3 Network Time Protocol Version 3 (NTP) distribution for Unix systems

Replied: Wed, 25 Mar 1998 16:26:37 -0500
Replied: "Kamal A Mostafa <[e-mail]> "
Received: from mail.eecis.udel.edu by whimsy.udel.edu id aa24025;
          25 Mar 1998 14:59 EST
Received: from green.images.com (green.images.com [198.207.178.4]) by orange.images.com (8.8.5/SCO5) with ESMTP id LAA08422 for <[e-mail]>; Wed, 25 Mar 1998 11:59:00 -0800 (PST)
Received: (from kamal@localhost) by green.images.com (8.8.5/SCO5) id LAA02740; Wed, 25 Mar 1998 11:58:58 -0800 (PST)
Message-ID: <19980325115858.63248@green>
Date: Wed, 25 Mar 1998 11:58:58 -0800
From: Kamal A Mostafa <[e-mail]>
To: [e-mail]
Cc: Kamal A Mostafa <[e-mail]>
Subject: TIMER_ENQUEUE functions + sanity checks
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="3/rZRmxL6MmkK24k"
X-Mailer: Mutt 0.89i


--3/rZRmxL6MmkK24k
Content-Type: text/plain; charset=us-ascii

-----BEGIN PGP SIGNED MESSAGE-----

Hiya...

Okay, I went ahead and reimplemented the TIMER_ENQUEUE family of macros as
functions, and added some sanity-checking stuff (patch to 5.92d attached,
modifies include/ntp.h and xntpd/ntp_timer.c)

For now, *all* of my changes are wrapped in #ifdef TIMERQUEUE_DEBUG.  I
suspect that my new code obviates the need for the pre-existing
TIMERQUEUE_DEBUG stuff in ntp_timer.c, but I left it in anyway, pending your
review.  I'm pretty sure that my code would pick up any timerqueue insanity
before the older TIMERQUEUE_DEBUG code would though.  I did take the liberty
of adding abort() calls to that older code -- if the timerqueue is hosed, it
doesn't make sense to just plow ahead anyway, right?

I've been running a handful of systems with these changes for two days, with
no problems whatsoever (timekeeping seems unaffected; no messages from the
sanity-check code, nor any coredumps).

Assuming that (a) you like my new sanity-check stuff, and (b) Dave Mills
says it's okay to use functions instead of macros here, I suggest that we:

1. Remove the #ifdef TIMERQUEUE_DEBUG wrappings from my new code to just let
   the new functions replace the macros permanently -- I still don't quite
   see the need for them being macros anwyay.

2. Re-wrap just the sanity-check parts of the new functions in
   #ifdef TIMERQUEUE_DEBUG.

3. Leave the old #ifdef TIMERQUEUE_DEBUG stuff as it was (or remove it,
   if you find my new sanity-check code to be a sufficient replacement).

4. Re-code a couple of pre-existing sanity checks (e.g. "tq was NIL!")
   to use my new sanity-check macros for consistency.

What do you think?

 -k


-----BEGIN PGP SIGNATURE-----
Version: 2.6.2

iQCVAwUBNRlh54rzk78EeS5RAQEkJwP/VBAugm6eBh89yL66KPeWaVboK8F2/Lz2
CFuK5UVhSqiXe6G3b513mDjrNWqCtd8Emq6gddc+XSm367PU7IuDnfwUKLwd5wgh
SCzWRhoW4LhJlXxH6BjhzUn8u7x6lVAXcioSJUR0L3/0xSTwjJDIQk9qtjdOgNop
qOtRpNW0PzA=
=Q1rk
-----END PGP SIGNATURE-----

--3/rZRmxL6MmkK24k
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="TQDEBUG.patch"

*** include/ntp.h.orig Wed Mar 18 23:04:58 1998
--- include/ntp.h Mon Mar 23 19:35:04 1998
***************
*** 104,109 ****
  #define	TIMER_NSLOTS	(1<<(NTP_MAXPOLL-(NTP_MINPOLL-1)))
  #define	TIMER_SLOT(t)	(((t) >> (NTP_MINPOLL-1)) & (TIMER_NSLOTS-1))
  
  #ifndef SYS_WINNT
  /*
   * TIMER_ENQUEUE() puts stuff on the timer queue.  It takes as
--- 104,118 ----
  #define	TIMER_NSLOTS	(1<<(NTP_MAXPOLL-(NTP_MINPOLL-1)))
  #define	TIMER_SLOT(t)	(((t) >> (NTP_MINPOLL-1)) & (TIMER_NSLOTS-1))
  
+ #ifdef TIMERQUEUE_DEBUG
+ 
+ /* use the non-macro versions of these routines from xntpd/ntp_timer.c */
+ void	TIMER_ENQUEUE(struct event *ea, struct event *iev);
+ void	TIMER_INSERT(struct event *ea, struct event *iev);
+ void	TIMER_DEQUEUE(struct event *ev);
+ 
+ #else /* TIMERQUEUE_DEBUG */
+ 
  #ifndef SYS_WINNT
  /*
   * TIMER_ENQUEUE() puts stuff on the timer queue.  It takes as
***************
*** 211,216 ****
   	}
  
  #endif /* SYS_WINNT */  
  
  /*
   * The interface structure is used to hold the addresses and socket
--- 220,227 ----
   	}
  
  #endif /* SYS_WINNT */  
+ 
+ #endif /* TIMERQUEUE_DEBUG */  
  
  /*
   * The interface structure is used to hold the addresses and socket
*** xntpd/ntp_timer.c.orig Mon Mar 23 19:11:34 1998
--- xntpd/ntp_timer.c Tue Mar 24 00:35:49 1998
***************
*** 1,5 ****
  /*
!  * ntp_event.c - event timer support routines
   */
  #ifdef HAVE_CONFIG_H
  #include <config.h>
--- 1,5 ----
  /*
!  * ntp_timer.c - event timer support routines
   */
  #ifdef HAVE_CONFIG_H
  #include <config.h>
***************
*** 226,231 ****
  }
  
  
  /*
   * timer - dispatch anyone who needs to be
   */
--- 226,261 ----
  }
  
  
+ #ifdef TIMERQUEUE_DEBUG
+ /* Timer queue sanity checking routines */
+ 
+ static void EV_ASSERT(struct event *ev, char *m)
+ {
+     if ( ! ev )
+ 	{ msyslog(LOG_ERR, "%s is NULL, aborting!",m); abort(); }
+ }
+ 
+ static void EV_LINKCHK(struct event *ev, char *m)
+ {
+     if ( ! ev )
+ 	{ msyslog(LOG_ERR, "%s is NULL, aborting!",m); abort(); }
+     if ( ! ev->next )
+ 	{ msyslog(LOG_ERR, "%s->next is NULL, aborting!",m); abort(); }
+     if ( ! ev->prev )
+ 	{ msyslog(LOG_ERR, "%s->prev is NULL, aborting!",m); abort(); }
+     if ( ev->next->prev != ev )
+ 	{ msyslog(LOG_ERR, "%s->next->prev != self, aborting!",m); abort(); }
+     if ( ev->prev->next != ev )
+ 	{ msyslog(LOG_ERR, "%s->prev->next != self, aborting!",m); abort(); }
+ }
+ 
+ #else /* TIMERQUEUE_DEBUG */
+ 
+ # define EV_ASSERT()	{}
+ # define EV_LINKCHK()	{}
+ 
+ #endif /* TIMERQUEUE_DEBUG */
+ 
  /*
   * timer - dispatch anyone who needs to be
   */
***************
*** 280,286 ****
  	struct event *qh;
  
  	qh = ev = &timerqueue[i];
! 	if (qh->event_time != 0)
  	  msyslog(LOG_ERR, "timerqueue[%d].event_time is %d instead of 0!",
  		  i, timerqueue[i].event_time);
  	j = 0;
--- 310,316 ----
  	struct event *qh;
  
  	qh = ev = &timerqueue[i];
! 	if (qh->event_time != 0) {
  	  msyslog(LOG_ERR, "timerqueue[%d].event_time is %d instead of 0!",
  		  i, timerqueue[i].event_time);
  	  abort();
***************
*** 283,288 ****
  	if (qh->event_time != 0)
  	  msyslog(LOG_ERR, "timerqueue[%d].event_time is %d instead of 0!",
  		  i, timerqueue[i].event_time);
  	j = 0;
  	do
  	  {
--- 313,320 ----
  	if (qh->event_time != 0) {
  	  msyslog(LOG_ERR, "timerqueue[%d].event_time is %d instead of 0!",
  		  i, timerqueue[i].event_time);
+ 	  abort();
+ 	}
  	j = 0;
  	do
  	  {
***************
*** 286,292 ****
  	j = 0;
  	do
  	  {
! 	    if (ev->prev->next != ev)
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->prev->next != ev",
  		      i, j);
  	    if (ev->next->prev != ev)
--- 318,324 ----
  	j = 0;
  	do
  	  {
! 	    if (ev->prev->next != ev) {
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->prev->next != ev",
  		      i, j);
  	      abort();
***************
*** 289,295 ****
  	    if (ev->prev->next != ev)
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->prev->next != ev",
  		      i, j);
! 	    if (ev->next->prev != ev)
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->next->prev != ev",
  		      i, j);
  	    ++j;
--- 321,329 ----
  	    if (ev->prev->next != ev) {
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->prev->next != ev",
  		      i, j);
! 	      abort();
! 	    }
! 	    if (ev->next->prev != ev) {
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->next->prev != ev",
  		      i, j);
  	      abort();
***************
*** 292,297 ****
  	    if (ev->next->prev != ev)
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->next->prev != ev",
  		      i, j);
  	    ++j;
  	    ev = ev->next;
  	  }
--- 326,333 ----
  	    if (ev->next->prev != ev) {
  	      msyslog(LOG_ERR, "timerqueue[%d]: #%d: ev->next->prev != ev",
  		      i, j);
+ 	      abort();
+ 	    }
  	    ++j;
  	    ev = ev->next;
  	  }
***************
*** 313,319 ****
        ev->event_handler(ev->peer);
        ev = tq->next;
      }
!     if (!ev)
        msyslog(LOG_ERR, "timer: ev was NIL!");
    } else {
      msyslog(LOG_ERR, "timer: tq was NIL!");
--- 349,355 ----
        ev->event_handler(ev->peer);
        ev = tq->next;
      }
!     if (!ev) {
        msyslog(LOG_ERR, "timer: ev was NIL!");
        abort();
      }
***************
*** 315,320 ****
      }
      if (!ev)
        msyslog(LOG_ERR, "timer: ev was NIL!");
    } else {
      msyslog(LOG_ERR, "timer: tq was NIL!");
    }
--- 351,358 ----
      }
      if (!ev) {
        msyslog(LOG_ERR, "timer: ev was NIL!");
+       abort();
+     }
    } else {
      msyslog(LOG_ERR, "timer: tq was NIL!");
      abort();
***************
*** 317,322 ****
        msyslog(LOG_ERR, "timer: ev was NIL!");
    } else {
      msyslog(LOG_ERR, "timer: tq was NIL!");
    }
  
    /* Added mutex to prevent race condition among threads under Windows NT */
--- 355,361 ----
      }
    } else {
      msyslog(LOG_ERR, "timer: tq was NIL!");
+     abort();
    }
  
    /* Added mutex to prevent race condition among threads under Windows NT */
***************
*** 432,434 ****
    timer_xmtcalls = 0;
    timer_timereset = current_time;
  }
--- 471,555 ----
    timer_xmtcalls = 0;
    timer_timereset = current_time;
  }
+ 
+ 
+ #ifdef TIMERQUEUE_DEBUG
+ /* macro versions of these routines are available in include/ntp.h */
+ 
+ # ifdef SYS_WINNT  /* WindowsNT apparently needs mutex locking around here */
+ #  define WINNT_WAIT() 	WaitForSingleObject(m_hListMutex,INFINITE)
+ #  define WINNT_RELS()	ReleaseMutex(m_hListMutex)
+ # else
+ #  define WINNT_WAIT()	{}
+ #  define WINNT_RELS()	{}
+ # endif
+ 
+ /*
+  * TIMER_ENQUEUE() puts stuff on the timer queue.  It takes as
+  * arguments (ea), an array of event slots, and (iev), the event
+  * to be inserted.  This one searches the hash bucket from the
+  * end, and is about optimum for the timing requirements of
+  * NTP peers.
+  */
+ void TIMER_ENQUEUE(struct event *ea, struct event *iev)
+ {
+ 	register struct event *ev;
+ 
+ 	EV_LINKCHK( ea, "TIMER_ENQUEUE(): ea" );
+ 	EV_ASSERT( iev, "TIMER_ENQUEUE(): iev" );
+ 	WINNT_WAIT();
+ 	ev = (ea)[TIMER_SLOT((iev)->event_time)].prev;
+ 	EV_LINKCHK( ev, "TIMER_ENQUEUE(): ev" );
+ 	while (ev->event_time > (iev)->event_time) {
+ 		ev = ev->prev;
+ 		EV_LINKCHK( ev, "TIMER_ENQUEUE(): ev" );
+ 	}
+ 	(iev)->prev = ev;
+ 	(iev)->next = ev->next;
+ 	(ev)->next->prev = (iev);
+ 	(ev)->next = (iev);
+ 	WINNT_RELS();
+ }
+ 
+ /*
+  * TIMER_INSERT() also puts stuff on the timer queue, but searches the
+  * bucket from the top.  This is better for things that do very short
+  * time outs, like clock support.
+  */
+ void TIMER_INSERT(struct event *ea, struct event *iev)
+ {
+ 	register struct event *ev;
+ 
+ 	EV_LINKCHK( ea, "TIMER_INSERT(): ea" );
+ 	EV_ASSERT( iev, "TIMER_INSERT(): iev" );
+ 	WINNT_WAIT();
+ 	ev = (ea)[TIMER_SLOT((iev)->event_time)].next;
+ 	EV_LINKCHK( ev, "TIMER_INSERT(): ev" );
+ 	while (ev->event_time != 0 &&
+ 	    ev->event_time < (iev)->event_time) {
+ 		ev = ev->next;
+ 		EV_LINKCHK( ev, "TIMER_INSERT(): ev" );
+ 	}
+ 	(iev)->next = ev;
+ 	(iev)->prev = ev->prev;
+ 	(ev)->prev->next = (iev);
+ 	(ev)->prev = (iev);
+ 	WINNT_RELS();
+ }
+ 
+ /*
+  * Remove an event from the queue.
+  */
+ void TIMER_DEQUEUE(struct event *ev)
+ {
+ 	EV_LINKCHK( ev, "TIMER_DEQUEUE(): ev" );
+ 	WINNT_WAIT();
+ 	if ((ev)->next != 0) {
+ 		(ev)->next->prev = (ev)->prev;
+ 		(ev)->prev->next = (ev)->next;
+ 		(ev)->next = (ev)->prev = 0;
+ 	}
+ 	WINNT_RELS();
+ }
+ 
+ #endif /* TIMERQUEUE_DEBUG */

--3/rZRmxL6MmkK24k--