From dda119873be898ede40af9007ca0a9e79e3a9796 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 3 Jan 2020 12:32:07 -0700 Subject: [PATCH] Revert "Fix a potential deadlock in helper_do_stat()" This reverts commit 31eca641ad91634e5ffcf369cd756b0506a700c1. Killing this for now as it breaks the Windows build. Signed-off-by: Jens Axboe --- helper_thread.c | 94 +++++++++++++++++++++---------------------------- 1 file changed, 40 insertions(+), 54 deletions(-) diff --git a/helper_thread.c b/helper_thread.c index 49cd4c75..eba2898a 100644 --- a/helper_thread.c +++ b/helper_thread.c @@ -10,52 +10,48 @@ #include "steadystate.h" #include "pshared.h" -enum action { - A_EXIT = 1, - A_RESET = 2, - A_DO_STAT = 3, -}; - static struct helper_data { volatile int exit; - int pipe[2]; /* 0: read end; 1: write end. */ + volatile int reset; + volatile int do_stat; struct sk_out *sk_out; pthread_t thread; + pthread_mutex_t lock; + pthread_cond_t cond; struct fio_sem *startup_sem; } *helper_data; void helper_thread_destroy(void) { - close(helper_data->pipe[0]); - close(helper_data->pipe[1]); + pthread_cond_destroy(&helper_data->cond); + pthread_mutex_destroy(&helper_data->lock); sfree(helper_data); } -static void submit_action(enum action a) +void helper_reset(void) { - const uint8_t data = a; - int ret; - if (!helper_data) return; - ret = write(helper_data->pipe[1], &data, sizeof(data)); - assert(ret == 1); -} + pthread_mutex_lock(&helper_data->lock); -void helper_reset(void) -{ - submit_action(A_RESET); + if (!helper_data->reset) { + helper_data->reset = 1; + pthread_cond_signal(&helper_data->cond); + } + + pthread_mutex_unlock(&helper_data->lock); } -/* - * May be invoked in signal handler context and hence must only call functions - * that are async-signal-safe. See also - * https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03. - */ void helper_do_stat(void) { - submit_action(A_DO_STAT); + if (!helper_data) + return; + + pthread_mutex_lock(&helper_data->lock); + helper_data->do_stat = 1; + pthread_cond_signal(&helper_data->cond); + pthread_mutex_unlock(&helper_data->lock); } bool helper_should_exit(void) @@ -68,9 +64,14 @@ bool helper_should_exit(void) void helper_thread_exit(void) { + void *ret; + + pthread_mutex_lock(&helper_data->lock); helper_data->exit = 1; - submit_action(A_EXIT); - pthread_join(helper_data->thread, NULL); + pthread_cond_signal(&helper_data->cond); + pthread_mutex_unlock(&helper_data->lock); + + pthread_join(helper_data->thread, &ret); } static void *helper_thread_main(void *data) @@ -78,7 +79,6 @@ static void *helper_thread_main(void *data) struct helper_data *hd = data; unsigned int msec_to_next_event, next_log, next_ss = STEADYSTATE_MSEC; struct timespec ts, last_du, last_ss; - uint8_t action; int ret = 0; sk_out_assign(hd->sk_out); @@ -96,23 +96,11 @@ static void *helper_thread_main(void *data) msec_to_next_event = DISK_UTIL_MSEC; while (!ret && !hd->exit) { uint64_t since_du, since_ss = 0; - struct timeval timeout = { - .tv_sec = DISK_UTIL_MSEC / 1000, - .tv_usec = (DISK_UTIL_MSEC % 1000) * 1000, - }; - fd_set rfds, efds; timespec_add_msec(&ts, msec_to_next_event); - if (read(hd->pipe[0], &action, sizeof(action)) < 0) { - FD_ZERO(&rfds); - FD_SET(hd->pipe[0], &rfds); - FD_ZERO(&efds); - FD_SET(hd->pipe[0], &efds); - select(1, &rfds, NULL, &efds, &timeout); - if (read(hd->pipe[0], &action, sizeof(action)) < 0) - action = 0; - } + pthread_mutex_lock(&hd->lock); + pthread_cond_timedwait(&hd->cond, &hd->lock, &ts); #ifdef CONFIG_PTHREAD_CONDATTR_SETCLOCK clock_gettime(CLOCK_MONOTONIC, &ts); @@ -120,11 +108,14 @@ static void *helper_thread_main(void *data) clock_gettime(CLOCK_REALTIME, &ts); #endif - if (action == A_RESET) { - last_du = ts; - last_ss = ts; + if (hd->reset) { + memcpy(&last_du, &ts, sizeof(ts)); + memcpy(&last_ss, &ts, sizeof(ts)); + hd->reset = 0; } + pthread_mutex_unlock(&hd->lock); + since_du = mtime_since(&last_du, &ts); if (since_du >= DISK_UTIL_MSEC || DISK_UTIL_MSEC - since_du < 10) { ret = update_io_ticks(); @@ -135,8 +126,10 @@ static void *helper_thread_main(void *data) } else msec_to_next_event = DISK_UTIL_MSEC - since_du; - if (action == A_DO_STAT) + if (hd->do_stat) { + hd->do_stat = 0; __show_running_run_stats(); + } next_log = calc_log_samples(); if (!next_log) @@ -180,17 +173,10 @@ int helper_thread_create(struct fio_sem *startup_sem, struct sk_out *sk_out) hd->sk_out = sk_out; -#ifdef __linux__ - ret = pipe2(hd->pipe, O_CLOEXEC); -#else - ret = pipe(hd->pipe); -#endif + ret = mutex_cond_init_pshared(&hd->lock, &hd->cond); if (ret) return 1; - ret = fcntl(hd->pipe[0], F_SETFL, O_NONBLOCK); - assert(ret >= 0); - hd->startup_sem = startup_sem; DRD_IGNORE_VAR(helper_data); -- 2.25.1