[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

unblock adjtimex (testing *and* unstable)



I maintain the adjtimex package, and am also the upstream developer.

I don't understand the status of the package.  I uploaded 1.25-1 on Aug 10:
  
  adjtimex_1.25-1_i386.changes uploaded successfully to localhost
  along with the files:
    adjtimex_1.25-1.dsc
    adjtimex_1.25.orig.tar.gz
    adjtimex_1.25-1.diff.gz
    adjtimex_1.25-1_i386.deb

However, the QA site lists 1.24-1 as the version in unstable, with no
version in testing.  1.24 works with most machines, but fails for
certain new hardware (freezing during installation).

http://release.debian.org/migration/testing.pl?package=adjtimex says:

  * adjtimex has no old version in testing (trying to add, not update)
  * Ignoring high urgency setting for NEW package
  * Ignoring request to block package by freeze, due to unblock request by freeze-exception
  * adjtimex (source, i386, alpha, amd64, arm, armel, hppa, ia64, mips, mipsel, powerpc, s390, sparc) has new bugs! 

I assume the bug report against 1.24 is preventing it from propagating
to testing, in spite of the unblock request.  I don't know why 1.25
isn't entering unstable though.

1.25 closes three bugs, and 1.26 (just uploaded) incorporates a fix
supplied by Alain Guibert for another problem he had noticed (though
there was nothing in the BTS yet).  Detailed descriptions of the
changes are appended, in the form of a commented diff between 1.24 and
1.25, and the patches from Alain Guilbert.

I would like to see 1.26 released with Lenny.  I planned to submit an
unblock request after it had spent ten days in unstable.  However,
that won't work if new versions can't even enter unstable.  Please let
me know if there's something I should do about that.  (Upload with
higher urgency?)

         - Jim Van Zandt


----------------------  changes from 1.24 to 1.25 ----------------


The significant changes are upstream, to fix important bugs 477637 and
474337 (freeze during installation because the read of /dev/rtc
blocks).  Here is the Debian changelog entry:


adjtimex (1.25-1) unstable; urgency=low

  * New upstream release.  Set an alarm to avoid blocking even if /dev/rtc
    never becomes readable, then fall back to busywait. New --nointerrupt
    option to force busywait without first waiting for the interrupt
    timeout (thanks to Alain Guibert). (closes:Bug#477637,Bug#474337).
    Update man page (closes:Bug#435956).

 -- James R. Van Zandt <jrv@debian.org>  Fri, 08 Aug 2008 19:43:38 -0400


Contents of this diff:
  1) adjtimex.c changes
  2) documentation
  3) other stuff


===================   1) adjtimex.c changes

Set an alarm to prevent blocking.  If interrupts don't come within two
seconds, fall back to busywait access.

Implement new option "--nointerrupt" to use the busywait access method
immediately.

Before, adjtimex would try /dev/rtc and /dev/rtc0.  Now add three more
options as in hwclock: /dev/efirtc for ia64, /dev/misc/rtc for devfs,
and /dev/misc/efirtc for devfs on ia64.


diff -ur adjtimex-1.24/adjtimex.c adjtimex-1.25/adjtimex.c
--- adjtimex-1.24/adjtimex.c	2008-04-04 21:35:55.000000000 -0400
+++ adjtimex-1.25/adjtimex.c	2008-08-07 21:46:48.000000000 -0400
@@ -31,6 +31,7 @@
 #include <sys/types.h>
 #include <syscall.h>
 #include <time.h>
+#include <signal.h>
 #include <unistd.h>
 #include <utmp.h>
 #include <fcntl.h>
@@ -65,6 +66,8 @@
 				 1 = using /dev/rtc
 				-1 = will use /dev/rtc if available,
 				     but have not tried to open it yet */
+static int nointerrupt = 0;	/* 0 = try to detect RTC tick via interrupt
+				   1 = via another method */
 
 struct hack {
   double ref;			/* reference time for time hack */
@@ -122,6 +125,7 @@
   {"tick", 1, NULL, 't'},
   {"utc", 0, NULL, 'u'},
   {"directisa", 0, NULL, 'd'},
+  {"nointerrupt", 0, NULL, 'n'},
   {"version", 0, NULL, 'v'},
   {"verbose", 0, NULL, 'V'},
   {"watch", 0, NULL, 'w'},
@@ -209,6 +213,7 @@
 "       -r, --review[=file]       review clock log file, estimate drifts\n"
 "       -u, --utc                 the CMOS clock is set to UTC\n"
 "       -d, --directisa           access the CMOS clock directly\n"
+"       -n, --nointerrupt         bypass the CMOS clock interrupt access method\n"
 "\n"
 "Informative Output:\n"
 "           --help                print this help, then exit\n"
@@ -376,6 +381,9 @@
 	  case 'd':
 	    using_dev_rtc = 0;
 	    break;
+	  case 'n':
+	    nointerrupt = 1;
+	    break;
 	  case 'v':
 	    {
 	      printf("adjtimex %s\n", VERSION);
@@ -530,39 +538,54 @@
 /*
  * Main initialisation of CMOS clock access methods, for all modes.
  * Set the global variable cmos_device to the first available RTC device
- * driver filename between /dev/rtc and /dev/rtc0, and set cmos_fd to
+ * driver filename (/dev/rtc, /dev/rtc0, etc.), and set cmos_fd to
  * a file descriptor for it.
  * Failing that, select and initialize direct I/O ports mode.
  */
 static
 void cmos_init ()
 {
+/*
+ following explanation taken from hwclock sources:
+ /dev/rtc is conventionally chardev 10/135
+ ia64 uses /dev/efirtc, chardev 10/136
+ devfs (obsolete) used /dev/misc/... for miscdev
+ new RTC framework + udev uses dynamic major and /dev/rtc0.../dev/rtcN
+*/
+  char *fls[] = {
+#ifdef __ia64__
+    "/dev/efirtc",
+    "/dev/misc/efirtc",
+#endif
+    "/dev/rtc",
+    "/dev/rtc0",
+    "/dev/misc/rtc",
+    NULL
+  };
+  char **p=fls;
+
   if (using_dev_rtc < 0)
     {
-      cmos_device = "/dev/rtc";
-      cmos_fd = open (cmos_device, O_RDONLY);
-      if (cmos_fd >= 0)
+      while ((cmos_device=*p++))
 	{
+	  cmos_fd = open (cmos_device, O_RDONLY);
+	  if (cmos_fd >= 0)
+	    {
+	      if(verbose)
+		fprintf (stdout, "opened %s for reading\n", cmos_device);
+	      using_dev_rtc = 1;
+	      return;
+	    }
 	  if(verbose)
-	    fprintf (stdout, "opened %s for reading\n", cmos_device);
-	  using_dev_rtc = 1;
-	  return;
+	    {
+	      int saveerr=errno;
+	      fprintf (stdout, "adjtimex: cannot open %s for reading\n", 
+		       cmos_device);
+	      errno = saveerr;
+	      perror("adjtimex");
+	    }
 	}
-      if(verbose)
-	fprintf (stdout, "cannot open %s for reading\n", cmos_device);
 
-      /* falback on /dev/rtc0 */
-      cmos_device = "/dev/rtc0";
-      cmos_fd = open (cmos_device, O_RDONLY);
-      if (cmos_fd >= 0)
-	{
-	  if(verbose)
-	    fprintf (stdout, "opened %s for reading\n", cmos_device);
-	  using_dev_rtc = 1;
-	  return;
-	}
-      if(verbose)
-	fprintf (stdout, "cannot open %s for reading\n", cmos_device);
       using_dev_rtc = 0;
     }
   else if (using_dev_rtc > 0)
@@ -626,6 +649,41 @@
   return (b & 15) + (b >> 4) * 10;
 }
 
+static int timeout;	/* An alarm signal has occurred */
+
+static void
+alarm_handler (int const dummy) {
+  timeout = 1;
+}
+
+/*
+ * starts (or cancels) a 2 second timeout period
+ * the global boolean timeout indicates that the timeout has been reached
+ */
+static void
+alarm_timeout (int onoff) {
+  static struct sigaction oldaction;
+
+  if (onoff)
+    {
+      struct sigaction action;
+
+      action.sa_handler = &alarm_handler;
+      sigemptyset(&action.sa_mask);
+      action.sa_flags = 0;
+      sigaction(SIGALRM, &action, &oldaction);	/* Install our signal handler */
+
+      timeout = 0;	/* reset global timeout flag */
+      alarm(2);		/* generate SIGALARM 2 seconds from now */
+    }
+  else
+    {
+      alarm(0);					/* cancel alarm */
+      sigaction(SIGALRM, &oldaction, NULL);	/* remove our signal handler */
+    }
+}
+
+
 /* return CMOS time in CMOS_TIMEP and sytem time in SYSP.  cmos_init()
    must have been called before this function. */
 static void
@@ -638,60 +696,110 @@
   time_t cmos_time;
   struct timeval now;
   int noint_fallback = 1;	/* detect tick by 0 => uip, 1 => time change */
+  int got_tick = 0;
+  int got_time = 0;
+  int saveerr;
 
   if (using_dev_rtc > 0)	/* access the CMOS clock thru /dev/rtc */
     {
-      ioctl (cmos_fd, RTC_PIE_OFF, NULL); /* disable periodic interrupts */
-      rc = ioctl (cmos_fd, RTC_UIE_ON, NULL); /* enable update
-					       complete interrupts */
-      if (rc == -1)	/* no interrupts? fallback to busywait */
-	{
-	  if (verbose)
-	    fprintf(stdout, 
-		    "%s doesn't allow user access to update interrupts\n"
-		    " - using busy wait instead\n", cmos_device);
+      if (!nointerrupt)		/* get RTC tick via update interrupt */
+	{
+	  ioctl (cmos_fd, RTC_PIE_OFF, NULL); /* disable periodic interrupts */
+	  rc = ioctl (cmos_fd, RTC_UIE_ON, NULL); /* enable update
+						     complete interrupts */
+	  if (rc == -1)	/* no interrupts? fallback to busywait */
+	    {
+	      if (verbose)
+		fprintf(stdout, 
+			"%s doesn't allow user access to update interrupts\n"
+			" - using busy wait instead\n", cmos_device);
+	      nointerrupt = 1;
+	    }
+ 	  else		/* wait for update-ended interrupt */
+	    {
+	      unsigned long interrupt_info;
+	      int type_uie, count;
+
+	      if (verbose)
+		fprintf (stdout, "waiting for CMOS update-ended interrupt\n");
+
+	      alarm_timeout(1);	/* arm a 2 seconds timeout */
+
+	      do {
+		rc = read(cmos_fd, &interrupt_info, sizeof(interrupt_info));
+		saveerr = errno;
+		gettimeofday(&now, NULL);
+
+		if (rc == -1)
+		  {
+		    if (saveerr == EINTR && timeout) /* timeout waiting for interrupt? fallback to busywait */
+		      {
+			if (verbose)
+			  fprintf(stdout,
+				  "timeout waiting for a CMOS update interrupt from %s\n"
+				  " - using busy wait instead\n", cmos_device);
+			nointerrupt = 1;
+		      }
+		    else	/* read(/dev/rtc) failed for another reason (not a timeout) */
+		      {
+			char message[128];
+			snprintf(message, sizeof(message),
+				 "read() from %s to wait for clock tick failed", cmos_device);
+			perror(message);
+			exit(1);
+		      }
+		  }
 
+		type_uie = (int)(interrupt_info & 0x80);
+		count = (int)(interrupt_info >> 8);
+	      } while (((type_uie == 0) || (count > 1)) && !nointerrupt);
+	      /* The low-order byte holds
+		 the interrupt type.  The first read may succeed
+		 immediately, but in that case the byte is zero, so we
+		 know to try again. If there has been more than one
+		 interrupt, then presumably periodic interrupts were
+		 enabled.  We need to try again for just the update
+		 interrupt.  */
+
+	      if ((type_uie) && (count == 1))
+		got_tick = 1;
+
+	      alarm_timeout(0);		/* disable timeout */
+
+	    }	    /* end of successfull RTC_UIE_ON case */
+	}	/* end of interrupt tryouts */
+
+
+      /* At this stage, we either just detected the clock tick via an
+	 update interrupt, or detected nothing yet (interrupts were
+	 bypassed, unavailable, or timeouted). Fallback to busywaiting
+	 for the tick. */
+
+      if (!got_tick)
+	{
 	  if (noint_fallback)
-	    busywait_second_change(&tm, &now);
+	    {
+	      busywait_second_change(&tm, &now);
+	      got_time = 1;
+	    }
 	  else
 	    busywait_uip_fall(&now);
+	  got_tick = 1;
 	}
-      else		/* wait for update-ended interrupt */
-	{
-	  unsigned long interrupt_info;
-	  int type, count;
 
-	  if (verbose)
-	    fprintf (stdout, "waiting for CMOS update-ended interrupt\n");
 
-	  do {
-	    rc = read(cmos_fd, &interrupt_info, sizeof(interrupt_info));
-	    gettimeofday(&now, NULL);
+      /* At this stage, we just detected the clock tick, by any method.
+	 Now get this just beginning RTC second, unless we already have it */
 
-	    if (rc == -1)
-	      {
-		char message[128];
-		snprintf(message, sizeof(message),
-			"read() from %s to wait for clock tick failed", cmos_device);
-		perror(message);
-		exit(1);
-	      }
-	    type = (int)(interrupt_info & 0xff);
-	    count = (int)(interrupt_info >> 8);
-	  } while ((type==0)||(count>1));	/* The low-order byte holds
-		the interrupt type.  The first read may succeed
-		immediately, but in that case the byte is zero, so we
-		know to try again. If there has been more than one
-		interrupt, then presumably periodic interrupts were
-		enabled.  We need to try again for just the update
-		interrupt.  */
-
-	} /* the CMOS clock tick just happened, and has been timestamped */
-
-      /* now get this just beginning RTC second */
-      ioctl (cmos_fd, RTC_RD_TIME, &tm);
-      ioctl (cmos_fd, RTC_UIE_OFF, NULL); /* disable update complete
-					     interrupts */
+      if (!got_time)
+	ioctl (cmos_fd, RTC_RD_TIME, &tm);
+
+
+      /* disable update complete interrupts
+	 It could seem more natural to do this above, just after we
+	 actually got the interrupt. But better do it here at the end,
+	 after all time-critical operations including the RTC_RD_TIME. */
+      ioctl (cmos_fd, RTC_UIE_OFF, NULL);
     }
   else		/* access the CMOS clock thru I/O ports */
     {
@@ -706,10 +814,10 @@
 	  tm.tm_wday = cmos_read_bcd(6)-1;/* RTC uses 1 - 7 for day of the week, 1=Sunday */
 	  tm.tm_mday = cmos_read_bcd (7);
 	  tm.tm_mon = cmos_read_bcd(8)-1; /* RTC uses 1 base */
-	  /* we assume we're not running on a PS/2, where century is in byte 55 */
+	  /* we assume we're not running on a PS/2, where century is
+	     in byte 55 */
 	  tm.tm_year = cmos_read_bcd(9)+100*cmos_read_bcd(50)-1900;
-	}
-      while (tm.tm_sec != cmos_read_bcd (0));
+	} while (tm.tm_sec != cmos_read_bcd (0));
     }
   tm.tm_isdst = -1;		/* don't know whether it's summer time */
 

===================   2) documentation
changelog entries.

Manpage: Correct macro spelling from ".pp" to ".PP" to silence a
lintian error.  Document new option --nointerrupt.  Confirm the
adjtimex status values are still correct for 2.6 kernels.  Mention
devices other than /dev/rtc.


diff -ur adjtimex-1.24/ChangeLog adjtimex-1.25/ChangeLog
--- adjtimex-1.24/ChangeLog	2008-04-04 22:03:52.000000000 -0400
+++ adjtimex-1.25/ChangeLog	2008-08-07 21:42:34.000000000 -0400
@@ -1,3 +1,23 @@
+2008-08-07  James R. Van Zandt  <jrv@vanzandt.comcast.net>
+
+	* Makefile.in (VERSION): version 1.25
+
+	* adjtimex.8: document new --nointerrupt option
+
+2008-08-07  James R. Van Zandt  <jrvz@comcast.net>
+
+	* adjtimex.8: Kernels through 2.6 are still using the same
+	adjtimex status values.  Mention new /dev file names.
+
+	* adjtimex.c (cmos_init): try alternate ia64 and devfs device
+	names (courtesy of hwclock sources).
+
+	* adjtimex.c (cmos_read_time): Set an alarm to avoid blocking even
+	if /dev/rtc never becomes readable, then fall back to
+	busywait_second_change(). New --nointerrupt option to force
+	busywait without waiting for the interrupt timeout (thanks to
+	Alain Guibert).
+
 2008-04-04  James R. Van Zandt  <jrv@vanzandt.comcast.net>
 
 	* Makefile (VERSION): version 1.24

diff -ur adjtimex-1.24/adjtimex.8 adjtimex-1.25/adjtimex.8
--- adjtimex-1.24/adjtimex.8	2007-10-05 20:14:33.000000000 -0400
+++ adjtimex-1.25/adjtimex.8	2008-08-08 19:52:03.000000000 -0400
@@ -1,5 +1,5 @@
 .\"{{{  Title                      Emacs major mode should be: -*- nroff -*-
-.TH ADJTIMEX 8 "May 23, 2006"
+.TH ADJTIMEX 8 "August 7, 2008"
 .\"}}}
 .\"{{{  Name
 .SH NAME
@@ -68,7 +68,7 @@
 .SH OPTIONS
 Options may be introduced by either \fB\-\fP or \fB\-\-\fP, and unique
 abbreviations may be used.
-.pp
+.PP
 Here is a summary of the options, grouped by type.  Explanations
 follow.
 .hy 0
@@ -117,6 +117,8 @@
 \-\-utc
 \-d
 \-\-directisa
+\-n
+\-\-nointerrupt
 .TP
 \fBInformative Output\fP
 \-\-help
@@ -141,7 +143,7 @@
       5   clock not externally synchronized (so the 
           kernel should leave the CMOS clock alone)
 .fi
-For Linux 2.0 kernels, the value is a sum of these:
+For Linux kernels 2.0 through 2.6, the value is a sum of these:
 .nf
       1   PLL updates enabled
       2   PPS freq discipline enabled
@@ -268,7 +270,9 @@
 .IP "\fB\-d\fP, \fB\-\-directisa\fP"
 To read the CMOS clock accurately, \fBadjtimex\fP usually accesses the
 clock via the /dev/rtc device driver of the kernel, and makes use of its
-CMOS update-ended interrupt to detect the beginning of seconds. When the
+CMOS update-ended interrupt to detect the beginning of seconds. It
+will also try /dev/rtc0 (for udev), /dev/misc/rtc (for the obsolete
+devfs) and possibly others.  When the
 /dev/rtc driver is absent, or when the interrupt is not available,
 \fBadjtimex\fP can sometimes automatically fallback to a direct access
 method. This method detects the start of seconds by polling the
@@ -280,6 +284,9 @@
 better than the direct access method. It is advisable to not use the
 \fB\-\-directisa\fP switch, unless the CMOS chip or the motherboard
 don't properly provide the necessary interrupt.
+.IP "\fB\-n\fP, \fB\-\-nointerrupt\fP"
+Force immediate use of busywait access method, without first waiting
+for the interrupt timeout.
 .IP "\fB\-l\fP[\fIfile\fP], \fB\-\-log\fP[\fB=\fP\fIfile\fP]"
 Save the current values of the system and CMOS clocks, and optionally
 a reference time, to \fIfile\fP (default \fI/var/log/clocks.log\fP).


===================   3) other stuff

Makefile.in: New version number.  (Note the 1.24 release incorrectly
reported its version as 1.23).

Makefile and adjtimex.lsm are autogenerated and not reported here.

diff -ur adjtimex-1.24/Makefile.in adjtimex-1.25/Makefile.in
--- adjtimex-1.24/Makefile.in	2007-10-05 20:22:23.000000000 -0400
+++ adjtimex-1.25/Makefile.in	2008-04-06 21:35:53.000000000 -0400
@@ -2,7 +2,7 @@
  srcdir = .
 VPATH = .
 
-VERSION=1.23
+VERSION=1.25
 
 CFLAGS = @CFLAGS@ -Wall
 prefix = @prefix@

diff -ur adjtimex-1.24/debian/changelog adjtimex-1.25/debian/changelog
--- adjtimex-1.24/debian/changelog	2008-04-04 22:16:12.000000000 -0400
+++ adjtimex-1.25/debian/changelog	2008-08-08 19:48:13.000000000 -0400
@@ -1,3 +1,13 @@
+adjtimex (1.25-1) unstable; urgency=low
+
+  * New upstream release.  Set an alarm to avoid blocking even if /dev/rtc
+    never becomes readable, then fall back to busywait. New --nointerrupt
+    option to force busywait without first waiting for the interrupt
+    timeout (thanks to Alain Guibert). (closes:Bug#477637,Bug#474337).
+    Update man page (closes:Bug#435956).
+
+ -- James R. Van Zandt <jrv@debian.org>  Fri, 08 Aug 2008 19:43:38 -0400
+
 adjtimex (1.24-1) unstable; urgency=low
 
   * New upstream release




----------------------  changes from 1.25 to 1.26 ----------------

Here's the change log entry:
  
  adjtimex (1.26-1) unstable; urgency=low
  
    * New upstream release. Fix a possible hang with the Intersil ISL1208,
      one of those new non-PC-compatible RTCs. Cleanly exits with an error
      message, instead of either hang or returning garbage. (Thanks to Alain
      Guibert).
    
    * debian/control: bump Debian policy version to 3.8.0 (no changes needed)
    
    * debian/rules: use "filter" instead of "findstring" to parse
      DEB_BUILD_OPTIONS.
  
   -- James R. Van Zandt <jrv@debian.org>  Sun, 17 Aug 2008 17:52:14 -0400

The substantive change is explained in the following message.

------- Start of forwarded message -------
X-Spam-Status: No, score=0.0 required=6.0 tests=none autolearn=unavailable 
	version=3.1.0
Date: Tue, 12 Aug 2008 23:36:11 +0200 (CEST)
From: Alain Guibert <alguibert@free.fr>
To: "James R. Van Zandt" <jrvz@comcast.net>
Cc: Julian Gilbey <jdg@debian.org>
Subject: Re: adjtimex hang (bug#474337, bug#477637
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="17pEHd4RhPHOinZp"
In-Reply-To: <20080811084011.GA615@free.fr>

- --17pEHd4RhPHOinZp

Hello James,

 On Monday, August 11, 2008 at 10:40:11 +0200, Alain Guibert wrote:

> another possible hang, with the Intersil ISL1208, one of those new
> non-PC-compatible RTCs. They can too easely stop ticking.

Fixed by RTC_RD_TIME-failures-timeouts.patch attached. It uses the brand
new alarm_timeout() during busywait_second_change(), and additionally
treats errors reported by the /dev/rtc driver. Treats, ie. cleanly exits
with an error message, instead of either hang or do GIGO.

While at it, also come attached 2 minor patches:

 - perror-adjtimex-prefix.patch: cosmetic change in error messages.

 - nointerrupt-kernel-bug.patch: fix wrong version in a comment.

All patches are against the candidate 1.25 from your homepage.


BTW this Intersil chip is quite nice: It has an integrated drift rate
correction. Your RTC runs fast by +20 PPM average? Write -20 to a
register, and it will tick at the good rythm... :-))


Alain.

- --17pEHd4RhPHOinZp
Content-Disposition: attachment; filename="RTC_RD_TIME-failures-timeouts.patch"

2008-08-12  Alain Guibert <alguibert@free.fr>

	* adjtimex.c (cmos_read_time, busywait_second_change): Treat all
	  forms of RTC_RD_TIME failures, with error reported by the
	  device driver, and without. Avoid hangs due to stopped RTCs
	  via an alarm timeout on the tick detection loop.


diff -prud adjtimex-1.25.orig/adjtimex.c adjtimex-1.25/adjtimex.c
- --- adjtimex-1.25.orig/adjtimex.c	Tue Aug 12 19:23:07 2008
+++ adjtimex-1.25/adjtimex.c	Tue Aug 12 19:46:58 2008
@@ -803,7 +805,25 @@ cmos_read_time (time_t *cmos_timep, doub
 	 Now get this just beginning RTC second, unless we already have it */
 
       if (!got_time)
- -	ioctl (cmos_fd, RTC_RD_TIME, &tm);
+	{
+	  rc = ioctl (cmos_fd, RTC_RD_TIME, &tm);
+
+	  /* RTC_RD_TIME can fail when the device driver detects
+	     that the RTC isn't running or contains invalid data.
+	     Such failure has been detected earlier, unless: We used
+	     noint_fallback=1 to get busywait_uip_fall() as fallback.
+	     Or: UIE interrupts do beat, but RTC is invalid. */
+	  if (rc == -1)
+	    {
+	      char message[128];
+	      snprintf(message, sizeof(message),
+			"adjtimex: "
+			"ioctl(%s, RTC_RD_TIME) to read the CMOS clock failed",
+			cmos_device);
+	      perror(message);
+	      exit(1);
+	    }
+	}
 
 
       /* disable update complete interrupts
@@ -911,7 +931,21 @@ cmos_read_time (time_t *cmos_timep, doub
 }
 
 
- -/* busywait for UIP fall and timestamp this event */
+/*
+ * busywait for UIP fall and timestamp this event
+ *
+ * Maintainance note: In order to detect its fall, we first have to
+ * detect the update-in-progress flag up state. This UIP up state lasts
+ * for 2 ms each second. Detecting a 2 ms event, when other processes
+ * can steal us one or several 10 ms timeslices, this is something that
+ * can very easely fail. We *can* miss a second tick, or even several in
+ * a row, under heavy system load. Missing ticks is not a severe problem
+ * for adjtimex, as long as we timestamp accurately the tick we'll
+ * finally catch. However there is a timeout issue: we can't just arm an
+ * alarm_timeout() for 2 seconds, as when waiting for UIE interrupts and
+ * in busywait_second_change(), because it would make us hard fail after
+ * only one or two missed ticks.
+ */
 static void
 busywait_uip_fall(struct timeval *timestamp)
 {
@@ -964,18 +998,54 @@ static void
 busywait_second_change(struct tm *cmos, struct timeval *timestamp)
 {
   struct tm begin;
+  int rc;
 
   if (verbose)
     fprintf (stdout, "waiting for CMOS time change\n");
 
- -  /* pick the time, then loop until it changes */
- -  ioctl (cmos_fd, RTC_RD_TIME, &begin);
+  alarm_timeout(1);	/* arm a 2 seconds timeout */
+
+  /* pick a reference time */
+  rc = ioctl (cmos_fd, RTC_RD_TIME, &begin);
+
+  /* RTC_RD_TIME can fail when the device driver detects
+     that the RTC isn't running or contains invalid data */
+  if (rc == -1)
+    {
+      char message[128];
+      snprintf(message, sizeof(message),
+		"adjtimex: "
+		"ioctl(%s, RTC_RD_TIME) to read the CMOS clock failed",
+		cmos_device);
+      perror(message);
+      exit(1);
+    }
+
+  /* loop until time changes */
   do
     {
       ioctl (cmos_fd, RTC_RD_TIME, cmos);
     }
- -  while (cmos->tm_sec == begin.tm_sec);
+  while ((cmos->tm_sec == begin.tm_sec) && !timeout);
   gettimeofday(timestamp, NULL);
+
+  alarm_timeout(0);	/* disable timeout */
+
+  /* Sometimes the RTC isn't running, but the device driver didn't
+     notice. Then the RTC_RD_TIME call succeeds, but provides us some
+     static wrong time. That's why we needed an alarm timeout */
+
+  /* timeout without a change? */
+  if (cmos->tm_sec == begin.tm_sec)
+    {
+      fprintf(stderr,
+		"adjtimex: timeout waiting for CMOS time change on %s\n"
+		"The CMOS clock appears to be stopped:\n"
+		"Please try to set and restart it with hwclock --systohc\n",
+		cmos_device);
+      exit (1);
+    }
+
 }
 
 static inline void 

- --17pEHd4RhPHOinZp
Content-Disposition: attachment; filename="perror-adjtimex-prefix.patch"

2008-08-12  Alain Guibert <alguibert@free.fr>

	* adjtimex.c (cmos_init_directisa, cmos_read_time): add an
	  "adjtimex:" prefix to several hard failure messages.


diff -prud adjtimex-1.25.orig/adjtimex.c adjtimex-1.25/adjtimex.c
- --- adjtimex-1.25.orig/adjtimex.c	Tue Aug 12 19:23:07 2008
+++ adjtimex-1.25/adjtimex.c	Tue Aug 12 19:46:58 2008
@@ -621,7 +621,7 @@ void cmos_init_directisa ()
 
   if (ioperm (0x70, 2, 1))
     {
- -      fprintf (stderr, "clock: unable to get I/O port access\n");
+      fprintf (stderr, "adjtimex: unable to get I/O port access\n");
       exit (1);
     }
 #else
@@ -635,7 +635,7 @@ void cmos_init_directisa ()
     port_fd = open ("/dev/port", O_RDWR);
   if (port_fd < 0)
     {
- -      perror ("unable to open /dev/port read/write : ");
+      perror ("adjtimex: unable to open /dev/port read/write");
       exit (1);
     }
   if (verbose)
@@ -643,7 +643,7 @@ void cmos_init_directisa ()
 
   if (lseek (port_fd, 0x70, 0) < 0 || lseek (port_fd, 0x71, 0) < 0)
     {
- -      perror ("unable to seek port 0x70 in /dev/port : ");
+      perror ("adjtimex: unable to seek port 0x70 in /dev/port");
       exit (1);
     }
 #endif
@@ -755,7 +755,9 @@ cmos_read_time (time_t *cmos_timep, doub
 		      {
 			char message[128];
 			snprintf(message, sizeof(message),
- -				 "read() from %s to wait for clock tick failed", cmos_device);
+				"adjtimex: "
+				"read() from %s to wait for clock tick failed",
+				cmos_device);
 			perror(message);
 			exit(1);
 		      }

- --17pEHd4RhPHOinZp
Content-Disposition: attachment; filename="nointerrupt-kernel-bug.patch"

2008-08-12  Alain Guibert <alguibert@free.fr>

	* adjtimex.c (busywait_second_change): fix in comment a mistaken
	  version: The RTC_RD_TIME misfeature severely perturbating the
	  accuracy of our --nointerrupt tick detection method has been
	  fixed in kernel 2.6.13, not 2.6.16


diff -prud adjtimex-1.25.orig/adjtimex.c adjtimex-1.25/adjtimex.c
- --- adjtimex-1.25.orig/adjtimex.c	Tue Aug 12 19:23:07 2008
+++ adjtimex-1.25/adjtimex.c	Tue Aug 12 19:46:58 2008
@@ -952,7 +986,7 @@ busywait_uip_fall(struct timeval *timest
  *
  * Important note: an ioctl(RTC_RD_TIME) call that happens while the RTC
  * is updating itself (UIP up, a 2 milliseconds long event) will block.
- - * Properly block until UIP release on recent Linux kernels since 2.6.16.
+ * Properly block until UIP release on recent Linux kernels since 2.6.13.
  * However all older Linux kernels had a misfeature: they blocked much
  * longer than necessary, up to 20 ms longer in the worst case.
  * The method used here cannot detect precisely the CMOS clock tick on

- --17pEHd4RhPHOinZp--
------- End of forwarded message -------


Reply to: