[ovs-dev] [PATCH 2/2] timeval: Recover from failed timer_create() calls.

Ethan Jackson ethan at nicira.com
Fri Oct 5 18:16:30 UTC 2012


Thanks for the review, here's an incremental.

---
 lib/timeval.c        | 11 ++++++-----
 lib/timeval.h        | 27 ++++++++++++---------------
 tests/test-timeval.c |  4 ++--
 3 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/lib/timeval.c b/lib/timeval.c
index 1fa099d..b2e634d 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -40,10 +40,11 @@ VLOG_DEFINE_THIS_MODULE(timeval);
  * to CLOCK_REALTIME. */
 static clockid_t monotonic_clock;
 
-/* Has the same meaning as the CACHE_TIME define.  On some systems
- * timer_create() is not implemented, in which case this variable can be
- * overwridden. */
-static bool cache_time = CACHE_TIME;
+#if defined __x86_64__ && defined __linux__
+bool cache_time = false;
+#else
+bool cache_time = true;
+#endif
 
 /* Has a timer tick occurred? Only relevant if cache_time is true.
  *
@@ -353,7 +354,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;
         }
diff --git a/lib/timeval.h b/lib/timeval.h
index 1384848..795b3ac 100644
--- 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)
 
@@ -59,6 +44,18 @@ BUILD_ASSERT_DECL(TYPE_IS_SIGNED(time_t));
  * much time will be wasted in signal handlers and calls to clock_gettime(). */
 #define TIME_UPDATE_INTERVAL 100
 
+/* 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. */
+extern bool cache_time;
+
 void time_disable_restart(void);
 void time_enable_restart(void);
 void time_postfork(void);
diff --git a/tests/test-timeval.c b/tests/test-timeval.c
index 9896cf7..61b4e8a 100644
--- 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 (!cache_time) {
             exit (77);
         }
 
@@ -110,7 +110,7 @@ main(int argc, char *argv[])
         char cwd[1024], *pidfile;
         FILE *success;
 
-        if (!CACHE_TIME) {
+        if (!cache_time) {
             exit (77);
         }
 
-- 
1.7.12




More information about the dev mailing list