[PATCH 00/15] Fifteen multipath-tools patches

[PATCH 00/15] Fifteen multipath-tools patches

[Bart Van Assche]

Hello Christophe,

As promised I am sending you the multipath-tools patches from my local 
repository. The names of these patches are:

0001-libmultipath-prioritizers-ontap-Remove-an-unused-inc.patch
0002-libmultipath-memory-Move-an-include-directive.patch
0003-libmultipath-checkers-tur-Declare-local-functions-st.patch
0004-libmultipath-checkers-rbd-Declare-local-functions-st.patch
0005-multipathd-libmultipathd-Make-delays-independent-of-.patch
0006-multipathd-Suppress-uninteresting-data-race-reports.patch
0007-multipathd-Fix-a-data-race.patch
0008-libmultipath-checkers-tur-Introduce-strlcpy.patch
0009-libmultipath-checkers-tur-Terminate-a-statement-with.patch
0010-libmultipath-checkers-tur-Move-pthread_attr_destroy-.patch
0011-libmultipath-checkers-tur-Protect-tur_checker_contex.patch
0012-libmultipath-checkers-tur-Serialize-tur_checker_cont.patch
0013-libmultipath-checkers-tur-Fix-races-on-tur_checker_c.patch
0014-libmultipath-Micro-optimize-snprint_size.patch
0015-Makefile.inc-Enable-Wextra-and-Wformat-nonliteral.patch

Thanks,

Bart.

[PATCH 01/15] libmultipath/prioritizers/ontap: Remove an unused #include directive

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/prioritizers/ontap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libmultipath/prioritizers/ontap.c b/libmultipath/prioritizers/ontap.c
index 4084c65..38495cd 100644
--- a/libmultipath/prioritizers/ontap.c
+++ b/libmultipath/prioritizers/ontap.c
@@ -17,7 +17,6 @@
 #include <string.h>
 #include <sys/ioctl.h>
 #include <errno.h>
-#include <assert.h>
 
 #include "sg_include.h"
 #include "debug.h"
-- 
2.10.0

[PATCH 02/15] libmultipath/memory: Move an #include directive

[Bart Van Assche]

Minimize the number of #include directives in libmultipath/memory.h
by moving #include <assert.h> into libmultipath/memory.c.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/memory.c | 1 +
 libmultipath/memory.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/memory.c b/libmultipath/memory.c
index 5441e6a..293a688 100644
--- a/libmultipath/memory.c
+++ b/libmultipath/memory.c
@@ -20,6 +20,7 @@
  * Copyright (C) 2001-2005 Alexandre Cassen, <acassen@linux-vs.org>
  */
 
+#include <assert.h>
 #include "memory.h"
 
 /*
diff --git a/libmultipath/memory.h b/libmultipath/memory.h
index 29a75ed..a3c478e 100644
--- a/libmultipath/memory.h
+++ b/libmultipath/memory.h
@@ -27,7 +27,6 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
-#include <assert.h>
 
 /* Local defines */
 #ifdef _DEBUG_
-- 
2.10.0

[PATCH 03/15] libmultipath/checkers/tur: Declare local functions static

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/checkers/tur.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index ad66918..94e4190 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -68,7 +68,7 @@ int libcheck_init (struct checker * c)
 	return 0;
 }
 
-void cleanup_context(struct tur_checker_context *ct)
+static void cleanup_context(struct tur_checker_context *ct)
 {
 	pthread_mutex_destroy(&ct->lock);
 	pthread_cond_destroy(&ct->active);
@@ -104,7 +104,7 @@ void libcheck_repair (struct checker * c)
 
 #define TUR_MSG(msg, fmt, args...) snprintf(msg, CHECKER_MSG_LEN, fmt, ##args);
 
-int
+static int
 tur_check(int fd, unsigned int timeout, char *msg)
 {
 	struct sg_io_hdr io_hdr;
@@ -191,7 +191,7 @@ retry:
 #define tur_thread_cleanup_push(ct) pthread_cleanup_push(cleanup_func, ct)
 #define tur_thread_cleanup_pop(ct) pthread_cleanup_pop(1)
 
-void cleanup_func(void *data)
+static void cleanup_func(void *data)
 {
 	int holders;
 	struct tur_checker_context *ct = data;
@@ -204,7 +204,7 @@ void cleanup_func(void *data)
 		cleanup_context(ct);
 }
 
-void *tur_thread(void *ctx)
+static void *tur_thread(void *ctx)
 {
 	struct tur_checker_context *ct = ctx;
 	int state;
@@ -235,7 +235,7 @@ void *tur_thread(void *ctx)
 }
 
 
-void tur_timeout(struct timespec *tsp)
+static void tur_timeout(struct timespec *tsp)
 {
 	struct timeval now;
 
@@ -245,7 +245,7 @@ void tur_timeout(struct timespec *tsp)
 	tsp->tv_nsec += 1000000; /* 1 millisecond */
 }
 
-void tur_set_async_timeout(struct checker *c)
+static void tur_set_async_timeout(struct checker *c)
 {
 	struct tur_checker_context *ct = c->context;
 	struct timeval now;
@@ -254,7 +254,7 @@ void tur_set_async_timeout(struct checker *c)
 	ct->time = now.tv_sec + c->timeout;
 }
 
-int tur_check_async_timeout(struct checker *c)
+static int tur_check_async_timeout(struct checker *c)
 {
 	struct tur_checker_context *ct = c->context;
 	struct timeval now;
-- 
2.10.0

[PATCH 04/15] libmultipath/checkers/rbd: Declare local functions static

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Mike Christie <mchristi@redhat.com>
---
 libmultipath/checkers/rbd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c
index 1ee29c4..41259c3 100644
--- a/libmultipath/checkers/rbd.c
+++ b/libmultipath/checkers/rbd.c
@@ -220,7 +220,7 @@ free_ct:
 	return 1;
 }
 
-void cleanup_context(struct rbd_checker_context *ct)
+static void cleanup_context(struct rbd_checker_context *ct)
 {
 	pthread_mutex_destroy(&ct->lock);
 	pthread_cond_destroy(&ct->active);
@@ -317,7 +317,7 @@ free_bufs:
 	return ret;
 }
 
-int rbd_check(struct rbd_checker_context *ct, char *msg)
+static int rbd_check(struct rbd_checker_context *ct, char *msg)
 {
 	if (ct->blacklisted || rbd_is_blacklisted(ct, msg) == 1)
 		return PATH_DOWN;
@@ -333,7 +333,7 @@ int rbd_check(struct rbd_checker_context *ct, char *msg)
 	return PATH_UP;
 }
 
-int safe_write(int fd, const void *buf, size_t count)
+static int safe_write(int fd, const void *buf, size_t count)
 {
 	while (count > 0) {
 		ssize_t r = write(fd, buf, count);
@@ -493,7 +493,7 @@ static int rbd_repair(struct rbd_checker_context *ct, char *msg)
 #define rbd_thread_cleanup_push(ct) pthread_cleanup_push(cleanup_func, ct)
 #define rbd_thread_cleanup_pop(ct) pthread_cleanup_pop(1)
 
-void cleanup_func(void *data)
+static void cleanup_func(void *data)
 {
 	int holders;
 	struct rbd_checker_context *ct = data;
@@ -506,7 +506,7 @@ void cleanup_func(void *data)
 		cleanup_context(ct);
 }
 
-void *rbd_thread(void *ctx)
+static void *rbd_thread(void *ctx)
 {
 	struct rbd_checker_context *ct = ctx;
 	int state;
-- 
2.10.0

[PATCH 05/15] multipathd, libmultipathd: Make delays independent of clock jumps

[Bart Van Assche]

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 <bart.vanassche@sandisk.com>
Cc: Mike Christie <mchristi@redhat.com>
---
 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..f4cd906 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;
+				     1000UL * 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

[PATCH 07/15] multipathd: Fix a data race

[Bart Van Assche]

Avoid that the path check interval initialization loop races with
other code that accesses the path vectors by executing that code
on the context of the main thread instead of the checker loop.
This patch avoids that DRD reports the following:

Conflicting store by thread 1 at 0x07f3f1f8 size 8
   at 0x40B50C: reconfigure (main.c:2014)
   by 0x40C2EC: child (main.c:2371)
   by 0x40CDA1: main (main.c:2609)
Address 0x7f3f1f8 is at offset 40 from 0x7f3f1d0. Allocation context:
   at 0x4C32995: calloc (in /usr/lib64/valgrind/vgpreload_drd-amd64-linux.so)
   by 0x5DDAC36: zalloc (memory.c:34)
   by 0x40B61A: init_vecs (main.c:2043)
   by 0x40BF01: child (main.c:2295)
   by 0x40CDA1: main (main.c:2609)

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 multipathd/main.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 3030e85..cdfafe8 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1742,6 +1742,19 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 	return 1;
 }
 
+static void init_path_check_interval(struct vectors *vecs)
+{
+	struct config *conf;
+	struct path *pp;
+	unsigned int i;
+
+	vector_foreach_slot (vecs->pathvec, pp, i) {
+		conf = get_multipath_config();
+		pp->checkint = conf->checkint;
+		put_multipath_config(conf);
+	}
+}
+
 static void *
 checkerloop (void *ap)
 {
@@ -1759,15 +1772,6 @@ checkerloop (void *ap)
 	vecs = (struct vectors *)ap;
 	condlog(2, "path checkers start up");
 
-	/*
-	 * init the path check interval
-	 */
-	vector_foreach_slot (vecs->pathvec, pp, i) {
-		conf = get_multipath_config();
-		pp->checkint = conf->checkint;
-		put_multipath_config(conf);
-	}
-
 	/* Tweak start time for initial path check */
 	if (clock_gettime(CLOCK_MONOTONIC, &last_time) != 0)
 		last_time.tv_sec = 0;
@@ -2327,6 +2331,8 @@ child (void * param)
 	 */
 	post_config_state(DAEMON_CONFIGURE);
 
+	init_path_check_interval(vecs);
+
 	/*
 	 * Start uevent listener early to catch events
 	 */
-- 
2.10.0

[PATCH 08/15] libmultipath/checkers/tur: Introduce strlcpy()

[Bart Van Assche]

Change the strncpy() calls into strlcpy() calls instead of open-coding
strlcpy().

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/checkers/tur.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index ac272d4..67933cb 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -21,6 +21,7 @@
 #include "../libmultipath/sg_include.h"
 #include "../libmultipath/uevent.h"
 #include "../libmultipath/time-util.h"
+#include "../libmultipath/util.h"
 
 #define TUR_CMD_LEN 6
 #define HEAVY_CHECK_COUNT       10
@@ -311,8 +312,7 @@ libcheck_check (struct checker * c)
 			/* TUR checker done */
 			ct->running = 0;
 			tur_status = ct->state;
-			strncpy(c->message, ct->message, CHECKER_MSG_LEN);
-			c->message[CHECKER_MSG_LEN - 1] = '\0';
+			strlcpy(c->message, ct->message, sizeof(c->message));
 		}
 		pthread_mutex_unlock(&ct->lock);
 	} else {
@@ -347,8 +347,7 @@ libcheck_check (struct checker * c)
 		tur_timeout(&tsp);
 		r = pthread_cond_timedwait(&ct->active, &ct->lock, &tsp);
 		tur_status = ct->state;
-		strncpy(c->message, ct->message,CHECKER_MSG_LEN);
-		c->message[CHECKER_MSG_LEN - 1] = '\0';
+		strlcpy(c->message, ct->message, sizeof(c->message));
 		pthread_mutex_unlock(&ct->lock);
 		if (ct->thread &&
 		    (tur_status == PATH_PENDING || tur_status == PATH_UNCHECKED)) {
-- 
2.10.0

[PATCH 09/15] libmultipath/checkers/tur: Terminate a statement with a semicolon

[Bart Van Assche]

This patch does not affect any functionality but helps editors to
auto-indent the code below that statement correctly.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/checkers/tur.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index 67933cb..3f97f2a 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -215,7 +215,7 @@ static void *tur_thread(void *ctx)
 
 	ct->message[0] = '\0';
 	/* This thread can be canceled, so setup clean up */
-	tur_thread_cleanup_push(ct)
+	tur_thread_cleanup_push(ct);
 
 	/* TUR checker start up */
 	pthread_mutex_lock(&ct->lock);
-- 
2.10.0

[PATCH 10/15] libmultipath/checkers/tur: Move pthread_attr_destroy() call

[Bart Van Assche]

Move the pthread_attr_destroy() call up such that this function
is also called if thread creation fails.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/checkers/tur.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index 3f97f2a..11f3c60 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -333,6 +333,7 @@ libcheck_check (struct checker * c)
 		tur_set_async_timeout(c);
 		setup_thread_attr(&attr, 32 * 1024, 1);
 		r = pthread_create(&ct->thread, &attr, tur_thread, ct);
+		pthread_attr_destroy(&attr);
 		if (r) {
 			pthread_spin_lock(&ct->hldr_lock);
 			ct->holders--;
@@ -343,7 +344,6 @@ libcheck_check (struct checker * c)
 				" sync mode", TUR_DEVT(ct));
 			return tur_check(c->fd, c->timeout, c->message);
 		}
-		pthread_attr_destroy(&attr);
 		tur_timeout(&tsp);
 		r = pthread_cond_timedwait(&ct->active, &ct->lock, &tsp);
 		tur_status = ct->state;
-- 
2.10.0

[PATCH 11/15] libmultipath/checkers/tur: Protect tur_checker_context.message changes

[Bart Van Assche]

Avoid that tur_checker_context.message is modified without holding
the tur_checker_context.lock mutex. This patch avoids that data
race detection tools like DRD complain about writing into
tur_checker_context.message.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/checkers/tur.c | 46 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index 11f3c60..be3d5ea 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -104,10 +104,17 @@ void libcheck_repair (struct checker * c)
 	return;
 }
 
-#define TUR_MSG(msg, fmt, args...) snprintf(msg, CHECKER_MSG_LEN, fmt, ##args);
+#define TUR_MSG(fmt, args...)					\
+	do {							\
+		char msg[CHECKER_MSG_LEN];			\
+								\
+		snprintf(msg, sizeof(msg), fmt, ##args);	\
+		copy_message(cb_arg, msg);			\
+	} while (0)
 
 static int
-tur_check(int fd, unsigned int timeout, char *msg)
+tur_check(int fd, unsigned int timeout,
+	  void (*copy_message)(void *, const char *), void *cb_arg)
 {
 	struct sg_io_hdr io_hdr;
 	unsigned char turCmdBlk[TUR_CMD_LEN] = { 0x00, 0, 0, 0, 0, 0 };
@@ -126,7 +133,7 @@ retry:
 	io_hdr.timeout = timeout * 1000;
 	io_hdr.pack_id = 0;
 	if (ioctl(fd, SG_IO, &io_hdr) < 0) {
-		TUR_MSG(msg, MSG_TUR_DOWN);
+		TUR_MSG(MSG_TUR_DOWN);
 		return PATH_DOWN;
 	}
 	if ((io_hdr.status & 0x7e) == 0x18) {
@@ -134,7 +141,7 @@ retry:
 		 * SCSI-3 arrays might return
 		 * reservation conflict on TUR
 		 */
-		TUR_MSG(msg, MSG_TUR_UP);
+		TUR_MSG(MSG_TUR_UP);
 		return PATH_UP;
 	}
 	if (io_hdr.info & SG_INFO_OK_MASK) {
@@ -179,14 +186,14 @@ retry:
 				 * LOGICAL UNIT NOT ACCESSIBLE,
 				 * TARGET PORT IN STANDBY STATE
 				 */
-				TUR_MSG(msg, MSG_TUR_GHOST);
+				TUR_MSG(MSG_TUR_GHOST);
 				return PATH_GHOST;
 			}
 		}
-		TUR_MSG(msg, MSG_TUR_DOWN);
+		TUR_MSG(MSG_TUR_DOWN);
 		return PATH_DOWN;
 	}
-	TUR_MSG(msg, MSG_TUR_UP);
+	TUR_MSG(MSG_TUR_UP);
 	return PATH_UP;
 }
 
@@ -206,6 +213,15 @@ static void cleanup_func(void *data)
 		cleanup_context(ct);
 }
 
+static void copy_msg_to_tcc(void *ct_p, const char *msg)
+{
+	struct tur_checker_context *ct = ct_p;
+
+	pthread_mutex_lock(&ct->lock);
+	strlcpy(ct->message, msg, sizeof(ct->message));
+	pthread_mutex_unlock(&ct->lock);
+}
+
 static void *tur_thread(void *ctx)
 {
 	struct tur_checker_context *ct = ctx;
@@ -213,16 +229,16 @@ static void *tur_thread(void *ctx)
 
 	condlog(3, "%d:%d: tur checker starting up", TUR_DEVT(ct));
 
-	ct->message[0] = '\0';
 	/* This thread can be canceled, so setup clean up */
 	tur_thread_cleanup_push(ct);
 
 	/* TUR checker start up */
 	pthread_mutex_lock(&ct->lock);
 	ct->state = PATH_PENDING;
+	ct->message[0] = '\0';
 	pthread_mutex_unlock(&ct->lock);
 
-	state = tur_check(ct->fd, ct->timeout, ct->message);
+	state = tur_check(ct->fd, ct->timeout, copy_msg_to_tcc, ct->message);
 
 	/* TUR checker done */
 	pthread_mutex_lock(&ct->lock);
@@ -262,6 +278,13 @@ static int tur_check_async_timeout(struct checker *c)
 	return (now.tv_sec > ct->time);
 }
 
+static void copy_msg_to_checker(void *c_p, const char *msg)
+{
+	struct checker *c = c_p;
+
+	strlcpy(c->message, msg, sizeof(c->message));
+}
+
 extern int
 libcheck_check (struct checker * c)
 {
@@ -279,7 +302,7 @@ libcheck_check (struct checker * c)
 		ct->devt = sb.st_rdev;
 
 	if (c->sync)
-		return tur_check(c->fd, c->timeout, c->message);
+		return tur_check(c->fd, c->timeout, copy_msg_to_checker, c);
 
 	/*
 	 * Async mode
@@ -342,7 +365,8 @@ libcheck_check (struct checker * c)
 			ct->thread = 0;
 			condlog(3, "%d:%d: failed to start tur thread, using"
 				" sync mode", TUR_DEVT(ct));
-			return tur_check(c->fd, c->timeout, c->message);
+			return tur_check(c->fd, c->timeout,
+					 copy_msg_to_checker, c);
 		}
 		tur_timeout(&tsp);
 		r = pthread_cond_timedwait(&ct->active, &ct->lock, &tsp);
-- 
2.10.0

[PATCH 12/15] libmultipath/checkers/tur: Serialize tur_checker_context.devt accesses

[Bart Van Assche]

Avoid that tur_checker_context.devt is accessed without holding
the tur_checker_context.lock mutex. This avoids that data race
detection tools complain about tur_checker_context.devt accesses.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/checkers/tur.c | 52 ++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index be3d5ea..7605fb9 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -48,7 +48,18 @@ struct tur_checker_context {
 	char message[CHECKER_MSG_LEN];
 };
 
-#define TUR_DEVT(c) major((c)->devt), minor((c)->devt)
+static const char *tur_devt(char *devt_buf, int size,
+			    struct tur_checker_context *ct)
+{
+	dev_t devt;
+
+	pthread_mutex_lock(&ct->lock);
+	devt = ct->devt;
+	pthread_mutex_unlock(&ct->lock);
+
+	snprintf(devt_buf, size, "%d:%d", major(devt), minor(devt));
+	return devt_buf;
+}
 
 int libcheck_init (struct checker * c)
 {
@@ -226,8 +237,10 @@ static void *tur_thread(void *ctx)
 {
 	struct tur_checker_context *ct = ctx;
 	int state;
+	char devt[32];
 
-	condlog(3, "%d:%d: tur checker starting up", TUR_DEVT(ct));
+	condlog(3, "%s: tur checker starting up",
+		tur_devt(devt, sizeof(devt), ct));
 
 	/* This thread can be canceled, so setup clean up */
 	tur_thread_cleanup_push(ct);
@@ -246,9 +259,10 @@ static void *tur_thread(void *ctx)
 	pthread_cond_signal(&ct->active);
 	pthread_mutex_unlock(&ct->lock);
 
-	condlog(3, "%d:%d: tur checker finished, state %s",
-		TUR_DEVT(ct), checker_state_name(state));
+	condlog(3, "%s: tur checker finished, state %s",
+		tur_devt(devt, sizeof(devt), ct), checker_state_name(state));
 	tur_thread_cleanup_pop(ct);
+
 	return ((void *)0);
 }
 
@@ -293,13 +307,17 @@ libcheck_check (struct checker * c)
 	struct stat sb;
 	pthread_attr_t attr;
 	int tur_status, r;
+	char devt[32];
 
 
 	if (!ct)
 		return PATH_UNCHECKED;
 
-	if (fstat(c->fd, &sb) == 0)
+	if (fstat(c->fd, &sb) == 0) {
+		pthread_mutex_lock(&ct->lock);
 		ct->devt = sb.st_rdev;
+		pthread_mutex_unlock(&ct->lock);
+	}
 
 	if (c->sync)
 		return tur_check(c->fd, c->timeout, copy_msg_to_checker, c);
@@ -309,8 +327,8 @@ libcheck_check (struct checker * c)
 	 */
 	r = pthread_mutex_lock(&ct->lock);
 	if (r != 0) {
-		condlog(2, "%d:%d: tur mutex lock failed with %d",
-			TUR_DEVT(ct), r);
+		condlog(2, "%s: tur mutex lock failed with %d",
+			tur_devt(devt, sizeof(devt), ct), r);
 		MSG(c, MSG_TUR_FAILED);
 		return PATH_WILD;
 	}
@@ -319,15 +337,15 @@ libcheck_check (struct checker * c)
 		/* Check if TUR checker is still running */
 		if (ct->thread) {
 			if (tur_check_async_timeout(c)) {
-				condlog(3, "%d:%d: tur checker timeout",
-					TUR_DEVT(ct));
+				condlog(3, "%s: tur checker timeout",
+					tur_devt(devt, sizeof(devt), ct));
 				pthread_cancel(ct->thread);
 				ct->running = 0;
 				MSG(c, MSG_TUR_TIMEOUT);
 				tur_status = PATH_TIMEOUT;
 			} else {
-				condlog(3, "%d:%d: tur checker not finished",
-					TUR_DEVT(ct));
+				condlog(3, "%s: tur checker not finished",
+					tur_devt(devt, sizeof(devt), ct));
 				ct->running++;
 				tur_status = PATH_PENDING;
 			}
@@ -342,8 +360,8 @@ libcheck_check (struct checker * c)
 		if (ct->thread) {
 			/* pthread cancel failed. continue in sync mode */
 			pthread_mutex_unlock(&ct->lock);
-			condlog(3, "%d:%d: tur thread not responding",
-				TUR_DEVT(ct));
+			condlog(3, "%s: tur thread not responding",
+				tur_devt(devt, sizeof(devt), ct));
 			return PATH_TIMEOUT;
 		}
 		/* Start new TUR checker */
@@ -363,8 +381,8 @@ libcheck_check (struct checker * c)
 			pthread_spin_unlock(&ct->hldr_lock);
 			pthread_mutex_unlock(&ct->lock);
 			ct->thread = 0;
-			condlog(3, "%d:%d: failed to start tur thread, using"
-				" sync mode", TUR_DEVT(ct));
+			condlog(3, "%s: failed to start tur thread, using"
+				" sync mode", tur_devt(devt, sizeof(devt), ct));
 			return tur_check(c->fd, c->timeout,
 					 copy_msg_to_checker, c);
 		}
@@ -375,8 +393,8 @@ libcheck_check (struct checker * c)
 		pthread_mutex_unlock(&ct->lock);
 		if (ct->thread &&
 		    (tur_status == PATH_PENDING || tur_status == PATH_UNCHECKED)) {
-			condlog(3, "%d:%d: tur checker still running",
-				TUR_DEVT(ct));
+			condlog(3, "%s: tur checker still running",
+				tur_devt(devt, sizeof(devt), ct));
 			ct->running = 1;
 			tur_status = PATH_PENDING;
 		}
-- 
2.10.0

[PATCH 13/15] libmultipath/checkers/tur: Fix races on tur_checker_context.thread

[Bart Van Assche]

Avoid that pthread_cancel(ct->thread) can get called after the TUR
thread exited because this is not allowed for detached threads.
Avoid that data-race detection tools complain about reading ct->thread
without holding ct->hldr_lock.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/checkers/tur.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index 7605fb9..a7a70f6 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -224,6 +224,17 @@ static void cleanup_func(void *data)
 		cleanup_context(ct);
 }
 
+static int tur_running(struct tur_checker_context *ct)
+{
+	pthread_t thread;
+
+	pthread_spin_lock(&ct->hldr_lock);
+	thread = ct->thread;
+	pthread_spin_unlock(&ct->hldr_lock);
+
+	return thread != 0;
+}
+
 static void copy_msg_to_tcc(void *ct_p, const char *msg)
 {
 	struct tur_checker_context *ct = ct_p;
@@ -334,7 +345,13 @@ libcheck_check (struct checker * c)
 	}
 
 	if (ct->running) {
-		/* Check if TUR checker is still running */
+		/*
+		 * Check if TUR checker is still running. Hold hldr_lock
+		 * around the pthread_cancel() call to avoid that
+		 * pthread_cancel() gets called after the (detached) TUR
+		 * thread has exited.
+		 */
+		pthread_spin_lock(&ct->hldr_lock);
 		if (ct->thread) {
 			if (tur_check_async_timeout(c)) {
 				condlog(3, "%s: tur checker timeout",
@@ -355,9 +372,10 @@ libcheck_check (struct checker * c)
 			tur_status = ct->state;
 			strlcpy(c->message, ct->message, sizeof(c->message));
 		}
+		pthread_spin_unlock(&ct->hldr_lock);
 		pthread_mutex_unlock(&ct->lock);
 	} else {
-		if (ct->thread) {
+		if (tur_running(ct)) {
 			/* pthread cancel failed. continue in sync mode */
 			pthread_mutex_unlock(&ct->lock);
 			condlog(3, "%s: tur thread not responding",
@@ -391,7 +409,7 @@ libcheck_check (struct checker * c)
 		tur_status = ct->state;
 		strlcpy(c->message, ct->message, sizeof(c->message));
 		pthread_mutex_unlock(&ct->lock);
-		if (ct->thread &&
+		if (tur_running(ct) &&
 		    (tur_status == PATH_PENDING || tur_status == PATH_UNCHECKED)) {
 			condlog(3, "%s: tur checker still running",
 				tur_devt(devt, sizeof(devt), ct));
-- 
2.10.0

[PATCH 14/15] libmultipath: Micro-optimize snprint_size()

[Bart Van Assche]

Eliminate the stack array fmt[]. An interesting side effect of
this patch is that it makes it possible for the compiler to
verify whether the snprintf() arguments have a type that is
appropriate for the format string.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 libmultipath/print.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 94d6384..9aa41ad 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -73,7 +73,6 @@ static int
 snprint_size (char * buff, size_t len, unsigned long long size)
 {
 	float s = (float)(size >> 1); /* start with KB */
-	char fmt[6] = {};
 	char units[] = {'K','M','G','T','P'};
 	char *u = units;
 
@@ -81,12 +80,8 @@ snprint_size (char * buff, size_t len, unsigned long long size)
 		s = s / 1024;
 		u++;
 	}
-	if (s < 10)
-		snprintf(fmt, 6, "%%.1f%c", *u);
-	else
-		snprintf(fmt, 6, "%%.0f%c", *u);
 
-	return snprintf(buff, len, fmt, s);
+	return snprintf(buff, len, "%.*f%c", s < 10, s, *u);
 }
 
 /*
-- 
2.10.0

[PATCH 15/15] Makefile.inc: Enable -Wextra and -Wformat-nonliteral

[Bart Van Assche]

Instead of selecting some of the options enabled by -Wextra, enable
-Wextra and disable some of the options enabled by -Wextra. Enable
-Wformat-nonliteral.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 Makefile.inc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Makefile.inc b/Makefile.inc
index 376f00a..be6b2bc 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -61,8 +61,10 @@ RM		= rm -f
 LN		= ln -sf
 INSTALL_PROGRAM	= install
 
-OPTFLAGS	= -Wunused -Wstrict-prototypes -O2 -g -pipe -Wformat-security -Wall \
-		  -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4
+OPTFLAGS	= -O2 -g -pipe -Wall -Wextra -Wformat=2 \
+		  -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered\
+		  -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector \
+		  --param=ssp-buffer-size=4
 
 CFLAGS		= $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
 SHARED_FLAGS	= -shared
-- 
2.10.0

Re: [PATCH 00/15] Fifteen multipath-tools patches

[Christophe Varoqui]

[-- Attachment #1.1: Type: text/plain, Size: 1279 bytes --]

The set is merged.
I just added a patch to fix the drd include in debug.c

Thanks,
Christophe

On Tue, Oct 4, 2016 at 7:36 PM, Bart Van Assche <bart.vanassche@sandisk.com>
wrote:

> Hello Christophe,
>
> As promised I am sending you the multipath-tools patches from my local
> repository. The names of these patches are:
>
> 0001-libmultipath-prioritizers-ontap-Remove-an-unused-inc.patch
> 0002-libmultipath-memory-Move-an-include-directive.patch
> 0003-libmultipath-checkers-tur-Declare-local-functions-st.patch
> 0004-libmultipath-checkers-rbd-Declare-local-functions-st.patch
> 0005-multipathd-libmultipathd-Make-delays-independent-of-.patch
> 0006-multipathd-Suppress-uninteresting-data-race-reports.patch
> 0007-multipathd-Fix-a-data-race.patch
> 0008-libmultipath-checkers-tur-Introduce-strlcpy.patch
> 0009-libmultipath-checkers-tur-Terminate-a-statement-with.patch
> 0010-libmultipath-checkers-tur-Move-pthread_attr_destroy-.patch
> 0011-libmultipath-checkers-tur-Protect-tur_checker_contex.patch
> 0012-libmultipath-checkers-tur-Serialize-tur_checker_cont.patch
> 0013-libmultipath-checkers-tur-Fix-races-on-tur_checker_c.patch
> 0014-libmultipath-Micro-optimize-snprint_size.patch
> 0015-Makefile.inc-Enable-Wextra-and-Wformat-nonliteral.patch
>
> Thanks,
>
> Bart.
>

[-- Attachment #1.2: Type: text/html, Size: 1818 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 07/15] multipathd: Fix a data race

[tang.junhui]

[-- Attachment #1.1: Type: text/plain, Size: 3461 bytes --]

Hello Bart,
This patch solved the data race problem,
but the assignment for paths check interval has no effect,
since there is no path in vecs when calling init_path_check_interval(vecs) 
in child(),
I think it is better to call  init_path_check_interval(vecs) at 
reconfigure() or configure() 
after the paths has created in vecs .

Thanks,
Tang





发件人:         Bart Van Assche <bart.vanassche@sandisk.com>
收件人:         Christophe Varoqui <christophe.varoqui@opensvc.com>, 
抄送:   device-mapper development <dm-devel@redhat.com>
日期:   2016/10/05 01:45
主题:   [dm-devel] [PATCH 07/15] multipathd: Fix a data race
发件人: dm-devel-bounces@redhat.com



Avoid that the path check interval initialization loop races with
other code that accesses the path vectors by executing that code
on the context of the main thread instead of the checker loop.
This patch avoids that DRD reports the following:

Conflicting store by thread 1 at 0x07f3f1f8 size 8
   at 0x40B50C: reconfigure (main.c:2014)
   by 0x40C2EC: child (main.c:2371)
   by 0x40CDA1: main (main.c:2609)
Address 0x7f3f1f8 is at offset 40 from 0x7f3f1d0. Allocation context:
   at 0x4C32995: calloc (in 
/usr/lib64/valgrind/vgpreload_drd-amd64-linux.so)
   by 0x5DDAC36: zalloc (memory.c:34)
   by 0x40B61A: init_vecs (main.c:2043)
   by 0x40BF01: child (main.c:2295)
   by 0x40CDA1: main (main.c:2609)

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 multipathd/main.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 3030e85..cdfafe8 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1742,6 +1742,19 @@ check_path (struct vectors * vecs, struct path * 
pp, int ticks)
                 return 1;
 }
 
+static void init_path_check_interval(struct vectors *vecs)
+{
+                struct config *conf;
+                struct path *pp;
+                unsigned int i;
+
+                vector_foreach_slot (vecs->pathvec, pp, i) {
+                                conf = get_multipath_config();
+                                pp->checkint = conf->checkint;
+                                put_multipath_config(conf);
+                }
+}
+
 static void *
 checkerloop (void *ap)
 {
@@ -1759,15 +1772,6 @@ checkerloop (void *ap)
                 vecs = (struct vectors *)ap;
                 condlog(2, "path checkers start up");
 
-                /*
-                 * init the path check interval
-                 */
-                vector_foreach_slot (vecs->pathvec, pp, i) {
-                                conf = get_multipath_config();
-                                pp->checkint = conf->checkint;
-                                put_multipath_config(conf);
-                }
-
                 /* Tweak start time for initial path check */
                 if (clock_gettime(CLOCK_MONOTONIC, &last_time) != 0)
                                 last_time.tv_sec = 0;
@@ -2327,6 +2331,8 @@ child (void * param)
                  */
                 post_config_state(DAEMON_CONFIGURE);
 
+                init_path_check_interval(vecs);
+
                 /*
                  * Start uevent listener early to catch events
                  */
-- 
2.10.0

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel



[-- Attachment #1.2: Type: text/html, Size: 6578 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 00/15] Fifteen multipath-tools patches

[tang.junhui]

[-- Attachment #1.1: Type: text/plain, Size: 1237 bytes --]

Hello Bart,
I also think that we do not need init_path_check_interval()，
since pp->checkint is initialized in configure() too.

Cheers,
Tang




发件人:         Bart Van Assche <bart.vanassche@sandisk.com>
收件人:         <tang.junhui@zte.com.cn>, 
<christophe.varoqui@opensvc.com>, 
抄送:   bart.vanassche@sandisk.com, dm-devel@redhat.com
日期:   2016/10/11 04:41
主题:   Re: [dm-devel] [PATCH 00/15] Fifteen multipath-tools patches
发件人: dm-devel-bounces@redhat.com



On 10/07/2016 07:05 PM, tang.junhui@zte.com.cn wrote:
> "0007-multipathd-Fix-a-data-race.patch"
>
> This patch solved the data race problem,
>
> but the assignment for paths check interval has no effect,
>
> since there is no path in vecs when calling
> init_path_check_interval(vecs) in child(),
>
> I think it is better to call
>  init_path_check_interval(vecs) at reconfigure() or configure()
>
> after the paths has created in vecs .

Hello Tang,

If init_path_check_interval() doesn't have any effect then I propose to 
remove that function. pp->checkint is namely already initialized in 
cli_add_path().

Bart.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel



[-- Attachment #1.2: Type: text/html, Size: 2640 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 00/15] Fifteen multipath-tools patches

[Bart Van Assche]

On 10/07/2016 07:05 PM, tang.junhui@zte.com.cn wrote:
> "0007-multipathd-Fix-a-data-race.patch"
>
> This patch solved the data race problem,
>
> but the assignment for paths check interval has no effect,
>
> since there is no path in vecs when calling
> init_path_check_interval(vecs) in child(),
>
> I think it is better to call
>  init_path_check_interval(vecs) at reconfigure() or configure()
>
> after the paths has created in vecs .

Hello Tang,

If init_path_check_interval() doesn't have any effect then I propose to 
remove that function. pp->checkint is namely already initialized in 
cli_add_path().

Bart.

Re: [PATCH 00/15] Fifteen multipath-tools patches

[tang.junhui]

[-- Attachment #1.1.1: Type: text/plain, Size: 1994 bytes --]

Hello Bart,



"0007-multipathd-Fix-a-data-race.patch"

This patch solved the data race problem, 

but the assignment for paths check interval has no effect,

since there is no path in vecs when calling init_path_check_interval(vecs) in child(),

I think it is better to call  init_path_check_interval(vecs) at reconfigure() or configure() 

after the paths has created in vecs .




Thanks,

Tang























原始邮件



发件人：ChristopheVaroqui
收件人：Bart Van Assche
抄送人：device-mapper development
日 期 ：2016年10月05日 14:56
主 题 ：Re: [dm-devel] [PATCH 00/15] Fifteen multipath-tools patches





The set is merged.I just added a patch to fix the drd include in debug.c

Thanks,
Christophe




On Tue, Oct 4, 2016 at 7:36 PM, Bart Van Assche ＜bart.vanassche@sandisk.com＞ wrote:
Hello Christophe,
 
 As promised I am sending you the multipath-tools patches from my local repository. The names of these patches are:
 
 0001-libmultipath-prioritizers-ontap-Remove-an-unused-inc.patch
 0002-libmultipath-memory-Move-an-include-directive.patch
 0003-libmultipath-checkers-tur-Declare-local-functions-st.patch
 0004-libmultipath-checkers-rbd-Declare-local-functions-st.patch
 0005-multipathd-libmultipathd-Make-delays-independent-of-.patch
 0006-multipathd-Suppress-uninteresting-data-race-reports.patch
 0007-multipathd-Fix-a-data-race.patch
 0008-libmultipath-checkers-tur-Introduce-strlcpy.patch
 0009-libmultipath-checkers-tur-Terminate-a-statement-with.patch
 0010-libmultipath-checkers-tur-Move-pthread_attr_destroy-.patch
 0011-libmultipath-checkers-tur-Protect-tur_checker_contex.patch
 0012-libmultipath-checkers-tur-Serialize-tur_checker_cont.patch
 0013-libmultipath-checkers-tur-Fix-races-on-tur_checker_c.patch
 0014-libmultipath-Micro-optimize-snprint_size.patch
 0015-Makefile.inc-Enable-Wextra-and-Wformat-nonliteral.patch
 
 Thanks,
 
 Bart.

[-- Attachment #1.1.2: Type: text/html , Size: 3885 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]