<div dir="ltr"><div>Hi Bart,</div><div> </div>This patch seems to depend on another patch flagging tur.c some function static.<div>It seems I missed this patch. Is it tanked in your tree, and you want to post it for inclusion ? Or do you prefer to rebase this "clock jump" patch ?</div><div> </div><div>Best regards,</div><div>Christophe Varoqui</div><div>OpenSVC</div></div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Sep 29, 2016 at 11:29 PM, Bart Van Assche <<a href="mailto:bart.vanassche@sandisk.com" target="_blank">bart.vanassche@sandisk.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Time synchronization software like ntpd can adjust the clock forwards and backwards. Avoid that the duration of a delay is influenced by clock jumps initiated by time synchronization software. Signed-off-by: Bart Van Assche <<a href="mailto:bart.vanassche@sandisk.com">bart.vanassche@sandisk.com</a>> Cc: Mike Christie <<a href="mailto:mchristi@redhat.com">mchristi@redhat.com</a>> --- libmultipath/Makefile | 2 +- libmultipath/checkers/rbd.c | 12 +++++------- libmultipath/checkers/tur.c | 20 +++++++++----------- libmultipath/time-util.c | 42 ++++++++++++++++++++++++++++++++++++++++++ libmultipath/time-util.h | 13 +++++++++++++ multipathd/cli.c | 6 ++---- multipathd/main.c | 39 +++++++++++++++++++++++---------------- multipathd/uxlsnr.c | 19 +++++++++++-------- 8 files changed, 106 insertions(+), 47 deletions(-) create mode 100644 libmultipath/time-util.c create mode 100644 libmultipath/time-util.h diff --git a/libmultipath/Makefile b/libmultipath/Makefile index 92f130c..495cebe 100644 --- a/libmultipath/Makefile +++ b/libmultipath/Makefile @@ -47,7 +47,7 @@ endif OBJS = memory.o parser.o vector.o devmapper.o callout.o \ hwtable.o blacklist.o util.o dmparser.o config.o \ structs.o discovery.o propsel.o dict.o \ - pgpolicies.o debug.o defaults.o uevent.o \ + pgpolicies.o debug.o defaults.o uevent.o time-util.o \ switchgroup.o uxsock.o print.o alias.o log_pthread.o \ log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \ lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c index 41259c3..bfec2c9 100644 --- a/libmultipath/checkers/rbd.c +++ b/libmultipath/checkers/rbd.c @@ -27,6 +27,7 @@ #include "../libmultipath/debug.h" #include "../libmultipath/uevent.h" +#include "../libmultipath/time-util.h" struct rbd_checker_context; typedef int (thread_fn)(struct rbd_checker_context *ct, char *msg); @@ -75,7 +76,7 @@ int libcheck_init(struct checker * c) return 1; memset(ct, 0, sizeof(struct rbd_checker_context)); ct->holders = 1; - pthread_cond_init(&ct->active, NULL); + pthread_cond_init_mono(&ct->active); pthread_mutex_init(&ct->lock, NULL); pthread_spin_init(&ct->hldr_lock, PTHREAD_PROCESS_PRIVATE); c->context = ct; @@ -538,12 +539,9 @@ static void *rbd_thread(void *ctx) static void rbd_timeout(struct timespec *tsp) { - struct timeval now; - - gettimeofday(&now, NULL); - tsp->tv_sec = now.tv_sec; - tsp->tv_nsec = now.tv_usec * 1000; - tsp->tv_nsec += 1000000; /* 1 millisecond */ + clock_gettime(CLOCK_MONOTONIC, tsp); + tsp->tv_nsec += 1000 * 1000; /* 1 millisecond */ + normalize_timespec(tsp); } static int rbd_exec_fn(struct checker *c, thread_fn *fn) diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c index 94e4190..ac272d4 100644 --- a/libmultipath/checkers/tur.c +++ b/libmultipath/checkers/tur.c @@ -20,6 +20,7 @@ #include "../libmultipath/debug.h" #include "../libmultipath/sg_include.h" #include "../libmultipath/uevent.h" +#include "../libmultipath/time-util.h" #define TUR_CMD_LEN 6 #define HEAVY_CHECK_COUNT 10 @@ -60,7 +61,7 @@ int libcheck_init (struct checker * c) ct->state = PATH_UNCHECKED; ct->fd = -1; ct->holders = 1; - pthread_cond_init(&ct->active, NULL); + pthread_cond_init_mono(&ct->active); pthread_mutex_init(&ct->lock, NULL); pthread_spin_init(&ct->hldr_lock, PTHREAD_PROCESS_PRIVATE); c->context = ct; @@ -237,29 +238,26 @@ static void *tur_thread(void *ctx) static void tur_timeout(struct timespec *tsp) { - struct timeval now; - - gettimeofday(&now, NULL); - tsp->tv_sec = now.tv_sec; - tsp->tv_nsec = now.tv_usec * 1000; - tsp->tv_nsec += 1000000; /* 1 millisecond */ + clock_gettime(CLOCK_MONOTONIC, tsp); + tsp->tv_nsec += 1000 * 1000; /* 1 millisecond */ + normalize_timespec(tsp); } static void tur_set_async_timeout(struct checker *c) { struct tur_checker_context *ct = c->context; - struct timeval now; + struct timespec now; - gettimeofday(&now, NULL); + clock_gettime(CLOCK_MONOTONIC, &now); ct->time = now.tv_sec + c->timeout; } static int tur_check_async_timeout(struct checker *c) { struct tur_checker_context *ct = c->context; - struct timeval now; + struct timespec now; - gettimeofday(&now, NULL); + clock_gettime(CLOCK_MONOTONIC, &now); return (now.tv_sec > ct->time); } diff --git a/libmultipath/time-util.c b/libmultipath/time-util.c new file mode 100644 index 0000000..6d79c0e --- /dev/null +++ b/libmultipath/time-util.c @@ -0,0 +1,42 @@ +#include <assert.h> +#include <pthread.h> +#include <time.h> +#include "time-util.h" + +/* Initialize @cond as a condition variable that uses the monotonic clock */ +void pthread_cond_init_mono(pthread_cond_t *cond) +{ + pthread_condattr_t attr; + int res; + + res = pthread_condattr_init(&attr); + assert(res == 0); + res = pthread_condattr_setclock(&attr, CLOCK_MONOTONIC); + assert(res == 0); + res = pthread_cond_init(cond, &attr); + assert(res == 0); + res = pthread_condattr_destroy(&attr); + assert(res == 0); +} + +/* Ensure that 0 <= ts->tv_nsec && ts->tv_nsec < 1000 * 1000 * 1000. */ +void normalize_timespec(struct timespec *ts) +{ + while (ts->tv_nsec < 0) { + ts->tv_nsec += 1000UL * 1000 * 1000; + ts->tv_sec--; + } + while (ts->tv_nsec >= 1000UL * 1000 * 1000) { + ts->tv_nsec -= 1000UL * 1000 * 1000; + ts->tv_sec++; + } +} + +/* Compute *res = *a - *b */ +void timespecsub(const struct timespec *a, const struct timespec *b, + struct timespec *res) +{ + res->tv_sec = a->tv_sec - b->tv_sec; + res->tv_nsec = a->tv_nsec - b->tv_nsec; + normalize_timespec(res); +} diff --git a/libmultipath/time-util.h b/libmultipath/time-util.h new file mode 100644 index 0000000..b76d2aa --- /dev/null +++ b/libmultipath/time-util.h @@ -0,0 +1,13 @@ +#ifndef _TIME_UTIL_H_ +#define _TIME_UTIL_H_ + +#include <pthread.h> + +struct timespec; + +void pthread_cond_init_mono(pthread_cond_t *cond); +void normalize_timespec(struct timespec *ts); +void timespecsub(const struct timespec *a, const struct timespec *b, + struct timespec *res); + +#endif /* _TIME_UTIL_H_ */ diff --git a/multipathd/cli.c b/multipathd/cli.c index 9a19728..e8a9384 100644 --- a/multipathd/cli.c +++ b/multipathd/cli.c @@ -454,7 +454,6 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) struct handler * h; vector cmdvec = NULL; struct timespec tmo; - struct timeval now; r = get_cmdvec(cmd, &cmdvec); @@ -476,9 +475,8 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) /* * execute handler */ - if (gettimeofday(&now, NULL) == 0) { - tmo.tv_sec = now.tv_sec + timeout; - tmo.tv_nsec = now.tv_usec * 1000; + if (clock_gettime(CLOCK_MONOTONIC, &tmo) == 0) { + tmo.tv_sec += timeout; } else { tmo.tv_sec = 0; } diff --git a/multipathd/main.c b/multipathd/main.c index 96ef01f..76c049f 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -25,6 +25,11 @@ #include <time.h> /* + * libmultipath + */ +#include "time-util.h" + +/* * libcheckers */ #include "checkers.h" @@ -106,7 +111,7 @@ int ignore_new_devs; enum daemon_status running_state = DAEMON_INIT; pid_t daemon_pid; pthread_mutex_t config_lock = PTHREAD_MUTEX_INITIALIZER; -pthread_cond_t config_cond = PTHREAD_COND_INITIALIZER; +pthread_cond_t config_cond; /* * global copy of vecs for use in sig handlers @@ -193,7 +198,7 @@ int set_config_state(enum daemon_status state) if (running_state != DAEMON_IDLE) { struct timespec ts; - clock_gettime(CLOCK_REALTIME, &ts); + clock_gettime(CLOCK_MONOTONIC, &ts); ts.tv_sec += 1; rc = pthread_cond_timedwait(&config_cond, &config_lock, &ts); @@ -1744,7 +1749,7 @@ checkerloop (void *ap) int count = 0; unsigned int i; struct itimerval timer_tick_it; - struct timeval last_time; + struct timespec last_time; struct config *conf; pthread_cleanup_push(rcu_unregister, NULL); @@ -1763,24 +1768,23 @@ checkerloop (void *ap) } /* Tweak start time for initial path check */ - if (gettimeofday(&last_time, NULL) != 0) + if (clock_gettime(CLOCK_MONOTONIC, &last_time) != 0) last_time.tv_sec = 0; else last_time.tv_sec -= 1; while (1) { - struct timeval diff_time, start_time, end_time; + struct timespec diff_time, start_time, end_time; int num_paths = 0, ticks = 0, signo, strict_timing, rc = 0; sigset_t mask; - if (gettimeofday(&start_time, NULL) != 0) + if (clock_gettime(CLOCK_MONOTONIC, &start_time) != 0) start_time.tv_sec = 0; if (start_time.tv_sec && last_time.tv_sec) { - timersub(&start_time, &last_time, &diff_time); + timespecsub(&start_time, &last_time, &diff_time); condlog(4, "tick (%lu.%06lu secs)", - diff_time.tv_sec, diff_time.tv_usec); - last_time.tv_sec = start_time.tv_sec; - last_time.tv_usec = start_time.tv_usec; + diff_time.tv_sec, diff_time.tv_nsec / 1000); + last_time = start_time; ticks = diff_time.tv_sec; } else { ticks = 1; @@ -1831,16 +1835,17 @@ checkerloop (void *ap) lock_cleanup_pop(vecs->lock); } - diff_time.tv_usec = 0; + diff_time.tv_nsec = 0; if (start_time.tv_sec && - gettimeofday(&end_time, NULL) == 0) { - timersub(&end_time, &start_time, &diff_time); + clock_gettime(CLOCK_MONOTONIC, &end_time) == 0) { + timespecsub(&end_time, &start_time, &diff_time); if (num_paths) { unsigned int max_checkint; condlog(3, "checked %d path%s in %lu.%06lu secs", num_paths, num_paths > 1 ? "s" : "", - diff_time.tv_sec, diff_time.tv_usec); + diff_time.tv_sec, + diff_time.tv_nsec / 1000); conf = get_multipath_config(); max_checkint = conf->max_checkint; put_multipath_config(conf); @@ -1861,10 +1866,10 @@ checkerloop (void *ap) else { timer_tick_it.it_interval.tv_sec = 0; timer_tick_it.it_interval.tv_usec = 0; - if (diff_time.tv_usec) { + if (diff_time.tv_nsec) { timer_tick_it.it_value.tv_sec = 0; timer_tick_it.it_value.tv_usec = - (unsigned long)1000000 - diff_time.tv_usec; + 1000ULL * 1000 * 1000 - diff_time.tv_nsec; } else { timer_tick_it.it_value.tv_sec = 1; timer_tick_it.it_value.tv_usec = 0; @@ -2523,6 +2528,8 @@ main (int argc, char *argv[]) strerror(errno)); umask(umask(077) | 022); + pthread_cond_init_mono(&config_cond); + udev = udev_new(); while ((arg = getopt(argc, argv, ":dsv:k::Bn")) != EOF ) { diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c index 7a9faf3..daaaa99 100644 --- a/multipathd/uxlsnr.c +++ b/multipathd/uxlsnr.c @@ -32,6 +32,7 @@ #include "defaults.h" #include "config.h" #include "mpath_cmd.h" +#include "time-util.h" #include "main.h" #include "cli.h" @@ -99,21 +100,22 @@ void free_polls (void) FREE(polls); } -void check_timeout(struct timeval start_time, char *inbuf, +void check_timeout(struct timespec start_time, char *inbuf, unsigned int timeout) { - struct timeval diff_time, end_time; + struct timespec diff_time, end_time; - if (start_time.tv_sec && gettimeofday(&end_time, NULL) == 0) { - timersub(&end_time, &start_time, &diff_time); + if (start_time.tv_sec && + clock_gettime(CLOCK_MONOTONIC, &end_time) == 0) { unsigned long msecs; + timespecsub(&end_time, &start_time, &diff_time); msecs = diff_time.tv_sec * 1000 + - diff_time.tv_usec / 1000; + diff_time.tv_nsec / (1000 * 1000); if (msecs > timeout) condlog(2, "cli cmd '%s' timeout reached " "after %lu.%06lu secs", inbuf, - diff_time.tv_sec, diff_time.tv_usec); + diff_time.tv_sec, diff_time.tv_nsec / 1000); } } @@ -220,7 +222,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data) /* see if a client wants to speak to us */ for (i = 1; i < num_clients + 1; i++) { if (polls[i].revents & POLLIN) { - struct timeval start_time; + struct timespec start_time; c = NULL; pthread_mutex_lock(&client_lock); @@ -236,7 +238,8 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data) i, polls[i].fd); continue; } - if (gettimeofday(&start_time, NULL) != 0) + if (clock_gettime(CLOCK_MONOTONIC, &start_time) + != 0) start_time.tv_sec = 0; if (recv_packet(c->fd, &inbuf, uxsock_timeout) != 0) { -- 2.10.0 </blockquote></div> </div>