Recent changes (master)

Jens Axboe
2020-07-05T12:00:08Z

The following changes since commit a504a3902e00b6db0183872b0ff62e35abae7119:

configure: fix bad indentation (2020-07-03 08:34:56 -0600)

are available in the Git repository at:

git://git.kernel.dk/fio.git master

for you to fetch changes up to 430083bf2a3db52bcb374919140d3a978391fbf0:

Merge branch 'windows' of https://github.com/bvanassche/fio (2020-07-04 17:29:32 -0600)

----------------------------------------------------------------
Bart Van Assche (9):
.appveyor.yml: Select the latest software image
t/run-fio-tests.py: Increase IOPS tolerance further
Fix a potentially infinite loop in check_overlap()
Enable error checking for the mutex that serializes overlapping I/O
Add a test for serialize_overlap=1
workqueue: Rework while loop locking
workqueue: Fix race conditions in the workqueue mechanism
os/windows/posix.c: Strip trailing whitespace
os/windows/posix.c: Use INVALID_SOCKET instead of < 0

Jens Axboe (3):
Merge branch 'overlap' of https://github.com/bvanassche/fio
Merge branch 'workqueue' of https://github.com/bvanassche/fio
Merge branch 'windows' of https://github.com/bvanassche/fio

.appveyor.yml | 3 +++
backend.c | 18 ++++++++++-----
ioengines.c | 6 +++--
os/windows/posix.c | 8 +++----
rate-submit.c | 66 ++++++++++++++++++++++++++++--------------------------
t/jobs/t0013.fio | 14 ++++++++++++
t/run-fio-tests.py | 19 +++++++++++++++-
workqueue.c | 50 +++++++++++++++++++++--------------------
8 files changed, 116 insertions(+), 68 deletions(-)
create mode 100644 t/jobs/t0013.fio

---

Diff of recent changes:

diff --git a/.appveyor.yml b/.appveyor.yml
index 70c337f8..5f4a33e2 100644
--- a/.appveyor.yml
+++ b/.appveyor.yml
@@ -1,5 +1,8 @@
clone_depth: 1 # NB: this stops FIO-VERSION-GEN making tag based versions

+image:
+ - Visual Studio 2019
+
environment:
CYG_MIRROR: http://cygwin.mirror.constant.com
CYG_ROOT: C:\cygwin64
diff --git a/backend.c b/backend.c
index 0075a733..0e454cdd 100644
--- a/backend.c
+++ b/backend.c
@@ -66,7 +66,11 @@ unsigned int stat_number = 0;
int shm_id = 0;
int temp_stall_ts;
unsigned long done_secs = 0;
+#ifdef PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
+pthread_mutex_t overlap_check = PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP;
+#else
pthread_mutex_t overlap_check = PTHREAD_MUTEX_INITIALIZER;
+#endif

#define JOB_START_TIMEOUT (5 * 1000)

@@ -1535,7 +1539,7 @@ static void *thread_main(void *data)
uint64_t bytes_done[DDIR_RWDIR_CNT];
int deadlock_loop_cnt;
bool clear_state;
- int ret;
+ int res, ret;

sk_out_assign(sk_out);
free(fd);
@@ -1860,11 +1864,15 @@ static void *thread_main(void *data)
* offload mode so that we don't clean up this job while
* another thread is checking its io_u's for overlap
*/
- if (td_offload_overlap(td))
- pthread_mutex_lock(&overlap_check);
+ if (td_offload_overlap(td)) {
+ int res = pthread_mutex_lock(&overlap_check);
+ assert(res == 0);
+ }
td_set_runstate(td, TD_FINISHING);
- if (td_offload_overlap(td))
- pthread_mutex_unlock(&overlap_check);
+ if (td_offload_overlap(td)) {
+ res = pthread_mutex_unlock(&overlap_check);
+ assert(res == 0);
+ }

update_rusage_stat(td);
td->ts.total_run_time = mtime_since_now(&td->epoch);
diff --git a/ioengines.c b/ioengines.c
index c1b430a1..1c5970a4 100644
--- a/ioengines.c
+++ b/ioengines.c
@@ -313,8 +313,10 @@ enum fio_q_status td_io_queue(struct thread_data *td, struct io_u *io_u)
* started the overlap check because the IO_U_F_FLIGHT
* flag is now set
*/
- if (td_offload_overlap(td))
- pthread_mutex_unlock(&overlap_check);
+ if (td_offload_overlap(td)) {
+ int res = pthread_mutex_unlock(&overlap_check);
+ assert(res == 0);
+ }

assert(fio_file_open(io_u->file));

diff --git a/os/windows/posix.c b/os/windows/posix.c
index e36453e9..31271de0 100644
--- a/os/windows/posix.c
+++ b/os/windows/posix.c
@@ -750,7 +750,7 @@ int setgid(gid_t gid)
int nice(int incr)
{
DWORD prioclass = NORMAL_PRIORITY_CLASS;
-
+
if (incr < -15)
prioclass = HIGH_PRIORITY_CLASS;
else if (incr < 0)
@@ -759,7 +759,7 @@ int nice(int incr)
prioclass = IDLE_PRIORITY_CLASS;
else if (incr > 0)
prioclass = BELOW_NORMAL_PRIORITY_CLASS;
-
+
if (!SetPriorityClass(GetCurrentProcess(), prioclass))
log_err("fio: SetPriorityClass failed\n");

@@ -883,7 +883,7 @@ int poll(struct pollfd fds[], nfds_t nfds, int timeout)
FD_ZERO(&exceptfds);

for (i = 0; i < nfds; i++) {
- if (fds[i].fd < 0) {
+ if (fds[i].fd == INVALID_SOCKET) {
fds[i].revents = 0;
continue;
}
@@ -900,7 +900,7 @@ int poll(struct pollfd fds[], nfds_t nfds, int timeout)

if (rc != SOCKET_ERROR) {
for (i = 0; i < nfds; i++) {
- if (fds[i].fd < 0)
+ if (fds[i].fd == INVALID_SOCKET)
continue;

if ((fds[i].events & POLLIN) && FD_ISSET(fds[i].fd, &readfds))
diff --git a/rate-submit.c b/rate-submit.c
index cf00d9bc..b7b70372 100644
--- a/rate-submit.c
+++ b/rate-submit.c
@@ -4,6 +4,7 @@
* Copyright (C) 2015 Jens Axboe <axboe@kernel.dk>
*
*/
+#include <assert.h>
#include "fio.h"
#include "ioengines.h"
#include "lib/getrusage.h"
@@ -11,40 +12,41 @@

static void check_overlap(struct io_u *io_u)
{
- int i;
+ int i, res;
struct thread_data *td;
- bool overlap = false;

- do {
- /*
- * Allow only one thread to check for overlap at a
- * time to prevent two threads from thinking the coast
- * is clear and then submitting IOs that overlap with
- * each other
- *
- * If an overlap is found, release the lock and
- * re-acquire it before checking again to give other
- * threads a chance to make progress
- *
- * If an overlap is not found, release the lock when the
- * io_u's IO_U_F_FLIGHT flag is set so that this io_u
- * can be checked by other threads as they assess overlap
- */
- pthread_mutex_lock(&overlap_check);
- for_each_td(td, i) {
- if (td->runstate <= TD_SETTING_UP ||
- td->runstate >= TD_FINISHING ||
- !td->o.serialize_overlap ||
- td->o.io_submit_mode != IO_MODE_OFFLOAD)
- continue;
-
- overlap = in_flight_overlap(&td->io_u_all, io_u);
- if (overlap) {
- pthread_mutex_unlock(&overlap_check);
- break;
- }
- }
- } while (overlap);
+ /*
+ * Allow only one thread to check for overlap at a time to prevent two
+ * threads from thinking the coast is clear and then submitting IOs
+ * that overlap with each other.
+ *
+ * If an overlap is found, release the lock and re-acquire it before
+ * checking again to give other threads a chance to make progress.
+ *
+ * If no overlap is found, release the lock when the io_u's
+ * IO_U_F_FLIGHT flag is set so that this io_u can be checked by other
+ * threads as they assess overlap.
+ */
+ res = pthread_mutex_lock(&overlap_check);
+ assert(res == 0);
+
+retry:
+ for_each_td(td, i) {
+ if (td->runstate <= TD_SETTING_UP ||
+ td->runstate >= TD_FINISHING ||
+ !td->o.serialize_overlap ||
+ td->o.io_submit_mode != IO_MODE_OFFLOAD)
+ continue;
+
+ if (!in_flight_overlap(&td->io_u_all, io_u))
+ continue;
+
+ res = pthread_mutex_unlock(&overlap_check);
+ assert(res == 0);
+ res = pthread_mutex_lock(&overlap_check);
+ assert(res == 0);
+ goto retry;
+ }
}

static int io_workqueue_fn(struct submit_worker *sw,
diff --git a/t/jobs/t0013.fio b/t/jobs/t0013.fio
new file mode 100644
index 00000000..b4ec1b42
--- /dev/null
+++ b/t/jobs/t0013.fio
@@ -0,0 +1,14 @@
+# Trigger the fio code that serializes overlapping I/O. The job size is very
+# small to make overlapping I/O more likely.
+
+[test]
+ioengine=null
+thread=1
+size=4K
+blocksize=4K
+io_submit_mode=offload
+iodepth=1
+serialize_overlap=1
+numjobs=8
+loops=1000000
+runtime=10
diff --git a/t/run-fio-tests.py b/t/run-fio-tests.py
index ae2cb096..c116bf5a 100755
--- a/t/run-fio-tests.py
+++ b/t/run-fio-tests.py
@@ -438,7 +438,7 @@ class FioJobTest_iops_rate(FioJobTest):
logging.debug("Test %d: iops1: %f", self.testnum, iops1)
logging.debug("Test %d: ratio: %f", self.testnum, ratio)

- if iops1 < 995 or iops1 > 1005:
+ if iops1 < 950 or iops1 > 1050:
self.failure_reason = "{0} iops value mismatch,".format(self.failure_reason)
self.passed = False

@@ -447,6 +447,13 @@ class FioJobTest_iops_rate(FioJobTest):
self.passed = False

+class FioJobTest_t0013(FioJobTest):
+ """Runs fio test job t0013"""
+
+ def check_result(self):
+ super(FioJobTest_t0013, self).check_result()
+
+
class Requirements(object):
"""Requirements consists of multiple run environment characteristics.
These are to determine if a particular test can be run"""
@@ -687,6 +694,16 @@ TEST_LIST = [
'requirements': [Requirements.not_macos],
# mac os does not support CPU affinity
},
+ {
+ 'test_id': 13,
+ 'test_class': FioJobTest_t0013,
+ 'job': 't0013.fio',
+ 'success': SUCCESS_DEFAULT,
+ 'pre_job': None,
+ 'pre_success': None,
+ 'output_format': 'json',
+ 'requirements': [],
+ },
{
'test_id': 1000,
'test_class': FioExeTest,
diff --git a/workqueue.c b/workqueue.c
index b5959512..9e6c41ff 100644
--- a/workqueue.c
+++ b/workqueue.c
@@ -85,15 +85,14 @@ static bool all_sw_idle(struct workqueue *wq)
*/
void workqueue_flush(struct workqueue *wq)
{
+ pthread_mutex_lock(&wq->flush_lock);
wq->wake_idle = 1;

- while (!all_sw_idle(wq)) {
- pthread_mutex_lock(&wq->flush_lock);
+ while (!all_sw_idle(wq))
pthread_cond_wait(&wq->flush_cond, &wq->flush_lock);
- pthread_mutex_unlock(&wq->flush_lock);
- }

wq->wake_idle = 0;
+ pthread_mutex_unlock(&wq->flush_lock);
}

/*
@@ -159,12 +158,10 @@ static void *worker_thread(void *data)
if (sw->flags & SW_F_ERROR)
goto done;

+ pthread_mutex_lock(&sw->lock);
while (1) {
- pthread_mutex_lock(&sw->lock);
-
if (flist_empty(&sw->work_list)) {
if (sw->flags & SW_F_EXIT) {
- pthread_mutex_unlock(&sw->lock);
break;
}

@@ -173,34 +170,41 @@ static void *worker_thread(void *data)
workqueue_pre_sleep(sw);
pthread_mutex_lock(&sw->lock);
}
-
- /*
- * We dropped and reaquired the lock, check
- * state again.
- */
- if (!flist_empty(&sw->work_list))
- goto handle_work;
-
+ }
+ /*
+ * We may have dropped and reaquired the lock, check state
+ * again.
+ */
+ if (flist_empty(&sw->work_list)) {
if (sw->flags & SW_F_EXIT) {
- pthread_mutex_unlock(&sw->lock);
break;
- } else if (!(sw->flags & SW_F_IDLE)) {
+ }
+ if (!(sw->flags & SW_F_IDLE)) {
sw->flags |= SW_F_IDLE;
wq->next_free_worker = sw->index;
+ pthread_mutex_unlock(&sw->lock);
+ pthread_mutex_lock(&wq->flush_lock);
if (wq->wake_idle)
pthread_cond_signal(&wq->flush_cond);
+ pthread_mutex_unlock(&wq->flush_lock);
+ pthread_mutex_lock(&sw->lock);
+ }
+ }
+ if (flist_empty(&sw->work_list)) {
+ if (sw->flags & SW_F_EXIT) {
+ break;
}
-
pthread_cond_wait(&sw->cond, &sw->lock);
} else {
-handle_work:
flist_splice_init(&sw->work_list, &local_list);
}
pthread_mutex_unlock(&sw->lock);
handle_list(sw, &local_list);
if (wq->ops.update_acct_fn)
wq->ops.update_acct_fn(sw);
+ pthread_mutex_lock(&sw->lock);
}
+ pthread_mutex_unlock(&sw->lock);

done:
sk_out_drop();
@@ -336,11 +340,11 @@ int workqueue_init(struct thread_data *td, struct workqueue *wq,
* Wait for them all to be started and initialized
*/
error = 0;
+ pthread_mutex_lock(&wq->flush_lock);
do {
struct submit_worker *sw;

running = 0;
- pthread_mutex_lock(&wq->flush_lock);
for (i = 0; i < wq->max_workers; i++) {
sw = &wq->workers[i];
pthread_mutex_lock(&sw->lock);
@@ -351,14 +355,12 @@ int workqueue_init(struct thread_data *td, struct workqueue *wq,
pthread_mutex_unlock(&sw->lock);
}

- if (error || running == wq->max_workers) {
- pthread_mutex_unlock(&wq->flush_lock);
+ if (error || running == wq->max_workers)
break;
- }

pthread_cond_wait(&wq->flush_cond, &wq->flush_lock);
- pthread_mutex_unlock(&wq->flush_lock);
} while (1);
+ pthread_mutex_unlock(&wq->flush_lock);

if (!error)
return 0;