From 00615bfb4a5903b87220bd1d8c18f9b6515bdae0 Mon Sep 17 00:00:00 2001 From: Sitsofe Wheeler Date: Sun, 6 Oct 2019 08:58:28 +0100 Subject: [PATCH] backend: fix final fsync behaviour Previously, fsync_on_close's "final" fsync was done after fio's accounting had finished thus causing the bandwidth reported to be too high (due to the reported time being too low). An example job that demonstrates the issue (on machines with a few gigabytes of RAM and non-fast disks) is the following: ./fio --gtod_reduce=1 --filename=fio.tmp --size=5G --bs=2M --rw=write \ --stonewall --name=end_fsync --end_fsync=1 --name=fsync_on_close \ --fsync_on_close=1 end_fsync: (g=0): rw=write, bs=(R) 2048KiB-2048KiB, (W) 2048KiB-2048KiB, (T) 2048KiB-2048KiB, ioengine=psync, iodepth=1 fsync_on_close: (g=1): rw=write, bs=(R) 2048KiB-2048KiB, (W) 2048KiB-2048KiB, (T) 2048KiB-2048KiB, ioengine=psync, iodepth=1 [...] Run status group 0 (all jobs): WRITE: bw=381MiB/s (400MB/s), 381MiB/s-381MiB/s (400MB/s-400MB/s), io=5120MiB (5369MB), run=13424-13424msec Run status group 1 (all jobs): WRITE: bw=1726MiB/s (1810MB/s), 1726MiB/s-1726MiB/s (1810MB/s-1810MB/s), io=5120MiB (5369MB), run=2966-2966msec This patch fixes the issue by doing an fsync for fsync_on_close at the same point as it does for end_fsync. Technically fsync_on_close will go and do another fsync after this point too but this should be harmless as there is no new write data. This patch also fixes the assert seen with the following job ./fio --size=8k --io_limit=32k --filename=fio.tmp --rw=write \ --end_fsync=1 --name=extended_end_fsync [...] fio: filesetup.c:1682: void get_file(struct fio_file *): Assertion `fio_file_open(f)' failed. by preventing auto-close on last I/O when it is being submitted as a final fsync (due to end_fsync, fsync_on_close). Finally it also fixes the strange IO depths seen with when using end_fsync: ./fio --size=4k --filename=fio.tmp --rw=write --end_fsync=1 \ --name=end_fsync_test end_fsync_test: (g=0): rw=write, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T) 4096B-4096B, ioengine=psync, iodepth=1 [...] IO depths : 1=200.0%, 2=0.0%, 4=0.0%, 8=0.0%, 16=0.0%, 32=0.0%, >=64=0.0% by no longer including such fsyncs in accounting. Fixes: https://github.com/axboe/fio/issues/831 Signed-off-by: Sitsofe Wheeler --- backend.c | 11 +++++++---- ioengines.c | 6 ++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/backend.c b/backend.c index 2f463293..fe868271 100644 --- a/backend.c +++ b/backend.c @@ -281,6 +281,7 @@ static bool fio_io_sync(struct thread_data *td, struct fio_file *f) io_u->ddir = DDIR_SYNC; io_u->file = f; + io_u_set(td, io_u, IO_U_F_NO_FILE_PUT); if (td_io_prep(td, io_u)) { put_io_u(td, io_u); @@ -314,7 +315,7 @@ requeue: static int fio_file_fsync(struct thread_data *td, struct fio_file *f) { - int ret; + int ret, ret2; if (fio_file_open(f)) return fio_io_sync(td, f); @@ -323,8 +324,10 @@ static int fio_file_fsync(struct thread_data *td, struct fio_file *f) return 1; ret = fio_io_sync(td, f); - td_io_close_file(td, f); - return ret; + ret2 = 0; + if (fio_file_open(f)) + ret2 = td_io_close_file(td, f); + return (ret || ret2); } static inline void __update_ts_cache(struct thread_data *td) @@ -1124,7 +1127,7 @@ reap: td->error = 0; } - if (should_fsync(td) && td->o.end_fsync) { + if (should_fsync(td) && (td->o.end_fsync || td->o.fsync_on_close)) { td_set_runstate(td, TD_FSYNCING); for_each_file(td, f, i) { diff --git a/ioengines.c b/ioengines.c index 40fa75c3..9e3fcc9f 100644 --- a/ioengines.c +++ b/ioengines.c @@ -376,14 +376,16 @@ enum fio_q_status td_io_queue(struct thread_data *td, struct io_u *io_u) } if (ret == FIO_Q_COMPLETED) { - if (ddir_rw(io_u->ddir) || ddir_sync(io_u->ddir)) { + if (ddir_rw(io_u->ddir) || + (ddir_sync(io_u->ddir) && td->runstate != TD_FSYNCING)) { io_u_mark_depth(td, 1); td->ts.total_io_u[io_u->ddir]++; } } else if (ret == FIO_Q_QUEUED) { td->io_u_queued++; - if (ddir_rw(io_u->ddir) || ddir_sync(io_u->ddir)) + if (ddir_rw(io_u->ddir) || + (ddir_sync(io_u->ddir) && td->runstate != TD_FSYNCING)) td->ts.total_io_u[io_u->ddir]++; if (td->io_u_queued >= td->o.iodepth_batch) -- 2.25.1