hwclock: hctosys drift compensation II

Allowing hctosys to drift compensate facilitates:

More precise setting of the System Clock early in
the boot process when --adjust cannot be used
because the file system is not writeable.

Applies sub second drift corrections immediately,
where as --adjust cannot.

Reduces boot time by not calling hwclock multiple
times, e.g., --hctosys early before fsck when the
file system is read-only, then --adjust later when
the file system is read-write and --hctosys again
for drift correction.

Use of --adjust elsewhere may no longer be
necessary.

Part II

After the original submission of this patch I
realized that now all operations except --systz
require drift corrected Hardware Clock time.
Therefore, it should be done only once early in
the process. Upon implementation of that premise
many improvements were facilitated:

* Adds drift correction to --hctosys.
* Adds setting system time with sub-second precision.
* Adds --get, a drift corrected 'show' operation.
* Improves drift factor calculation precision while
   greatly simplifying its algorithm.
* Fixes --show bug, printing integer sub-seconds, and
   now uses a more intuitive positive value.
* Fixes --predict bug, drift correction must be
   negated to predict future RTC time.
* Reduces the number of function arguments and
   lines of code.

Signed-off-by: J William Piggott <elseifthen@gmx.com>

1 files changed, 91 insertions, 114 deletions

diff --git a/sys-utils/hwclock.c b/sys-utils/hwclock.c
index e7801583b..30db8b628 100644
--- a/sys-utils/hwclock.c
+++ b/sys-utils/hwclock.c
@@ -182,6 +182,18 @@ static void read_date_from_file(struct tm *tm)
 }
 
 /*
+ * time_t to timeval conversion.
+ */
+static struct timeval t2tv(time_t timet)
+{
+	struct timeval rettimeval;
+
+	rettimeval.tv_sec = timet;
+	rettimeval.tv_usec = 0;
+	return rettimeval;
+}
+
+/*
  * The difference in seconds between two times in "timeval" format.
  */
 double time_diff(struct timeval subtrahend, struct timeval subtractor)
@@ -679,8 +691,7 @@ set_hardware_clock_exact(const time_t sethwtime,
  * Include in the output the adjustment "sync_duration".
  */
 static void
-display_time(const bool hclock_valid, const time_t systime,
-	     const double sync_duration)
+display_time(const bool hclock_valid, struct timeval hwctime)
 {
 	if (!hclock_valid)
 		warnx(_
@@ -692,9 +703,9 @@ display_time(const bool hclock_valid, const time_t systime,
 		char *format = "%c";
 		char ctime_now[200];
 
-		lt = localtime(&systime);
+		lt = localtime(&hwctime.tv_sec);
 		strftime(ctime_now, sizeof(ctime_now), format, lt);
-		printf(_("%s  %.6f seconds\n"), ctime_now, -(sync_duration));
+		printf(_("%s  .%06d seconds\n"), ctime_now, (int)hwctime.tv_usec);
 	}
 }
 
@@ -808,7 +819,7 @@ static int interpret_date_string(const char *date_opt, time_t * const time_p)
  * have.
  */
 static int
-set_system_clock(const bool hclock_valid, const time_t newtime,
+set_system_clock(const bool hclock_valid, const struct timeval newtime,
 		 const bool testing)
 {
 	int retcode;
@@ -819,15 +830,11 @@ set_system_clock(const bool hclock_valid, const time_t newtime,
 		       "we cannot set the System Time from it."));
 		retcode = 1;
 	} else {
-		struct timeval tv;
 		struct tm *broken;
 		int minuteswest;
 		int rc;
 
-		tv.tv_sec = newtime;
-		tv.tv_usec = 0;
-
-		broken = localtime(&newtime);
+		broken = localtime(&newtime.tv_sec);
 #ifdef HAVE_TM_GMTOFF
 		minuteswest = -broken->tm_gmtoff / 60;	/* GNU extension */
 #else
@@ -839,7 +846,7 @@ set_system_clock(const bool hclock_valid, const time_t newtime,
 		if (debug) {
 			printf(_("Calling settimeofday:\n"));
 			printf(_("\ttv.tv_sec = %ld, tv.tv_usec = %ld\n"),
-			       (long)tv.tv_sec, (long)tv.tv_usec);
+			       (long)newtime.tv_sec, (long)newtime.tv_usec);
 			printf(_("\ttz.tz_minuteswest = %d\n"), minuteswest);
 		}
 		if (testing) {
@@ -849,7 +856,7 @@ set_system_clock(const bool hclock_valid, const time_t newtime,
 		} else {
 			const struct timezone tz = { minuteswest, 0 };
 
-			rc = settimeofday(&tv, &tz);
+			rc = settimeofday(&newtime, &tz);
 			if (rc) {
 				if (errno == EPERM) {
 					warnx(_
@@ -975,9 +982,9 @@ static int set_system_clock_timezone(const bool universal, const bool testing)
  */
 static void
 adjust_drift_factor(struct adjtime *adjtime_p,
-		    const time_t nowtime,
+		    const struct timeval nowtime,
 		    const bool hclock_valid,
-		    const time_t hclocktime, const double sync_delay)
+		    const struct timeval hclocktime)
 {
 	if (!hclock_valid) {
 		if (debug)
@@ -990,7 +997,7 @@ adjust_drift_factor(struct adjtime *adjtime_p,
 				 "calibration time is zero,\n"
 				 "so history is bad and calibration startover "
 				 "is necessary.\n"));
-	} else if ((hclocktime - adjtime_p->last_calib_time) < 24 * 60 * 60) {
+	} else if ((hclocktime.tv_sec - adjtime_p->last_calib_time) < 24 * 60 * 60) {
 		if (debug)
 			printf(_("Not adjusting drift factor because it has "
 				 "been less than a day since the last "
@@ -1010,39 +1017,16 @@ adjust_drift_factor(struct adjtime *adjtime_p,
 		 * but 195 days + 1 second equals 195 days in floats.)
 		 */
 		const double sec_per_day = 24.0 * 60.0 * 60.0;
-		double atime_per_htime;
-		double adj_days, cal_days;
-		double exp_drift, unc_drift;
 		double factor_adjust;
 		double drift_factor;
+		struct timeval last_calib;
 
-		/* Adjusted time units per hardware time unit */
-		atime_per_htime = 1.0 + adjtime_p->drift_factor / sec_per_day;
+		last_calib = t2tv(adjtime_p->last_calib_time);
 
-		/* Days since last adjustment (in hardware clock time) */
-		adj_days = (double)(hclocktime - adjtime_p->last_adj_time)
-		    / sec_per_day;
+		factor_adjust = time_diff(nowtime, hclocktime) /
+				(time_diff(nowtime, last_calib) / sec_per_day);
 
-		/* Expected drift (sec) since last adjustment */
-		exp_drift = adj_days * adjtime_p->drift_factor
-		    + adjtime_p->not_adjusted;
-
-		/* Uncorrected drift (sec) since last calibration */
-		unc_drift = (double)(nowtime - hclocktime)
-		    + sync_delay - exp_drift;
-
-		/* Days since last calibration (in hardware clock time) */
-		cal_days = ((double)(adjtime_p->last_adj_time
-				     - adjtime_p->last_calib_time)
-			    + adjtime_p->not_adjusted)
-		    / (sec_per_day * atime_per_htime) + adj_days;
-
-		/* Amount to add to previous drift factor */
-		factor_adjust = unc_drift / cal_days;
-
-		/* New drift factor */
 		drift_factor = adjtime_p->drift_factor + factor_adjust;
-
 		if (fabs(drift_factor) > MAX_DRIFT) {
 			if (debug)
 				printf(_("Clock drift factor was calculated as "
@@ -1053,19 +1037,19 @@ adjust_drift_factor(struct adjtime *adjtime_p,
 		} else {
 			if (debug)
 				printf(_("Clock drifted %.1f seconds in the past "
-					 "%d seconds in spite of a drift factor of "
+					 "%.1f seconds\nin spite of a drift factor of "
 					 "%f seconds/day.\n"
 					 "Adjusting drift factor by %f seconds/day\n"),
-				       unc_drift,
-				       (int)(nowtime - adjtime_p->last_calib_time),
+				       time_diff(nowtime, hclocktime),
+				       time_diff(nowtime, last_calib),
 				       adjtime_p->drift_factor, factor_adjust);
 		}
 
 		adjtime_p->drift_factor = drift_factor;
 	}
-	adjtime_p->last_calib_time = nowtime;
+	adjtime_p->last_calib_time = nowtime.tv_sec;
 
-	adjtime_p->last_adj_time = nowtime;
+	adjtime_p->last_adj_time = nowtime.tv_sec;
 
 	adjtime_p->not_adjusted = 0;
 
@@ -1088,26 +1072,23 @@ static void
 calculate_adjustment(const double factor,
 		     const time_t last_time,
 		     const double not_adjusted,
-		     const time_t systime, int *adjustment_p, double *retro_p)
+		     const time_t systime, struct timeval *tdrift_p)
 {
 	double exact_adjustment;
 
 	exact_adjustment =
 	    ((double)(systime - last_time)) * factor / (24 * 60 * 60)
 	    + not_adjusted;
-	*adjustment_p = FLOOR(exact_adjustment);
-
-	*retro_p = exact_adjustment - (double)*adjustment_p;
+	tdrift_p->tv_sec = FLOOR(exact_adjustment);
+	tdrift_p->tv_usec = (exact_adjustment -
+				 (double)tdrift_p->tv_sec) * 1E6;
 	if (debug) {
 		printf(P_("Time since last adjustment is %d second\n",
 			"Time since last adjustment is %d seconds\n",
 		       (int)(systime - last_time)),
 		       (int)(systime - last_time));
-		printf(P_("Need to insert %d second and refer time back "
-			 "%.6f seconds ago\n",
-			 "Need to insert %d seconds and refer time back "
-			 "%.6f seconds ago\n", *adjustment_p),
-			 *adjustment_p, *retro_p);
+		printf(_("Calculated Hardware Clock drift is %ld.%06d seconds\n"),
+		       (long)tdrift_p->tv_sec, (int)tdrift_p->tv_usec);
 	}
 }
 
@@ -1201,7 +1182,7 @@ static void save_adjtime(const struct adjtime adjtime, const bool testing)
  */
 static void
 do_adjustment(struct adjtime *adjtime_p,
-	      const bool hclock_valid, const time_t hclocktime,
+	      const bool hclock_valid, const struct timeval hclocktime,
 	      const struct timeval read_time,
 	      const bool universal, const bool testing)
 {
@@ -1221,27 +1202,13 @@ do_adjustment(struct adjtime *adjtime_p,
 			printf(_("Not setting clock because drift factor %f is far too high.\n"),
 				adjtime_p->drift_factor);
 	} else {
-		int adjustment;
-		/* Number of seconds we must insert in the Hardware Clock */
-		double retro;
-		/*
-		 * Fraction of second we have to remove from clock after
-		 * inserting <adjustment> whole seconds.
-		 */
-		calculate_adjustment(adjtime_p->drift_factor,
-				     adjtime_p->last_adj_time,
-				     adjtime_p->not_adjusted,
-				     hclocktime, &adjustment, &retro);
-		if (adjustment > 0 || adjustment < -1) {
-			set_hardware_clock_exact(hclocktime + adjustment,
-						 time_inc(read_time, -retro),
-						 universal, testing);
-			adjtime_p->last_adj_time = hclocktime + adjustment;
-			adjtime_p->not_adjusted = 0;
-			adjtime_p->dirty = TRUE;
-		} else if (debug)
-			printf(_("Needed adjustment is less than one second, "
-				 "so not setting clock.\n"));
+		set_hardware_clock_exact(hclocktime.tv_sec,
+					 time_inc(read_time,
+						  -(hclocktime.tv_usec / 1E6)),
+					 universal, testing);
+		adjtime_p->last_adj_time = hclocktime.tv_sec;
+		adjtime_p->not_adjusted = 0;
+		adjtime_p->dirty = TRUE;
 	}
 }
 
@@ -1281,7 +1248,7 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
 		 const bool hctosys, const bool systohc, const bool systz,
 		 const struct timeval startup_time,
 		 const bool utc, const bool local_opt,
-		 const bool testing, const bool predict)
+		 const bool testing, const bool predict, const bool get)
 {
 	/* Contents of the adjtime file, or what they should be. */
 	struct adjtime adjtime;
@@ -1300,7 +1267,8 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
 	 * synchronized to its next clock tick when we
 	 * started up. Defined only if hclock_valid is true.
 	 */
-	time_t hclocktime = 0;
+	struct timeval hclocktime = { 0, 0 };
+	struct timeval tdrift;
 	/* local return code */
 	int rc = 0;
 
@@ -1310,13 +1278,12 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
 			return EX_NOPERM;
 	}
 
-	if (!noadjfile
-	    && (adjust || set || systohc || (!utc && !local_opt) || predict)) {
+	if (!noadjfile && !(systz && (utc || local_opt))) {
 		rc = read_adjtime(&adjtime);
 		if (rc)
 			return rc;
 	} else {
-		/* A little trick to avoid reading the file if we don't have to */
+		/* A little trick to avoid writing the file if we don't have to */
 		adjtime.dirty = FALSE;
 	}
 
@@ -1328,7 +1295,7 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
 		adjtime.dirty = TRUE;
 	}
 
-	if (show || adjust || hctosys || (!noadjfile && !systz && !predict)) {
+	if (show || get || adjust || hctosys || (!noadjfile && !systz && !predict)) {
 		/* data from HW-clock are required */
 		rc = synchronize_to_clock_tick();
 
@@ -1351,26 +1318,40 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
 		 */
 		if (!rc) {
 			rc = read_hardware_clock(universal,
-						 &hclock_valid, &hclocktime);
+						 &hclock_valid, &hclocktime.tv_sec);
 			if (rc && !set && !systohc)
 				return EX_IOERR;
 		}
 	}
-
-	if (show) {
-		display_time(hclock_valid, hclocktime,
-			     time_diff(read_time, startup_time));
+	hclocktime = predict ? t2tv(set_time) : hclocktime;
+	calculate_adjustment(adjtime.drift_factor,
+			     adjtime.last_adj_time,
+			     adjtime.not_adjusted,
+			     hclocktime.tv_sec, &tdrift);
+	if (!show && !predict)
+		hclocktime = time_inc(tdrift, hclocktime.tv_sec);
+	if (predict)
+		hclocktime = time_inc(hclocktime, (double)
+				      -(tdrift.tv_sec + tdrift.tv_sec / 1E6));
+	if (show || get) {
+		display_time(hclock_valid,
+			     time_inc(hclocktime, -time_diff
+				      (read_time, startup_time)));
 	} else if (set) {
 		set_hardware_clock_exact(set_time, startup_time,
 					 universal, testing);
 		if (!noadjfile)
-			adjust_drift_factor(&adjtime, set_time,
-					    hclock_valid,
-					    hclocktime,
-					    time_diff(read_time, startup_time));
+			adjust_drift_factor(&adjtime,
+					    time_inc(t2tv(set_time), time_diff
+						     (read_time, startup_time)),
+					    hclock_valid, hclocktime);
 	} else if (adjust) {
-		do_adjustment(&adjtime, hclock_valid,
-			      hclocktime, read_time, universal, testing);
+		if (tdrift.tv_sec > 0 || tdrift.tv_sec < -1)
+			do_adjustment(&adjtime, hclock_valid,
+				      hclocktime, read_time, universal, testing);
+		else
+			printf(_("Needed adjustment is less than one second, "
+				 "so not setting clock.\n"));
 	} else if (systohc) {
 		struct timeval nowtime, reftime;
 		/*
@@ -1386,10 +1367,8 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
 					 reftime.tv_sec,
 					 reftime, universal, testing);
 		if (!noadjfile)
-			adjust_drift_factor(&adjtime, (time_t)
-					    reftime.tv_sec,
-					    hclock_valid, hclocktime, (double)
-					    read_time.tv_usec / 1E6);
+			adjust_drift_factor(&adjtime, nowtime,
+					    hclock_valid, hclocktime);
 	} else if (hctosys) {
 		rc = set_system_clock(hclock_valid, hclocktime, testing);
 		if (rc) {
@@ -1403,19 +1382,12 @@ manipulate_clock(const bool show, const bool adjust, const bool noadjfile,
 			return rc;
 		}
 	} else if (predict) {
-		int adjustment;
-		double retro;
-
-		calculate_adjustment(adjtime.drift_factor,
-				     adjtime.last_adj_time,
-				     adjtime.not_adjusted,
-				     set_time, &adjustment, &retro);
 		if (debug) {
 			printf(_
 			       ("At %ld seconds after 1969, RTC is predicted to read %ld seconds after 1969.\n"),
-			       set_time, set_time + adjustment);
+			       set_time, (long)hclocktime.tv_sec);
 		}
-		display_time(TRUE, set_time + adjustment, -retro);
+		display_time(TRUE, hclocktime);
 	}
 	if (!noadjfile)
 		save_adjtime(adjtime, testing);
@@ -1644,7 +1616,7 @@ int main(int argc, char **argv)
 	/* Variables set by various options; show may also be set later */
 	/* The options debug, badyear and epoch_option are global */
 	bool show, set, systohc, hctosys, systz, adjust, getepoch, setepoch,
-	    predict, compare;
+	    predict, compare, get;
 	bool utc, testing, local_opt, noadjfile, directisa;
 	char *date_opt;
 #ifdef __alpha__
@@ -1657,6 +1629,7 @@ int main(int argc, char **argv)
 		OPT_DATE,
 		OPT_DIRECTISA,
 		OPT_EPOCH,
+		OPT_GET,
 		OPT_GETEPOCH,
 		OPT_LOCALTIME,
 		OPT_NOADJFILE,
@@ -1704,13 +1677,14 @@ int main(int argc, char **argv)
 		{"adjfile",	1, 0, OPT_ADJFILE},
 		{"systz",	0, 0, OPT_SYSTZ},
 		{"predict-hc",	0, 0, OPT_PREDICT_HC},
+		{"get",		0, 0, OPT_GET},
 		{NULL,		0, NULL, 0}
 	};
 
 	static const ul_excl_t excl[] = {	/* rows and cols in in ASCII order */
 		{ 'a','r','s','w',
-		  OPT_GETEPOCH,	OPT_PREDICT_HC, OPT_SET,
-		  OPT_SETEPOCH, OPT_SYSTZ },
+		  OPT_GET, OPT_GETEPOCH, OPT_PREDICT_HC,
+		  OPT_SET, OPT_SETEPOCH, OPT_SYSTZ },
 		{ 'u', OPT_LOCALTIME},
 		{ OPT_ADJFILE, OPT_NOADJFILE },
 		{ 0 }
@@ -1747,7 +1721,7 @@ int main(int argc, char **argv)
 
 	/* Set option defaults */
 	show = set = systohc = hctosys = systz = adjust = noadjfile = predict =
-	    compare = FALSE;
+	    compare = get = FALSE;
 	getepoch = setepoch = utc = local_opt = directisa = testing = debug = FALSE;
 #ifdef __alpha__
 	ARCconsole = Jensen = SRM = funky_toy = badyear = FALSE;
@@ -1837,6 +1811,9 @@ int main(int argc, char **argv)
 		case OPT_PREDICT_HC:
 			predict = TRUE;		/* --predict-hc */
 			break;
+		case OPT_GET:
+			get = TRUE;		/* --get */
+			break;
 #ifdef __linux__
 		case 'f':
 			rtc_dev_name = optarg;	/* --rtc */
@@ -1894,7 +1871,7 @@ int main(int argc, char **argv)
 	}
 
 	if (!(show | set | systohc | hctosys | systz | adjust | getepoch
-	      | setepoch | predict | compare))
+	      | setepoch | predict | compare | get))
 		show = 1;	/* default to show */
 
 	if (getuid() == 0)
@@ -1951,7 +1928,7 @@ int main(int argc, char **argv)
 	} else
 		rc = manipulate_clock(show, adjust, noadjfile, set, set_time,
 			      hctosys, systohc, systz, startup_time, utc,
-			      local_opt, testing, predict);
+			      local_opt, testing, predict, get);
 
 	hwclock_exit(rc);
 	return rc;		/* Not reached */