From c4c80fb4eaf22b7aab6a9fd9d77f096a380f3e39 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sun, 11 Apr 2021 15:00:34 +0200 Subject: [PATCH] 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. --- include/haproxy/time.h | 2 ++ src/haproxy.c | 4 +-- src/time.c | 63 +++++++++++++++++++++++++++++------------- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/include/haproxy/time.h b/include/haproxy/time.h index f5c0da692..0ce06decc 100644 --- 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 3d5e85f16..27ac49e90 100644 --- 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 9901dab10..0855c8cc0 100644 --- 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 , and the return value in (a non-zero - * value means that we have not expired the timeout). Calling it with (-1,*) - * sets both and 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