Fix for a race when fio prints I/O statistics periodically
authorVasily Tarasov <tarasov@vasily.name>
Mon, 10 Nov 2014 03:22:24 +0000 (20:22 -0700)
committerJens Axboe <axboe@fb.com>
Mon, 10 Nov 2014 03:24:14 +0000 (20:24 -0700)
commite5437a073e658e8154b9e87bab5c7b3b06ed4255
treec8eb238cfc1a5a9e4c2b2e199b79c732563d851b
parent988d97bac90c9ff3ee40a8ccd588fdf78f19320d
Fix for a race when fio prints I/O statistics periodically

Below is the demonstration for the latest code in git:

Job-file:

[job]
filename=/dev/sda
rw=randread
blocksize=4096
direct=1
time_based=1
runtime=300
numjobs=16

Command line:

fio --output-format=json --status-interval=10  /tmp/job.file | tee /tmp/fio.out

After parsing json and printing per-job "read->runtime" and "read->iops" in two
columns, one can see this:

runtime iops
9854 4762
19861 4724
29867 4711
39873 4705
49880 4701
59886 4698
69894 4696
79900 4694
89907 4693
99913 4692
109919 4692
119926 4691
129933 4691
139940 4691
149946 4691
159953 4690
169959 4690
179966 4690
189972 4690
199978 4690
209984 4690
219990 4690
229996 4690
240002 4690
250009 4690
260015 4690
270021 4690
280027 4690
290033 4690
600043 2344 <--- You can see a spurious jump in runtime here
300001 4690

The problem is two-fold, IMHO:

1) td->ts.runtime[DDIR_READ/DDIR_WRITE/DDIR_TRIM] are set in microseconds
   instead of milliseconds in thread_main() function. The values are later
   converted to milliseconds (after exiting from the loop in thread_main()),
   but it is too late for the out-of-band access by the helper_thread
   (the one that prints periodic statistics).

2) The helper_thread in __show_running_run_stats() temporarily adds to
   td->ts.runtime[DDIR_READ/DDIR_WRITE/DDIR_TRIM]
   the time that has passed since the beginning of the loop (td->start).
   However, the same addition happens in thread_main() by the job
   thread and td->start is not reset atomically with this addition.
   As a result, the runtime can be added twice. (That's why in the output
   we see 600043, which is approximately twice larger than the
   previous reading).

My proposal is to make the job threads updating both td->ts.runtime and
td->start atomically by using an already existing stat_mutex.
__show_running_run_stats() already takes this mutex where needed.

Signed-off-by: Jens Axboe <axboe@fb.com>
backend.c
stat.c
stat.h