multipathd: Remove a busy-waiting loop
authorBart Van Assche <bart.vanassche@sandisk.com>
Mon, 15 Aug 2016 15:28:29 +0000 (08:28 -0700)
committerChristophe Varoqui <christophe.varoqui@opensvc.com>
Tue, 16 Aug 2016 07:02:10 +0000 (09:02 +0200)
Use pthread_join() to wait until worker threads have finished
instead of using a counter to keep track of how many threads are
trying to grab a mutex. Remove mutex_lock.depth since this member
variable is no longer needed.

This patch fixes two race conditions:
* Incrementing "depth" in lock() without holding a mutex.
* Destroying a mutex from the main thread without waiting for the
  worker threads to finish using that mutex.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
libmultipath/lock.h
multipathd/main.c

index 9808480..a170efe 100644 (file)
@@ -3,35 +3,22 @@
 
 #include <pthread.h>
 
-/*
- * Wrapper for the mutex. Includes a ref-count to keep
- * track of how many there are out-standing threads blocking
- * on a mutex. */
 struct mutex_lock {
        pthread_mutex_t mutex;
-       int depth;
 };
 
 static inline void lock(struct mutex_lock *a)
 {
-       a->depth++;
        pthread_mutex_lock(&a->mutex);
 }
 
 static inline int timedlock(struct mutex_lock *a, struct timespec *tmo)
 {
-       int r;
-
-       a->depth++;
-       r = pthread_mutex_timedlock(&a->mutex, tmo);
-       if (r)
-               a->depth--;
-       return r;
+       return pthread_mutex_timedlock(&a->mutex, tmo);
 }
 
 static inline void unlock(struct mutex_lock *a)
 {
-       a->depth--;
        pthread_mutex_unlock(&a->mutex);
 }
 
index 5dff07c..001eb8c 100644 (file)
@@ -2048,7 +2048,6 @@ init_vecs (void)
                return NULL;
 
        pthread_mutex_init(&vecs->lock.mutex, NULL);
-       vecs->lock.depth = 0;
 
        return vecs;
 }
@@ -2396,16 +2395,16 @@ child (void * param)
        pthread_cancel(uxlsnr_thr);
        pthread_cancel(uevq_thr);
 
+       pthread_join(check_thr, NULL);
+       pthread_join(uevent_thr, NULL);
+       pthread_join(uxlsnr_thr, NULL);
+       pthread_join(uevq_thr, NULL);
+
        lock(&vecs->lock);
        free_pathvec(vecs->pathvec, FREE_PATHS);
        vecs->pathvec = NULL;
        unlock(&vecs->lock);
-       /* Now all the waitevent threads will start rushing in. */
-       while (vecs->lock.depth > 0) {
-               sleep (1); /* This is weak. */
-               condlog(3, "Have %d wait event checkers threads to de-alloc,"
-                       " waiting...", vecs->lock.depth);
-       }
+
        pthread_mutex_destroy(&vecs->lock.mutex);
        FREE(vecs);
        vecs = NULL;