* [PATCH 01/15] libmultipath/prioritizers/ontap: Remove an unused #include directive
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
@ 2016-10-04 17:36 ` Bart Van Assche
2016-10-04 17:37 ` [PATCH 02/15] libmultipath/memory: Move an " Bart Van Assche
` (13 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:36 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 02/15] libmultipath/memory: Move an #include directive
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
2016-10-04 17:36 ` [PATCH 01/15] libmultipath/prioritizers/ontap: Remove an unused #include directive Bart Van Assche
@ 2016-10-04 17:37 ` Bart Van Assche
2016-10-04 17:37 ` [PATCH 03/15] libmultipath/checkers/tur: Declare local functions static Bart Van Assche
` (12 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:37 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 03/15] libmultipath/checkers/tur: Declare local functions static
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
2016-10-04 17:36 ` [PATCH 01/15] libmultipath/prioritizers/ontap: Remove an unused #include directive Bart Van Assche
2016-10-04 17:37 ` [PATCH 02/15] libmultipath/memory: Move an " Bart Van Assche
@ 2016-10-04 17:37 ` Bart Van Assche
2016-10-04 17:38 ` [PATCH 04/15] libmultipath/checkers/rbd: " Bart Van Assche
` (11 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:37 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 04/15] libmultipath/checkers/rbd: Declare local functions static
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (2 preceding siblings ...)
2016-10-04 17:37 ` [PATCH 03/15] libmultipath/checkers/tur: Declare local functions static Bart Van Assche
@ 2016-10-04 17:38 ` Bart Van Assche
2016-10-04 17:38 ` [PATCH 05/15] multipathd, libmultipathd: Make delays independent of clock jumps Bart Van Assche
` (10 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:38 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Mike Christie
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 05/15] multipathd, libmultipathd: Make delays independent of clock jumps
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (3 preceding siblings ...)
2016-10-04 17:38 ` [PATCH 04/15] libmultipath/checkers/rbd: " Bart Van Assche
@ 2016-10-04 17:38 ` Bart Van Assche
2016-10-04 17:39 ` [PATCH 07/15] multipathd: Fix a data race Bart Van Assche
` (9 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:38 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Mike Christie
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 07/15] multipathd: Fix a data race
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (4 preceding siblings ...)
2016-10-04 17:38 ` [PATCH 05/15] multipathd, libmultipathd: Make delays independent of clock jumps Bart Van Assche
@ 2016-10-04 17:39 ` Bart Van Assche
2016-10-08 2:14 ` tang.junhui
2016-10-04 17:39 ` [PATCH 08/15] libmultipath/checkers/tur: Introduce strlcpy() Bart Van Assche
` (8 subsequent siblings)
14 siblings, 1 reply; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:39 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 07/15] multipathd: Fix a data race
2016-10-04 17:39 ` [PATCH 07/15] multipathd: Fix a data race Bart Van Assche
@ 2016-10-08 2:14 ` tang.junhui
0 siblings, 0 replies; 17+ messages in thread
From: tang.junhui @ 2016-10-08 2:14 UTC (permalink / raw)
To: Bart Van Assche; +Cc: device-mapper development, dm-devel-bounces
[-- 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 --]
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 08/15] libmultipath/checkers/tur: Introduce strlcpy()
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (5 preceding siblings ...)
2016-10-04 17:39 ` [PATCH 07/15] multipathd: Fix a data race Bart Van Assche
@ 2016-10-04 17:39 ` Bart Van Assche
2016-10-04 17:40 ` [PATCH 09/15] libmultipath/checkers/tur: Terminate a statement with a semicolon Bart Van Assche
` (7 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:39 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 09/15] libmultipath/checkers/tur: Terminate a statement with a semicolon
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (6 preceding siblings ...)
2016-10-04 17:39 ` [PATCH 08/15] libmultipath/checkers/tur: Introduce strlcpy() Bart Van Assche
@ 2016-10-04 17:40 ` Bart Van Assche
2016-10-04 17:40 ` [PATCH 10/15] libmultipath/checkers/tur: Move pthread_attr_destroy() call Bart Van Assche
` (6 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:40 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 10/15] libmultipath/checkers/tur: Move pthread_attr_destroy() call
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (7 preceding siblings ...)
2016-10-04 17:40 ` [PATCH 09/15] libmultipath/checkers/tur: Terminate a statement with a semicolon Bart Van Assche
@ 2016-10-04 17:40 ` Bart Van Assche
2016-10-04 17:40 ` [PATCH 11/15] libmultipath/checkers/tur: Protect tur_checker_context.message changes Bart Van Assche
` (5 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:40 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 11/15] libmultipath/checkers/tur: Protect tur_checker_context.message changes
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (8 preceding siblings ...)
2016-10-04 17:40 ` [PATCH 10/15] libmultipath/checkers/tur: Move pthread_attr_destroy() call Bart Van Assche
@ 2016-10-04 17:40 ` Bart Van Assche
2016-10-04 17:41 ` [PATCH 12/15] libmultipath/checkers/tur: Serialize tur_checker_context.devt accesses Bart Van Assche
` (4 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:40 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 12/15] libmultipath/checkers/tur: Serialize tur_checker_context.devt accesses
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (9 preceding siblings ...)
2016-10-04 17:40 ` [PATCH 11/15] libmultipath/checkers/tur: Protect tur_checker_context.message changes Bart Van Assche
@ 2016-10-04 17:41 ` Bart Van Assche
2016-10-04 17:41 ` [PATCH 13/15] libmultipath/checkers/tur: Fix races on tur_checker_context.thread Bart Van Assche
` (3 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:41 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 13/15] libmultipath/checkers/tur: Fix races on tur_checker_context.thread
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (10 preceding siblings ...)
2016-10-04 17:41 ` [PATCH 12/15] libmultipath/checkers/tur: Serialize tur_checker_context.devt accesses Bart Van Assche
@ 2016-10-04 17:41 ` Bart Van Assche
2016-10-04 17:41 ` [PATCH 14/15] libmultipath: Micro-optimize snprint_size() Bart Van Assche
` (2 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:41 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 14/15] libmultipath: Micro-optimize snprint_size()
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (11 preceding siblings ...)
2016-10-04 17:41 ` [PATCH 13/15] libmultipath/checkers/tur: Fix races on tur_checker_context.thread Bart Van Assche
@ 2016-10-04 17:41 ` Bart Van Assche
2016-10-04 17:42 ` [PATCH 15/15] Makefile.inc: Enable -Wextra and -Wformat-nonliteral Bart Van Assche
2016-10-05 6:49 ` [PATCH 00/15] Fifteen multipath-tools patches Christophe Varoqui
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:41 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 15/15] Makefile.inc: Enable -Wextra and -Wformat-nonliteral
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (12 preceding siblings ...)
2016-10-04 17:41 ` [PATCH 14/15] libmultipath: Micro-optimize snprint_size() Bart Van Assche
@ 2016-10-04 17:42 ` Bart Van Assche
2016-10-05 6:49 ` [PATCH 00/15] Fifteen multipath-tools patches Christophe Varoqui
14 siblings, 0 replies; 17+ messages in thread
From: Bart Van Assche @ 2016-10-04 17:42 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 00/15] Fifteen multipath-tools patches
2016-10-04 17:36 [PATCH 00/15] Fifteen multipath-tools patches Bart Van Assche
` (13 preceding siblings ...)
2016-10-04 17:42 ` [PATCH 15/15] Makefile.inc: Enable -Wextra and -Wformat-nonliteral Bart Van Assche
@ 2016-10-05 6:49 ` Christophe Varoqui
14 siblings, 0 replies; 17+ messages in thread
From: Christophe Varoqui @ 2016-10-05 6:49 UTC (permalink / raw)
To: Bart Van Assche; +Cc: device-mapper development
[-- 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 --]
^ permalink raw reply [flat|nested] 17+ messages in thread