From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Thu, 26 Jul 2018 17:58:11 -0500 Subject: [PATCH] libmultipath: cleanup tur locking There are only three variables whose access needs to be synchronized between the tur thread and the path checker itself: state, message, and active. The rest of the variables are either only written when the tur thread isn't running, or they aren't accessed by the tur thread, or they are atomics that are themselves used to synchronize things. This patch limits the amount of code that is covered by ct->lock to only what needs to be locked. It also makes ct->lock no longer a recursive lock. To make this simpler, tur_thread now only sets the state and message one time, instead of twice, since PATH_UNCHECKED was never able to be returned anyway. One benefit of this is that the tur checker thread gets more time to call tur_check() and return before libcheck_check() gives up and return PATH_PENDING. Signed-off-by: Benjamin Marzinski --- libmultipath/checkers/tur.c | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c index 0c7b5ca..983ba4c 100644 --- a/libmultipath/checkers/tur.c +++ b/libmultipath/checkers/tur.c @@ -53,7 +53,6 @@ struct tur_checker_context { int libcheck_init (struct checker * c) { struct tur_checker_context *ct; - pthread_mutexattr_t attr; struct stat sb; ct = malloc(sizeof(struct tur_checker_context)); @@ -65,10 +64,7 @@ int libcheck_init (struct checker * c) ct->fd = -1; uatomic_set(&ct->holders, 1); pthread_cond_init_mono(&ct->active); - pthread_mutexattr_init(&attr); - pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&ct->lock, &attr); - pthread_mutexattr_destroy(&attr); + pthread_mutex_init(&ct->lock, NULL); if (fstat(c->fd, &sb) == 0) ct->devt = sb.st_rdev; c->context = ct; @@ -213,12 +209,6 @@ static void *tur_thread(void *ctx) condlog(3, "%d:%d : tur checker starting up", major(ct->devt), minor(ct->devt)); - /* 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, msg); pthread_testcancel(); @@ -283,13 +273,6 @@ int libcheck_check(struct checker * c) /* * Async mode */ - r = pthread_mutex_lock(&ct->lock); - if (r != 0) { - condlog(2, "%s: tur mutex lock failed with %d", ct->devt, r); - MSG(c, MSG_TUR_FAILED); - return PATH_WILD; - } - if (ct->thread) { if (tur_check_async_timeout(c)) { int running = uatomic_xchg(&ct->running, 0); @@ -307,21 +290,24 @@ int libcheck_check(struct checker * c) } else { /* TUR checker done */ ct->thread = 0; + pthread_mutex_lock(&ct->lock); tur_status = ct->state; strlcpy(c->message, ct->message, sizeof(c->message)); + pthread_mutex_unlock(&ct->lock); } - pthread_mutex_unlock(&ct->lock); } else { if (uatomic_read(&ct->holders) > 1) { /* The thread has been cancelled but hasn't * quilt. Fail back to synchronous mode */ - pthread_mutex_unlock(&ct->lock); condlog(3, "%d:%d : tur checker failing back to sync", major(ct->devt), minor(ct->devt)); return tur_check(c->fd, c->timeout, c->message); } /* Start new TUR checker */ - ct->state = PATH_UNCHECKED; + pthread_mutex_lock(&ct->lock); + tur_status = ct->state = PATH_PENDING; + ct->message[0] = '\0'; + pthread_mutex_unlock(&ct->lock); ct->fd = c->fd; ct->timeout = c->timeout; uatomic_add(&ct->holders, 1); @@ -334,21 +320,23 @@ int libcheck_check(struct checker * c) uatomic_sub(&ct->holders, 1); uatomic_set(&ct->running, 0); ct->thread = 0; - pthread_mutex_unlock(&ct->lock); condlog(3, "%d:%d : failed to start tur thread, using" " sync mode", major(ct->devt), minor(ct->devt)); return tur_check(c->fd, c->timeout, c->message); } tur_timeout(&tsp); - r = pthread_cond_timedwait(&ct->active, &ct->lock, &tsp); - tur_status = ct->state; - strlcpy(c->message, ct->message, sizeof(c->message)); + pthread_mutex_lock(&ct->lock); + if (ct->state == PATH_PENDING) + r = pthread_cond_timedwait(&ct->active, &ct->lock, + &tsp); + if (!r) { + tur_status = ct->state; + strlcpy(c->message, ct->message, sizeof(c->message)); + } pthread_mutex_unlock(&ct->lock); - if (uatomic_read(&ct->running) != 0 && - (tur_status == PATH_PENDING || tur_status == PATH_UNCHECKED)) { + if (tur_status == PATH_PENDING) { condlog(3, "%d:%d : tur checker still running", major(ct->devt), minor(ct->devt)); - tur_status = PATH_PENDING; } else { int running = uatomic_xchg(&ct->running, 0); if (running) -- 2.7.4