Signal td->free_cond with the associated mutex held

Calling pthread_cond_signal() or pthread_cond_broadcast() without
holding the associated mutex can lead to missed wakeups. Hence ensure
that td->io_u_lock is held around pthread_cond_signal(&td->free_cond)
calls. A quote from the POSIX spec
(http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cond_broadcast.html):

"The pthread_cond_broadcast() or pthread_cond_signal() functions may be
called by a thread whether or not it currently owns the mutex that
threads calling pthread_cond_wait() or pthread_cond_timedwait() have
associated with the condition variable during their waits; however, if
predictable scheduling behavior is required, then that mutex shall be
locked by the thread calling pthread_cond_broadcast() or
pthread_cond_signal()."

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>

diff --git a/io_u.c b/io_u.c
index 84d9315512c9d5b4c1b68924f341daa05fbb4d82..f3b593227043626d6978005c35ebec5c5a6472dc 100644 (file)
--- a/io_u.c
+++ b/io_u.c
@@ -856,8 +856,8 @@ void put_io_u(struct thread_data *td, struct io_u *io_u)
                assert(!(td->flags & TD_F_CHILD));
        }
        io_u_qpush(&td->io_u_freelist, io_u);
                assert(!(td->flags & TD_F_CHILD));
        }
        io_u_qpush(&td->io_u_freelist, io_u);

-       td_io_u_unlock(td);

        td_io_u_free_notify(td);
        td_io_u_free_notify(td);

+       td_io_u_unlock(td);

 }
 
 void clear_io_u(struct thread_data *td, struct io_u *io_u)
 }
 
 void clear_io_u(struct thread_data *td, struct io_u *io_u)


@@ -889,8 +889,8 @@ void requeue_io_u(struct thread_data *td, struct io_u **io_u)
        }
 
        io_u_rpush(&td->io_u_requeues, __io_u);
        }
 
        io_u_rpush(&td->io_u_requeues, __io_u);

-       td_io_u_unlock(td);

        td_io_u_free_notify(td);
        td_io_u_free_notify(td);

+       td_io_u_unlock(td);

        *io_u = NULL;
 }
 
        *io_u = NULL;
 }
 

diff --git a/verify.c b/verify.c
index 17af3bb92ab8e0747c532836aa5e9bd1e5ce2812..d10670bb0c328b8ff520ec3e719876b7707ae775 100644 (file)
--- a/verify.c
+++ b/verify.c
@@ -1454,9 +1454,9 @@ static void *verify_async_thread(void *data)
 done:
        pthread_mutex_lock(&td->io_u_lock);
        td->nr_verify_threads--;
 done:
        pthread_mutex_lock(&td->io_u_lock);
        td->nr_verify_threads--;

+       pthread_cond_signal(&td->free_cond);

        pthread_mutex_unlock(&td->io_u_lock);
 
        pthread_mutex_unlock(&td->io_u_lock);
 

-       pthread_cond_signal(&td->free_cond);

        return NULL;
 }
 
        return NULL;
 }
 


@@ -1492,9 +1492,12 @@ int verify_async_init(struct thread_data *td)
 
        if (i != td->o.verify_async) {
                log_err("fio: only %d verify threads started, exiting\n", i);
 
        if (i != td->o.verify_async) {
                log_err("fio: only %d verify threads started, exiting\n", i);

+
+               pthread_mutex_lock(&td->io_u_lock);

                td->verify_thread_exit = 1;
                td->verify_thread_exit = 1;

-               write_barrier();

                pthread_cond_broadcast(&td->verify_cond);
                pthread_cond_broadcast(&td->verify_cond);

+               pthread_mutex_unlock(&td->io_u_lock);
+

                return 1;
        }
 
                return 1;
        }
 


@@ -1503,12 +1506,10 @@ int verify_async_init(struct thread_data *td)
 
 void verify_async_exit(struct thread_data *td)
 {
 
 void verify_async_exit(struct thread_data *td)
 {

+       pthread_mutex_lock(&td->io_u_lock);

        td->verify_thread_exit = 1;
        td->verify_thread_exit = 1;

-       write_barrier();

        pthread_cond_broadcast(&td->verify_cond);
 
        pthread_cond_broadcast(&td->verify_cond);
 

-       pthread_mutex_lock(&td->io_u_lock);
-

        while (td->nr_verify_threads)
                pthread_cond_wait(&td->free_cond, &td->io_u_lock);
 
        while (td->nr_verify_threads)
                pthread_cond_wait(&td->free_cond, &td->io_u_lock);