[ovs-dev] [PATCH 2/2] timeval: On Linux x86-64 systems refresh time whenever it is requested.

[Leo Alterman]

64-bit Linux appears to avoid syscalls for clock_gettime(), so we can get
higher resolution timing and avoid having a timer firing off SIGALRM
without introducing extra overhead.

Signed-off-by: Leo Alterman <lalterman at nicira.com>
---
 lib/timeval.c        |   36 +++++++++++++++++++++++++++++-------
 lib/timeval.h        |   15 +++++++++++++++
 tests/test-timeval.c |   10 ++++++++++
 3 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/lib/timeval.c b/lib/timeval.c
index d29b661..3d339e4 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -40,7 +40,7 @@ VLOG_DEFINE_THIS_MODULE(timeval);
  * to CLOCK_REALTIME. */
 static clockid_t monotonic_clock;

-/* Has a timer tick occurred?
+/* Has a timer tick occurred? Only relevant if CACHE_TIME is 1.
  *
  * 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. */
@@ -94,8 +94,11 @@ time_init(void)
         VLOG_DBG("monotonic timer not available");
     }

-    set_up_signal(SA_RESTART);
-    set_up_timer();
+    if (CACHE_TIME) {
+        set_up_signal(SA_RESTART);
+        set_up_timer();
+    }
+
     boot_time = time_msec();
 }

@@ -168,7 +171,16 @@ void
 time_postfork(void)
 {
     time_init();
-    set_up_timer();
+
+    if (CACHE_TIME) {
+        set_up_timer();
+    } else {
+        /* If we are not caching  kernel time, the only reason the timer should
+         * exist is if time_alarm() was called and deadline is set */
+        if (deadline != TIME_MIN) {
+            set_up_timer();
+        }
+    }
 }

 static void
@@ -199,7 +211,9 @@ 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. */
+ * automatically at least every TIME_UPDATE_INTERVAL milliseconds.  If
+ * CACHE_TIME is 0, we will always refresh the current time so this
+ * function has no effect. */
 void
 time_refresh(void)
 {
@@ -275,9 +289,17 @@ time_alarm(unsigned int secs)
     sigset_t oldsigs;

     time_init();
+
     block_sigalrm(&oldsigs);
     deadline = secs ? time_add(time_now(), secs) : TIME_MIN;
     unblock_sigalrm(&oldsigs);
+
+    if (!CACHE_TIME) {
+        /* If we aren't timing the gaps between kernel time refreshes we need to
+         * to start the timer up now */
+        set_up_signal(SA_RESTART);
+        set_up_timer();
+    }
 }

 /* Like poll(), except:
@@ -366,7 +388,7 @@ sigalrm_handler(int sig_nr)
 static void
 refresh_wall_if_ticked(void)
 {
-    if (wall_tick) {
+    if (!CACHE_TIME || wall_tick) {
         refresh_wall();
     }
 }
@@ -374,7 +396,7 @@ refresh_wall_if_ticked(void)
 static void
 refresh_monotonic_if_ticked(void)
 {
-    if (monotonic_tick) {
+    if (!CACHE_TIME || monotonic_tick) {
         refresh_monotonic();
     }
 }
diff --git a/lib/timeval.h b/lib/timeval.h
index e8413ff..9cfcded 100644
--- a/lib/timeval.h
+++ b/lib/timeval.h
@@ -36,6 +36,21 @@ 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 __LP64__ && 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)

diff --git a/tests/test-timeval.c b/tests/test-timeval.c
index d277fc9..9896cf7 100644
--- a/tests/test-timeval.c
+++ b/tests/test-timeval.c
@@ -97,6 +97,12 @@ main(int argc, char *argv[])
     if (argc != 2) {
         usage();
     } 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) {
+            exit (77);
+        }
+
         do_test();
     } else if (!strcmp(argv[1], "daemon")) {
         /* Test that time still advances even in a daemon.  This is an
@@ -104,6 +110,10 @@ main(int argc, char *argv[])
         char cwd[1024], *pidfile;
         FILE *success;

+        if (!CACHE_TIME) {
+            exit (77);
+        }
+
         assert(getcwd(cwd, sizeof cwd) == cwd);

         unlink("test-timeval.success");
--
1.7.9.5