[dm-devel] [RFC PATCH] multipathd: strict_timing without signals

[Martin Wilck]

The internal usage of SIGALRM by setitimer() function may cause subtle
conflicts with other uses of SIGALRM, either by multipath-tools code itself
(e.g. lock_file()) or libc (e.g. glob()).

This patch changes the checkerloop to use an interval timer and a pthread
condition variable. No signals are used except those used by libpthread
behind the scenes.

Signed-off-by: Martin Wilck <mwilck at suse.com>
---
 multipathd/Makefile |   2 +-
 multipathd/main.c   | 113 ++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/multipathd/Makefile b/multipathd/Makefile
index 4c9d29634160..c348f02ef7f8 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -10,7 +10,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathpersistdir) \
 	  -I$(mpathcmddir) -I$(thirdpartydir)
 LDFLAGS += $(BIN_LDFLAGS)
 LIBDEPS += -L$(multipathdir) -lmultipath -L$(mpathpersistdir) -lmpathpersist \
-	   -L$(mpathcmddir) -lmpathcmd -ludev -ldl -lurcu -lpthread \
+	   -L$(mpathcmddir) -lmpathcmd -ludev -ldl -lurcu -lpthread -lrt \
 	   -ldevmapper -lreadline
 
 ifdef SYSTEMD
diff --git a/multipathd/main.c b/multipathd/main.c
index 6e6c52a52783..c9c57c5baece 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1848,6 +1848,68 @@ static void init_path_check_interval(struct vectors *vecs)
 	}
 }
 
+static pthread_mutex_t checker_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t checker_cond = PTHREAD_COND_INITIALIZER;
+
+static void checker_notify(union sigval sv) {
+
+	pthread_mutex_lock(&checker_lock);
+	pthread_cond_broadcast(&checker_cond);
+	pthread_mutex_unlock(&checker_lock);
+}
+
+static void unlock_mutex(void *arg)
+{
+	pthread_mutex_unlock((pthread_mutex_t*)arg);
+}
+
+void cleanup_timer(void *arg)
+{
+	timer_t tmr = (timer_t)arg;
+
+	if (tmr != 0)
+		timer_delete(tmr);
+}
+
+static timer_t setup_check_timer(void)
+{
+	timer_t tmr = (timer_t)0;
+	struct sigevent se;
+
+	memset(&se, 0, sizeof(se));
+	se.sigev_notify = SIGEV_THREAD;
+	se.sigev_notify_function = checker_notify;
+
+	if (timer_create(CLOCK_MONOTONIC, &se, &tmr)) {
+		condlog(2, "%s: errno %d creating monotonic timer",
+			__func__, errno);
+		if (timer_create(CLOCK_REALTIME, &se, &tmr))
+			condlog(0, "%s: errno %d creating timer",
+				__func__, errno);
+	}
+	return tmr;
+}
+
+static int set_check_timer(timer_t tmr, bool arm)
+{
+	struct itimerspec is;
+
+	if (tmr == 0)
+		return -1;
+
+	if (arm) {
+		is.it_value.tv_sec = 1;
+		is.it_interval.tv_sec = 1;
+	} else {
+		is.it_value.tv_sec = 0;
+		is.it_interval.tv_sec = 0;
+	}
+	is.it_value.tv_nsec = 0;
+	is.it_interval.tv_nsec = 0;
+
+	return timer_settime(tmr, 0, &is, NULL);
+}
+
 static void *
 checkerloop (void *ap)
 {
@@ -1855,9 +1917,10 @@ checkerloop (void *ap)
 	struct path *pp;
 	int count = 0;
 	unsigned int i;
-	struct itimerval timer_tick_it;
 	struct timespec last_time;
 	struct config *conf;
+	timer_t check_timer;
+	int old_strict_timing = 0;
 
 	pthread_cleanup_push(rcu_unregister, NULL);
 	rcu_register_thread();
@@ -1865,6 +1928,9 @@ checkerloop (void *ap)
 	vecs = (struct vectors *)ap;
 	condlog(2, "path checkers start up");
 
+	check_timer = setup_check_timer();
+	pthread_cleanup_push(cleanup_timer, (void*)check_timer);
+
 	/* Tweak start time for initial path check */
 	if (clock_gettime(CLOCK_MONOTONIC, &last_time) != 0)
 		last_time.tv_sec = 0;
@@ -1873,8 +1939,7 @@ checkerloop (void *ap)
 
 	while (1) {
 		struct timespec diff_time, start_time, end_time;
-		int num_paths = 0, ticks = 0, signo, strict_timing, rc = 0;
-		sigset_t mask;
+		int num_paths = 0, ticks = 0, strict_timing, rc = 0;
 
 		if (clock_gettime(CLOCK_MONOTONIC, &start_time) != 0)
 			start_time.tv_sec = 0;
@@ -1956,40 +2021,30 @@ checkerloop (void *ap)
 		}
 		check_foreign();
 		post_config_state(DAEMON_IDLE);
+
 		conf = get_multipath_config();
 		strict_timing = conf->strict_timing;
 		put_multipath_config(conf);
-		if (!strict_timing)
+
+		if (check_timer && (old_strict_timing != strict_timing) &&
+		    set_check_timer(check_timer, !!strict_timing) != 0) {
+			condlog(1, "%s: errno %d %sarming interrupt timer",
+				__func__, errno, strict_timing ? "" : "dis");
+			strict_timing = 0;
+		}
+		old_strict_timing = strict_timing;
+
+		if (!strict_timing || !check_timer)
 			sleep(1);
 		else {
-			timer_tick_it.it_interval.tv_sec = 0;
-			timer_tick_it.it_interval.tv_usec = 0;
-			if (diff_time.tv_nsec) {
-				timer_tick_it.it_value.tv_sec = 0;
-				timer_tick_it.it_value.tv_usec =
-				     1000UL * 1000 * 1000 - diff_time.tv_nsec;
-			} else {
-				timer_tick_it.it_value.tv_sec = 1;
-				timer_tick_it.it_value.tv_usec = 0;
-			}
-			setitimer(ITIMER_REAL, &timer_tick_it, NULL);
-
-			sigemptyset(&mask);
-			sigaddset(&mask, SIGALRM);
-			condlog(3, "waiting for %lu.%06lu secs",
-				timer_tick_it.it_value.tv_sec,
-				timer_tick_it.it_value.tv_usec);
-			if (sigwait(&mask, &signo) != 0) {
-				condlog(3, "sigwait failed with error %d",
-					errno);
-				conf = get_multipath_config();
-				conf->strict_timing = 0;
-				put_multipath_config(conf);
-				break;
-			}
+			pthread_mutex_lock(&checker_lock);
+			pthread_cleanup_push(unlock_mutex, &checker_lock);
+			pthread_cond_wait(&checker_cond, &checker_lock);
+			pthread_cleanup_pop(1);
 		}
 	}
 	pthread_cleanup_pop(1);
+	pthread_cleanup_pop(1);
 	return NULL;
 }
 
-- 
2.16.1