Make access to basetime MP safe and interrupt-race safe by using a simple
authorMatthew Dillon <dillon@dragonflybsd.org>
Sat, 23 Apr 2005 20:34:32 +0000 (20:34 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Sat, 23 Apr 2005 20:34:32 +0000 (20:34 +0000)
tail-chasing FIFO for updates to basetime.

Reorganize the PROC sysctl's.  This actually undoes part of the last commit
and redoes it, though there was nothing wrong with the last commit.

Move the SYSCTL_OUT phase to *after* the SYSCTL_IN phase.

sys/kern/kern_clock.c
sys/kern/kern_ntptime.c
sys/kern/kern_time.c

index e9a4be1..41dd49c 100644 (file)
@@ -70,7 +70,7 @@
  *
  *     @(#)kern_clock.c        8.5 (Berkeley) 1/21/94
  * $FreeBSD: src/sys/kern/kern_clock.c,v 1.105.2.10 2002/10/17 13:19:40 maxim Exp $
- * $DragonFly: src/sys/kern/kern_clock.c,v 1.36 2005/04/20 17:57:16 joerg Exp $
+ * $DragonFly: src/sys/kern/kern_clock.c,v 1.37 2005/04/23 20:34:32 dillon Exp $
  */
 
 #include "opt_ntp.h"
@@ -127,25 +127,53 @@ SYSCTL_OPAQUE(_kern, OID_AUTO, cp_time, CTLFLAG_RD, &cp_time, sizeof(cp_time),
  * whenever the real time is set based on the compensated elapsed time
  * in seconds (gd->gd_time_seconds).
  *
- * basetime is used to calculate the compensated real time of day.  Chunky
- * changes to the time, aka settimeofday(), are made by modifying basetime.
- *
  * The gd_time_seconds and gd_cpuclock_base fields remain fairly monotonic.
  * Slight adjustments to gd_cpuclock_base are made to phase-lock it to
  * the real time.
  */
 struct timespec boottime;      /* boot time (realtime) for reference only */
-static struct timespec basetime;       /* base time adjusts uptime -> realtime */
 time_t time_second;            /* read-only 'passive' uptime in seconds */
 
+/*
+ * basetime is used to calculate the compensated real time of day.  The
+ * basetime can be modified on a per-tick basis by the adjtime(), 
+ * ntp_adjtime(), and sysctl-based time correction APIs.
+ *
+ * Note that frequency corrections can also be made by adjusting
+ * gd_cpuclock_base.
+ *
+ * basetime is a tail-chasing FIFO, updated only by cpu #0.  The FIFO is
+ * used on both SMP and UP systems to avoid MP races between cpu's and
+ * interrupt races on UP systems.
+ */
+#define BASETIME_ARYSIZE       16
+#define BASETIME_ARYMASK       (BASETIME_ARYSIZE - 1)
+static struct timespec basetime[BASETIME_ARYSIZE];
+static volatile int basetime_index;
+
+static int
+sysctl_get_basetime(SYSCTL_HANDLER_ARGS)
+{
+       struct timespec *bt;
+       int error;
+
+       bt = &basetime[basetime_index];
+       if (req->oldptr != NULL)
+               error = SYSCTL_OUT(req, bt, sizeof(*bt));
+       else
+               error = 0;
+       return (error);
+}
+
 SYSCTL_STRUCT(_kern, KERN_BOOTTIME, boottime, CTLFLAG_RD,
     &boottime, timeval, "System boottime");
-SYSCTL_STRUCT(_kern, OID_AUTO, basetime, CTLFLAG_RD,
-    &basetime, timeval, "System basetime");
+SYSCTL_PROC(_kern, OID_AUTO, basetime, CTLTYPE_STRUCT|CTLFLAG_RD, 0, 0,
+    sysctl_get_basetime, "S,timeval", "System basetime");
 
 static void hardclock(systimer_t info, struct intrframe *frame);
 static void statclock(systimer_t info, struct intrframe *frame);
 static void schedclock(systimer_t info, struct intrframe *frame);
+static void getnanotime_nbt(struct timespec *nbt, struct timespec *tsp);
 
 int    ticks;                  /* system master ticks at hz */
 int    clocks_running;         /* tsleep/timeout clocks operational */
@@ -216,22 +244,28 @@ initclocks_pcpu(void)
  * instead we adjust basetime so basetime + gd_* results in the current
  * time of day.  This way the gd_* fields are guarenteed to represent
  * a monotonically increasing 'uptime' value.
+ *
+ * When set_timeofday() is called from userland, the system call forces it
+ * onto cpu #0 since only cpu #0 can update basetime_index.
  */
 void
 set_timeofday(struct timespec *ts)
 {
-       struct timespec ts2;
+       struct timespec *nbt;
+       int ni;
 
        /*
         * XXX SMP / non-atomic basetime updates
         */
        crit_enter();
-       nanouptime(&ts2);
-       basetime.tv_sec = ts->tv_sec - ts2.tv_sec;
-       basetime.tv_nsec = ts->tv_nsec - ts2.tv_nsec;
-       if (basetime.tv_nsec < 0) {
-           basetime.tv_nsec += 1000000000;
-           --basetime.tv_sec;
+       ni = (basetime_index + 1) & BASETIME_ARYMASK;
+       nbt = &basetime[ni];
+       nanouptime(nbt);
+       nbt->tv_sec = ts->tv_sec - nbt->tv_sec;
+       nbt->tv_nsec = ts->tv_nsec - nbt->tv_nsec;
+       if (nbt->tv_nsec < 0) {
+           nbt->tv_nsec += 1000000000;
+           --nbt->tv_sec;
        }
 
        /*
@@ -246,8 +280,15 @@ set_timeofday(struct timespec *ts)
         * suitable for use in the boottime calculation.  It is already taken
         * into account in the basetime calculation above.
         */
-       boottime.tv_sec = basetime.tv_sec;
+       boottime.tv_sec = nbt->tv_sec;
        ntp_delta = 0;
+
+       /*
+        * We now have a new basetime, update the index.
+        */
+       cpu_mb1();
+       basetime_index = ni;
+
        crit_exit();
 }
        
@@ -293,8 +334,10 @@ hardclock(systimer_t info, struct intrframe *frame)
         * by updating basetime.
         */
        if (gd->gd_cpuid == 0) {
+           struct timespec *nbt;
            struct timespec nts;
            int leap;
+           int ni;
 
            ++ticks;
 
@@ -306,90 +349,113 @@ hardclock(systimer_t info, struct intrframe *frame)
            if (tco->tc_poll_pps) 
                tco->tc_poll_pps(tco);
 #endif
+
            /*
-            * Apply adjtime corrections.  At the moment only do this if 
-            * we can get the MP lock to interlock with adjtime's modification
-            * of these variables.  Note that basetime adjustments are not
-            * MP safe either XXX.
+            * Calculate the new basetime index.  We are in a critical section
+            * on cpu #0 and can safely play with basetime_index.  Start
+            * with the current basetime and then make adjustments.
+            */
+           ni = (basetime_index + 1) & BASETIME_ARYMASK;
+           nbt = &basetime[ni];
+           *nbt = basetime[basetime_index];
+
+           /*
+            * Apply adjtime corrections.  (adjtime() API)
+            *
+            * adjtime() only runs on cpu #0 so our critical section is
+            * sufficient to access these variables.
             */
            if (ntp_delta != 0) {
-               basetime.tv_nsec += ntp_tick_delta;
+               nbt->tv_nsec += ntp_tick_delta;
                ntp_delta -= ntp_tick_delta;
                if ((ntp_delta > 0 && ntp_delta < ntp_tick_delta) ||
                    (ntp_delta < 0 && ntp_delta > ntp_tick_delta)) {
-                               ntp_tick_delta = ntp_delta;
+                       ntp_tick_delta = ntp_delta;
                }
            }
 
+           /*
+            * Apply permanent frequency corrections.  (sysctl API)
+            */
            if (ntp_tick_permanent != 0) {
                ntp_tick_acc += ntp_tick_permanent;
                if (ntp_tick_acc >= (1LL << 32)) {
-                   basetime.tv_nsec += ntp_tick_acc >> 32;
+                   nbt->tv_nsec += ntp_tick_acc >> 32;
                    ntp_tick_acc -= (ntp_tick_acc >> 32) << 32;
                } else if (ntp_tick_acc <= -(1LL << 32)) {
                    /* Negate ntp_tick_acc to avoid shifting the sign bit. */
-                   basetime.tv_nsec -= (-ntp_tick_acc) >> 32;
+                   nbt->tv_nsec -= (-ntp_tick_acc) >> 32;
                    ntp_tick_acc += ((-ntp_tick_acc) >> 32) << 32;
                }
            }
 
-           if (basetime.tv_nsec >= 1000000000) {
-                   basetime.tv_sec++;
-                   basetime.tv_nsec -= 1000000000;
-           } else if (basetime.tv_nsec < 0) {
-                   basetime.tv_sec--;
-                   basetime.tv_nsec += 1000000000;
-           }
-
-           if (ntp_leap_second) {
-               struct timespec tsp;
-               nanotime(&tsp);
-
-               if (ntp_leap_second == tsp.tv_sec) {
-                       if (ntp_leap_insert)
-                               basetime.tv_sec++;
-                       else
-                               basetime.tv_sec--;
-                       ntp_leap_second--;
-               }
+           if (nbt->tv_nsec >= 1000000000) {
+                   nbt->tv_sec++;
+                   nbt->tv_nsec -= 1000000000;
+           } else if (nbt->tv_nsec < 0) {
+                   nbt->tv_sec--;
+                   nbt->tv_nsec += 1000000000;
            }
 
            /*
-            * Apply per-tick compensation.  ticks_adj adjusts for both
-            * offset and frequency, and could be negative.
+            * Another per-tick compensation.  (for ntp_adjtime() API)
             */
-           if (nsec_adj != 0 && try_mplock()) {
+           if (nsec_adj != 0) {
                nsec_acc += nsec_adj;
                if (nsec_acc >= 0x100000000LL) {
-                   basetime.tv_nsec += nsec_acc >> 32;
+                   nbt->tv_nsec += nsec_acc >> 32;
                    nsec_acc = (nsec_acc & 0xFFFFFFFFLL);
                } else if (nsec_acc <= -0x100000000LL) {
-                   basetime.tv_nsec -= -nsec_acc >> 32;
+                   nbt->tv_nsec -= -nsec_acc >> 32;
                    nsec_acc = -(-nsec_acc & 0xFFFFFFFFLL);
                }
-               if (basetime.tv_nsec >= 1000000000) {
-                   basetime.tv_nsec -= 1000000000;
-                   ++basetime.tv_sec;
-               } else if (basetime.tv_nsec < 0) {
-                   basetime.tv_nsec += 1000000000;
-                   --basetime.tv_sec;
+               if (nbt->tv_nsec >= 1000000000) {
+                   nbt->tv_nsec -= 1000000000;
+                   ++nbt->tv_sec;
+               } else if (nbt->tv_nsec < 0) {
+                   nbt->tv_nsec += 1000000000;
+                   --nbt->tv_sec;
+               }
+           }
+
+           /************************************************************
+            *                  LEAP SECOND CORRECTION                  *
+            ************************************************************
+            *
+            * Taking into account all the corrections made above, figure
+            * out the new real time.  If the seconds field has changed
+            * then apply any pending leap-second corrections.
+            */
+           getnanotime_nbt(nbt, &nts);
+
+           /*
+            * Apply leap second (sysctl API)
+            */
+           if (ntp_leap_second) {
+               if (ntp_leap_second == nts.tv_sec) {
+                       if (ntp_leap_insert)
+                               nbt->tv_sec++;
+                       else
+                               nbt->tv_sec--;
+                       ntp_leap_second--;
                }
-               rel_mplock();
            }
 
            /*
-            * If the realtime-adjusted seconds hand rolls over then tell
-            * ntp_update_second() what we did in the last second so it can
-            * calculate what to do in the next second.  It may also add
-            * or subtract a leap second.
+            * Apply leap second (ntp_adjtime() API)
             */
-           getnanotime(&nts);
            if (time_second != nts.tv_sec) {
                leap = ntp_update_second(time_second, &nsec_adj);
-               basetime.tv_sec += leap;
-               time_second = nts.tv_sec + leap;
+               nbt->tv_sec += leap;
+               time_second = nbt->tv_sec;
                nsec_adj /= hz;
            }
+
+           /*
+            * Finally, our new basetime is ready to go live!
+            */
+           cpu_mb1();
+           basetime_index = ni;
        }
 
        /*
@@ -796,6 +862,7 @@ void
 getmicrotime(struct timeval *tvp)
 {
        struct globaldata *gd = mycpu;
+       struct timespec *bt;
        sysclock_t delta;
 
        do {
@@ -809,8 +876,9 @@ getmicrotime(struct timeval *tvp)
        }
        tvp->tv_usec = (cputimer_freq64_usec * delta) >> 32;
 
-       tvp->tv_sec += basetime.tv_sec;
-       tvp->tv_usec += basetime.tv_nsec / 1000;
+       bt = &basetime[basetime_index];
+       tvp->tv_sec += bt->tv_sec;
+       tvp->tv_usec += bt->tv_nsec / 1000;
        while (tvp->tv_usec >= 1000000) {
                tvp->tv_usec -= 1000000;
                ++tvp->tv_sec;
@@ -821,6 +889,7 @@ void
 getnanotime(struct timespec *tsp)
 {
        struct globaldata *gd = mycpu;
+       struct timespec *bt;
        sysclock_t delta;
 
        do {
@@ -834,18 +903,46 @@ getnanotime(struct timespec *tsp)
        }
        tsp->tv_nsec = (cputimer_freq64_nsec * delta) >> 32;
 
-       tsp->tv_sec += basetime.tv_sec;
-       tsp->tv_nsec += basetime.tv_nsec;
+       bt = &basetime[basetime_index];
+       tsp->tv_sec += bt->tv_sec;
+       tsp->tv_nsec += bt->tv_nsec;
        while (tsp->tv_nsec >= 1000000000) {
                tsp->tv_nsec -= 1000000000;
                ++tsp->tv_sec;
        }
 }
 
+static void
+getnanotime_nbt(struct timespec *nbt, struct timespec *tsp)
+{
+       struct globaldata *gd = mycpu;
+       sysclock_t delta;
+
+       do {
+               tsp->tv_sec = gd->gd_time_seconds;
+               delta = gd->gd_hardclock.time - gd->gd_cpuclock_base;
+       } while (tsp->tv_sec != gd->gd_time_seconds);
+
+       if (delta >= cputimer_freq) {
+               tsp->tv_sec += delta / cputimer_freq;
+               delta %= cputimer_freq;
+       }
+       tsp->tv_nsec = (cputimer_freq64_nsec * delta) >> 32;
+
+       tsp->tv_sec += nbt->tv_sec;
+       tsp->tv_nsec += nbt->tv_nsec;
+       while (tsp->tv_nsec >= 1000000000) {
+               tsp->tv_nsec -= 1000000000;
+               ++tsp->tv_sec;
+       }
+}
+
+
 void
 microtime(struct timeval *tvp)
 {
        struct globaldata *gd = mycpu;
+       struct timespec *bt;
        sysclock_t delta;
 
        do {
@@ -859,8 +956,9 @@ microtime(struct timeval *tvp)
        }
        tvp->tv_usec = (cputimer_freq64_usec * delta) >> 32;
 
-       tvp->tv_sec += basetime.tv_sec;
-       tvp->tv_usec += basetime.tv_nsec / 1000;
+       bt = &basetime[basetime_index];
+       tvp->tv_sec += bt->tv_sec;
+       tvp->tv_usec += bt->tv_nsec / 1000;
        while (tvp->tv_usec >= 1000000) {
                tvp->tv_usec -= 1000000;
                ++tvp->tv_sec;
@@ -871,6 +969,7 @@ void
 nanotime(struct timespec *tsp)
 {
        struct globaldata *gd = mycpu;
+       struct timespec *bt;
        sysclock_t delta;
 
        do {
@@ -884,8 +983,9 @@ nanotime(struct timespec *tsp)
        }
        tsp->tv_nsec = (cputimer_freq64_nsec * delta) >> 32;
 
-       tsp->tv_sec += basetime.tv_sec;
-       tsp->tv_nsec += basetime.tv_nsec;
+       bt = &basetime[basetime_index];
+       tsp->tv_sec += bt->tv_sec;
+       tsp->tv_nsec += bt->tv_nsec;
        while (tsp->tv_nsec >= 1000000000) {
                tsp->tv_nsec -= 1000000000;
                ++tsp->tv_sec;
@@ -900,7 +1000,10 @@ time_t
 get_approximate_time_t(void)
 {
        struct globaldata *gd = mycpu;
-       return(gd->gd_time_seconds + basetime.tv_sec);
+       struct timespec *bt;
+
+       bt = &basetime[basetime_index];
+       return(gd->gd_time_seconds + bt->tv_sec);
 }
 
 int
@@ -976,6 +1079,7 @@ pps_event(struct pps_state *pps, sysclock_t count, int event)
        struct globaldata *gd;
        struct timespec *tsp;
        struct timespec *osp;
+       struct timespec *bt;
        struct timespec ts;
        sysclock_t *pcount;
 #ifdef PPS_SYNC
@@ -1021,8 +1125,9 @@ pps_event(struct pps_state *pps, sysclock_t count, int event)
                delta %= cputimer_freq;
        }
        ts.tv_nsec = (cputimer_freq64_nsec * delta) >> 32;
-       ts.tv_sec += basetime.tv_sec;
-       ts.tv_nsec += basetime.tv_nsec;
+       bt = &basetime[basetime_index];
+       ts.tv_sec += bt->tv_sec;
+       ts.tv_nsec += bt->tv_nsec;
        while (ts.tv_nsec >= 1000000000) {
                ts.tv_nsec -= 1000000000;
                ++ts.tv_sec;
index 9cc5843..1db7052 100644 (file)
@@ -29,7 +29,7 @@
  * confusing and/or plain wrong in that context.
  *
  * $FreeBSD: src/sys/kern/kern_ntptime.c,v 1.32.2.2 2001/04/22 11:19:46 jhay Exp $
- * $DragonFly: src/sys/kern/kern_ntptime.c,v 1.10 2005/04/20 16:37:09 cpressey Exp $
+ * $DragonFly: src/sys/kern/kern_ntptime.c,v 1.11 2005/04/23 20:34:32 dillon Exp $
  */
 
 #include "opt_ntp.h"
@@ -423,6 +423,8 @@ ntp_adjtime(struct ntp_adjtime_args *uap)
  * This routine is ordinarily called from hardclock() whenever the seconds
  * hand rolls over.  It returns leap seconds to add or drop, and sets nsec_adj
  * to the total adjustment to make over the next second in (ns << 32).
+ *
+ * This routine is only called by cpu #0.
  */
 int
 ntp_update_second(time_t newsec, int64_t *nsec_adj)
index dc4b810..f64e5fd 100644 (file)
@@ -32,7 +32,7 @@
  *
  *     @(#)kern_time.c 8.1 (Berkeley) 6/10/93
  * $FreeBSD: src/sys/kern/kern_time.c,v 1.68.2.1 2002/10/01 08:00:41 bde Exp $
- * $DragonFly: src/sys/kern/kern_time.c,v 1.26 2005/04/23 19:59:05 joerg Exp $
+ * $DragonFly: src/sys/kern/kern_time.c,v 1.27 2005/04/23 20:34:32 dillon Exp $
  */
 
 #include <sys/param.h>
@@ -560,10 +560,6 @@ sysctl_adjtime(SYSCTL_HANDLER_ARGS)
        int64_t delta;
        int error;
 
-       delta = 0;
-       error = SYSCTL_OUT(req, &delta, sizeof(delta));
-       if (error)
-               return (error);
        if (req->newptr != NULL) {
                if (suser(curthread))
                        return (EPERM);
@@ -572,7 +568,11 @@ sysctl_adjtime(SYSCTL_HANDLER_ARGS)
                        return (error);
                kern_reladjtime(delta);
        }
-       return (0);
+
+       if (req->oldptr)
+               kern_get_ntp_delta(&delta);
+       error = SYSCTL_OUT(req, &delta, sizeof(delta));
+       return (error);
 }
 
 static int
@@ -588,16 +588,12 @@ sysctl_delta(SYSCTL_HANDLER_ARGS)
                if (error)
                        return (error);
                kern_adjtime(delta, &old_delta);
-               /* Fall through for writing old_delta */
-       } else if (req->oldptr != NULL) {
-               kern_get_ntp_delta(&old_delta);
        }
 
+       if (req->oldptr != NULL)
+               kern_get_ntp_delta(&old_delta);
        error = SYSCTL_OUT(req, &old_delta, sizeof(old_delta));
-       if (error)
-               return (error);
-
-       return (0);
+       return (error);
 }
 
 static int
@@ -606,10 +602,6 @@ sysctl_adjfreq(SYSCTL_HANDLER_ARGS)
        int64_t freqdelta;
        int error;
 
-       freqdelta = ntp_tick_permanent * hz;
-       error = SYSCTL_OUT(req, &freqdelta, sizeof(freqdelta));
-       if (error)
-               return (error);
        if (req->newptr != NULL) {
                if (suser(curthread))
                        return (EPERM);
@@ -620,23 +612,35 @@ sysctl_adjfreq(SYSCTL_HANDLER_ARGS)
                freqdelta /= hz;
                kern_adjfreq(freqdelta);
        }
+
+       if (req->oldptr != NULL)
+               freqdelta = ntp_tick_permanent * hz;
+       error = SYSCTL_OUT(req, &freqdelta, sizeof(freqdelta));
+       if (error)
+               return (error);
+
        return (0);
 }
 
 SYSCTL_NODE(_kern, OID_AUTO, ntp, CTLFLAG_RW, 0, "NTP related controls");
-SYSCTL_PROC(_kern_ntp, OID_AUTO, permanent, CTLTYPE_QUAD|CTLFLAG_RW, 0, 0,
+SYSCTL_PROC(_kern_ntp, OID_AUTO, permanent,
+    CTLTYPE_QUAD|CTLFLAG_RW, 0, 0,
     sysctl_adjfreq, "Q", "permanent correction per second");
 SYSCTL_PROC(_kern_ntp, OID_AUTO, delta,
-    CTLTYPE_QUAD | CTLFLAG_RW, 0, 0,
+    CTLTYPE_QUAD|CTLFLAG_RW, 0, 0,
     sysctl_delta, "Q", "one-time delta");
-SYSCTL_QUAD(_kern_ntp, OID_AUTO, big_delta, CTLFLAG_RD,
-    &ntp_big_delta, 0, "threshold for fast adjustment");
-SYSCTL_QUAD(_kern_ntp, OID_AUTO, tick_delta, CTLFLAG_RD,
-    &ntp_tick_delta, 0, "per-tick adjustment");
-SYSCTL_QUAD(_kern_ntp, OID_AUTO, default_tick_delta, CTLFLAG_RD,
-    &ntp_default_tick_delta, 0, "default per-tick adjustment");
+SYSCTL_OPAQUE(_kern_ntp, OID_AUTO, big_delta, CTLFLAG_RD,
+    &ntp_big_delta, sizeof(ntp_big_delta), "Q",
+    "threshold for fast adjustment");
+SYSCTL_OPAQUE(_kern_ntp, OID_AUTO, tick_delta, CTLFLAG_RD,
+    &ntp_tick_delta, sizeof(ntp_tick_delta), "LU",
+    "per-tick adjustment");
+SYSCTL_OPAQUE(_kern_ntp, OID_AUTO, default_tick_delta, CTLFLAG_RD,
+    &ntp_default_tick_delta, sizeof(ntp_default_tick_delta), "LU",
+    "default per-tick adjustment");
 SYSCTL_OPAQUE(_kern_ntp, OID_AUTO, next_leap_second, CTLFLAG_RW,
-    &ntp_leap_second, sizeof(time_t), "T,time_t", "next leap second");
+    &ntp_leap_second, sizeof(ntp_leap_second), "LU",
+    "next leap second");
 SYSCTL_INT(_kern_ntp, OID_AUTO, insert_leap_second, CTLFLAG_RW,
     &ntp_leap_insert, 0, "insert or remove leap second");
 SYSCTL_PROC(_kern_ntp, OID_AUTO, adjust,