timeval: Recover from failed timer_create() calls.

The timer_create() system call is not supported in ESX and returns
an error when called.  Aborting when this system call fails seems a
bit extreme. So instead, this patch simply falls back to disabling
the cached time optimization.

Signed-off-by: Ethan Jackson <ethan@nicira.com>

diff --git a/lib/timeval.c b/lib/timeval.c
index 835b91f7b7270fd8c1644058ed070c182b4088a6..77247e80ac71583c9690522bf7e9153bfdfc7d26 100644 (file)
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -40,7 +40,15 @@ VLOG_DEFINE_THIS_MODULE(timeval);
  * to CLOCK_REALTIME. */
 static clockid_t monotonic_clock;
 
-/* Has a timer tick occurred? Only relevant if CACHE_TIME is 1.
+/* Controls whether or not calls to clock_gettime() are cached.  See
+ * time_cached() for a detailed explanation. */
+#if defined __x86_64__ && defined __linux__
+static bool cache_time = false;
+#else
+static bool cache_time = true;
+#endif
+
+/* Has a timer tick occurred? Only relevant if cache_time is true.
  *
  * We initialize these to true to force time_init() to get called on the first
  * call to time_msec() or another function that queries the current time. */
@@ -93,10 +101,8 @@ time_init(void)
         VLOG_DBG("monotonic timer not available");
     }
 
-    if (CACHE_TIME) {
-        set_up_signal(SA_RESTART);
-        set_up_timer();
-    }
+    set_up_signal(SA_RESTART);
+    set_up_timer();
 
     boot_time = time_msec();
 }
@@ -148,8 +154,15 @@ set_up_timer(void)
     static timer_t timer_id;    /* "static" to avoid apparent memory leak. */
     struct itimerspec itimer;
 
+    if (!cache_time) {
+        return;
+    }
+
     if (timer_create(monotonic_clock, NULL, &timer_id)) {
-        VLOG_FATAL("timer_create failed (%s)", strerror(errno));
+        VLOG_WARN("timer_create failed (%s), disabling cached timing",
+                  strerror(errno));
+        cache_time = false;
+        return;
     }
 
     itimer.it_interval.tv_sec = 0;
@@ -170,10 +183,7 @@ void
 time_postfork(void)
 {
     time_init();
-
-    if (CACHE_TIME) {
-        set_up_timer();
-    }
+    set_up_timer();
 }
 
 static void
@@ -205,7 +215,7 @@ refresh_monotonic(void)
 /* Forces a refresh of the current time from the kernel.  It is not usually
  * necessary to call this function, since the time will be refreshed
  * automatically at least every TIME_UPDATE_INTERVAL milliseconds.  If
- * CACHE_TIME is 0, we will always refresh the current time so this
+ * cache_time is false, we will always refresh the current time so this
  * function has no effect. */
 void
 time_refresh(void)
@@ -346,7 +356,7 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long int timeout_when,
             break;
         }
 
-        if (!blocked && !CACHE_TIME) {
+        if (!blocked && !cache_time) {
             block_sigalrm(&oldsigs);
             blocked = true;
         }
@@ -360,6 +370,23 @@ time_poll(struct pollfd *pollfds, int n_pollfds, long long int timeout_when,
     return retval;
 }
 
+/* True on systems (particularly x86-64 Linux) where clock_gettime() is
+ * inexpensive.  On these systems, we don't bother caching the current time.
+ * Instead, we consult clock_gettime() directly when needed.
+ *
+ * False on systems where clock_gettime() is relatively expensive.  On these
+ * systems, we cache the current time and set up a periodic SIGALRM to remind
+ * us to update it.
+ *
+ * Also false on systems (e.g. ESX) that don't support setting up timers based
+ * on a monotonically increasing clock. */
+bool
+time_cached(void)
+{
+    time_init();
+    return cache_time;
+}
+
 static void
 sigalrm_handler(int sig_nr OVS_UNUSED)
 {
@@ -370,7 +397,7 @@ sigalrm_handler(int sig_nr OVS_UNUSED)
 static void
 refresh_wall_if_ticked(void)
 {
-    if (!CACHE_TIME || wall_tick) {
+    if (!cache_time || wall_tick) {
         refresh_wall();
     }
 }
@@ -378,7 +405,7 @@ refresh_wall_if_ticked(void)
 static void
 refresh_monotonic_if_ticked(void)
 {
-    if (!CACHE_TIME || monotonic_tick) {
+    if (!cache_time || monotonic_tick) {
         refresh_monotonic();
     }
 }

diff --git a/lib/timeval.h b/lib/timeval.h
index 1384848a1cec8f9b19087ce3c83413dd64e651a9..d9eb3c7a61be526f245f08821577514e08a2aa46 100644 (file)
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -36,21 +36,6 @@ BUILD_ASSERT_DECL(TYPE_IS_INTEGER(time_t));
  * ever encounter such a platform. */
 BUILD_ASSERT_DECL(TYPE_IS_SIGNED(time_t));
 
-/* On x86-64 systems, Linux avoids using syscalls for clock_gettime().
- *
- * For systems which do invoke a system call we wait at least
- * TIME_UPDATE_INTERVAL ms between clock_gettime() calls and cache the time for
- * the interim.
- *
- * For systems which do not invoke a system call, we just call clock_gettime()
- * whenever the time is requested.  As a result we don't start the background
- * SIGALRM timer unless explicitly needed by time_alarm() */
-#if defined __x86_64__ && defined __linux__
-#define CACHE_TIME 0
-#else
-#define CACHE_TIME 1
-#endif
-
 #define TIME_MAX TYPE_MAXIMUM(time_t)
 #define TIME_MIN TYPE_MINIMUM(time_t)
 
@@ -72,6 +57,7 @@ void time_wall_timespec(struct timespec *);
 void time_alarm(unsigned int secs);
 int time_poll(struct pollfd *, int n_pollfds, long long int timeout_when,
               int *elapsed);
+bool time_cached(void);
 
 long long int timespec_to_msec(const struct timespec *);
 long long int timeval_to_msec(const struct timeval *);

diff --git a/tests/test-timeval.c b/tests/test-timeval.c
index 9896cf7372ff880b8af487c3819e47ee69c36c7b..a58c04169bad54dc53f85d7810ce5d62524aeec5 100644 (file)
--- a/tests/test-timeval.c
+++ b/tests/test-timeval.c
@@ -99,7 +99,7 @@ main(int argc, char *argv[])
     } else if (!strcmp(argv[1], "plain")) {
         /* If we're not caching time there isn't much to test and SIGALRM won't
          * be around to pull us out of the select() call, so just skip out */
-        if (!CACHE_TIME) {
+        if (!time_cached()) {
             exit (77);
         }
 
@@ -110,7 +110,7 @@ main(int argc, char *argv[])
         char cwd[1024], *pidfile;
         FILE *success;
 
-        if (!CACHE_TIME) {
+        if (!time_cached()) {
             exit (77);
         }