MINOR: time: move the time initialization out of tv_update_date()

The time initialization was made a bit complex because we rely on a
dummy negative argument to reset all fields, leaving no distinction
between process-level initialization and thread-level initialization.
This patch changes this by introducing two functions, one for the
process and the second one for the threads. This removes ambigous
test and makes sure that the relevant fields are always initialized
exactly once. This also offers a better solution to the bug fixed in
commit b48e7c001 ("BUG/MEDIUM: time: make sure to always initialize
the global tick") as there is no more special values for global_now_ms.

It's simple enough to be backported if any other time-related issues
are encountered in stable versions in the future.

diff --git a/include/haproxy/time.h b/include/haproxy/time.h
index f5c0da6926aea283326331af3ce3e3f56c319356..0ce06decc0e335c9d2057464b398f5ce4f27ddfa 100644 (file)
--- a/include/haproxy/time.h
+++ b/include/haproxy/time.h
@@ -88,6 +88,8 @@ int tv_ms_cmp2(const struct timeval *tv1, const struct timeval *tv2);
  * timeout).
  */
 void tv_update_date(int max_wait, int interrupted);
+void tv_init_process_date();
+void tv_init_thread_date();
 
 char *timeofday_as_iso_us(int pad);
 

diff --git a/src/haproxy.c b/src/haproxy.c
index 3d5e85f168ec6b0230de01de4d9567c3e5754c8f..27ac49e90033c61bed36e44e4c507faf4de44eee 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1311,7 +1311,7 @@ static void init(int argc, char **argv)
 #endif
 
        tzset();
-       tv_update_date(-1,-1);
+       tv_init_process_date();
        start_date = now;
 
        ha_random_boot(argv);
@@ -2457,7 +2457,7 @@ static void *run_thread_poll_loop(void *data)
                init_left = global.nbthread;
        init_left--;
 
-       tv_update_date(-1,-1);
+       tv_init_thread_date();
 
        /* per-thread alloc calls performed here are not allowed to snoop on
         * other threads, so they are free to initialize at their own rhythm

diff --git a/src/time.c b/src/time.c
index 9901dab103d6086d31de612aff672c5f94b607a8..0855c8cc030d04edc4ea347eec25a96ef6b4cfc1 100644 (file)
--- a/src/time.c
+++ b/src/time.c
@@ -163,9 +163,10 @@ int _tv_isgt(const struct timeval *tv1, const struct timeval *tv2)
  * an offset to correct them. This function should be called once after each
  * poll, and never farther apart than MAX_DELAY_MS*2. The poll's timeout should
  * be passed in <max_wait>, and the return value in <interrupted> (a non-zero
- * value means that we have not expired the timeout). Calling it with (-1,*)
- * sets both <date> and <now> to current date, and calling it with (0,1) simply
- * updates the values.
+ * value means that we have not expired the timeout).
+ *
+ * tv_init_process_date() must have been called once first, and
+ * tv_init_thread_date() must also have been called once for each thread.
  *
  * An offset is used to adjust the current time (date), to have a monotonic time
  * (now). It must be global and thread-safe. But a timeval cannot be atomically
@@ -182,20 +183,6 @@ void tv_update_date(int max_wait, int interrupted)
        unsigned long long new_now;
 
        gettimeofday(&date, NULL);
-       if (unlikely(max_wait < 0)) {
-               tv_zero(&tv_offset);
-               adjusted = date;
-               after_poll = date;
-               samp_time = idle_time = 0;
-               ti->idle_pct = 100;
-               old_now = global_now;
-               if (!old_now) { // never set
-                       new_now = (((unsigned long long)adjusted.tv_sec) << 32) + (unsigned int)adjusted.tv_usec;
-                       _HA_ATOMIC_CAS(&global_now, &old_now, new_now);
-               }
-               goto to_ms;
-       }
-
        __tv_add(&adjusted, &date, &tv_offset);
 
        /* compute the minimum and maximum local date we may have reached based
@@ -246,7 +233,6 @@ void tv_update_date(int max_wait, int interrupted)
                tv_offset.tv_sec--;
        }
 
- to_ms:
        now = adjusted;
        now_ms = now.tv_sec * 1000 + now.tv_usec / 1000;
 
@@ -254,13 +240,52 @@ void tv_update_date(int max_wait, int interrupted)
        old_now_ms = global_now_ms;
        do {
                new_now_ms = old_now_ms;
-               if (tick_is_lt(new_now_ms, now_ms) || !new_now_ms)
+               if (tick_is_lt(new_now_ms, now_ms))
                        new_now_ms = now_ms;
        }  while (!_HA_ATOMIC_CAS(&global_now_ms, &old_now_ms, new_now_ms));
 
        return;
 }
 
+/* must be called once at boot to initialize some global variables */
+void tv_init_process_date()
+{
+       tv_zero(&tv_offset);
+       gettimeofday(&date, NULL);
+       now = after_poll = before_poll = date;
+       global_now = ((ullong)date.tv_sec << 32) + (uint)date.tv_usec;
+       global_now_ms = now.tv_sec * 1000 + now.tv_usec / 1000;
+       samp_time = idle_time = 0;
+       ti->idle_pct = 100;
+       tv_update_date(0, 1);
+}
+
+/* must be called once per thread to initialize their thread-local variables.
+ * Note that other threads might also be initializing and running in parallel.
+ */
+void tv_init_thread_date()
+{
+       ullong old_now;
+
+       gettimeofday(&date, NULL);
+       after_poll = before_poll = date;
+
+       old_now = _HA_ATOMIC_LOAD(&global_now);
+       now.tv_sec = old_now >> 32;
+       now.tv_usec = (uint)old_now;
+
+       tv_offset.tv_sec  = now.tv_sec  - date.tv_sec;
+       tv_offset.tv_usec = now.tv_usec - date.tv_usec;
+       if (tv_offset.tv_usec < 0) {
+               tv_offset.tv_usec += 1000000;
+               tv_offset.tv_sec--;
+       }
+
+       samp_time = idle_time = 0;
+       ti->idle_pct = 100;
+       tv_update_date(0, 1);
+}
+
 /* returns the current date as returned by gettimeofday() in ISO+microsecond
  * format. It uses a thread-local static variable that the reader can consume
  * for as long as it wants until next call. Thus, do not call it from a signal