On 2012-03-16 18:46, Daniel Ehrenberg wrote: > On Fri, Mar 16, 2012 at 3:07 AM, Jens Axboe <axboe@xxxxxxxxx> wrote: >> On 03/16/2012 04:12 AM, Dan Ehrenberg wrote: >>> Previously, when fio had written a volume of I/O equal to the size >>> argument, it restarted the main do_io loop. >>> >>> This patch allows time_based tests to be run for longer than one >>> cycle in the do_io main loop. This has a couple of advantages: >>> * The random number generator is not reset on each iteration >>> of the loop, so running longer will reach different locations. >>> * There is not a throughput-reducing point where all operations >>> must be reaped before new operations are submitted. >>> >>> The implementation consists of two minor changes: >>> * In the do_io loop, a time_based test will not exit the loop for >>> reading or writing too much data. >>> * When reading or writing sequentially, the operations wrap around >>> to the beginning after reading the end within the >>> get_next_seq_block function. >> >> This looks good, but one question - does it really behave with random >> IO, when the random map is enabled? I set write_iolog and looked at the >> patterns. From the beginning: >> >> foo.1.0 add >> foo.1.0 open >> foo.1.0 read 8093696 4096 >> foo.1.0 read 99356672 4096 >> foo.1.0 read 113164288 4096 >> [...] >> foo.1.0 close >> foo.1.0 open >> foo.1.0 read 8093696 4096 >> foo.1.0 read 99356672 4096 >> foo.1.0 read 113164288 4096 >> [...] >> >> etc. So it's definitely repeating the same sequence there. We don't want >> to close/open the file for this case either, how does the below >> look/work for you? It's your patch, and an update in >> get_next_rand_block() to handle this case too. >> >> diff --git a/backend.c b/backend.c >> index 7343286..1d9b0a2 100644 >> --- a/backend.c >> +++ b/backend.c >> @@ -555,7 +555,8 @@ static void do_io(struct thread_data *td) >> td_set_runstate(td, TD_RUNNING); >> >> while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) || >> - (!flist_empty(&td->trim_list)) || !io_bytes_exceeded(td)) { >> + (!flist_empty(&td->trim_list)) || !io_bytes_exceeded(td) || >> + td->o.time_based) { >> struct timeval comp_time; >> unsigned long bytes_done[2] = { 0, 0 }; >> int min_evts = 0; >> diff --git a/io_u.c b/io_u.c >> index 20794c3..3bda0e6 100644 >> --- a/io_u.c >> +++ b/io_u.c >> @@ -238,13 +238,18 @@ ret: >> static int get_next_rand_block(struct thread_data *td, struct fio_file *f, >> enum fio_ddir ddir, unsigned long long *b) >> { >> - if (get_next_rand_offset(td, f, ddir, b)) { >> - dprint(FD_IO, "%s: rand offset failed, last=%llu, size=%llu\n", >> - f->file_name, f->last_pos, f->real_file_size); >> - return 1; >> + if (!get_next_rand_offset(td, f, ddir, b)) >> + return 0; >> + >> + if (td->o.time_based) { >> + fio_file_reset(f); >> + if (!get_next_rand_offset(td, f, ddir, b)) >> + return 0; >> } >> >> - return 0; >> + dprint(FD_IO, "%s: rand offset failed, last=%llu, size=%llu\n", >> + f->file_name, f->last_pos, f->real_file_size); >> + return 1; >> } >> >> static int get_next_seq_block(struct thread_data *td, struct fio_file *f, >> @@ -252,6 +257,9 @@ static int get_next_seq_block(struct thread_data *td, struct fio_file *f, >> { >> assert(ddir_rw(ddir)); >> >> + if (f->last_pos >= f->io_size && td->o.time_based) >> + f->last_pos = f->last_pos - f->io_size; >> + >> if (f->last_pos < f->real_file_size) { >> unsigned long long pos; >> >> >> >> -- >> Jens Axboe >> > > Thanks for the correction, Jens. I only tested my code with > norandommap, forgetting about the default case. Your modified patch > seems to fix the issue in a nice simple way. I would be happy to see > this new patch committed. Thanks for confirming, I will commit the above variant. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe fio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html