On Wed, Feb 05 2014, Grant Grundler wrote:
> On Wed, Feb 5, 2014 at 11:30 AM, Jens Axboe <axboe@xxxxxxxxx> wrote:
> ...
> > The below should take care of it. Juan's fix didn't account for verify
> > exceeding size=.
>
> Ah...Juan didn't test with that option and Puthikorn did.
>
> > diff --git a/backend.c b/backend.c
> > index bf9d066e012a..3ac72e771cb9 100644
> > --- a/backend.c
> > +++ b/backend.c
> > @@ -642,7 +642,7 @@ static uint64_t do_io(struct thread_data *td)
> > uint64_t bytes_done[DDIR_RWDIR_CNT] = { 0, 0, 0 };
> > unsigned int i;
> > int ret = 0;
> > - uint64_t bytes_issued = 0;
> > + uint64_t total_bytes, bytes_issued = 0;
> >
> > if (in_ramp_time(td))
> > td_set_runstate(td, TD_RAMP);
> > @@ -651,6 +651,10 @@ static uint64_t do_io(struct thread_data *td)
> >
> > lat_target_init(td);
> >
> > + total_bytes = td->o.size;
> > + if (td->o.verify != VERIFY_NONE && td_write(td))
> > + total_bytes += td->o.size;
> > +
>
> The amount of verify IO depends on what percentage is write traffic.
> Is each thread strictly reader or writer?
It does indeed. Note I updated this to depend on verify_backlog as well,
since that is the important bit. Each thread can have any mix of
read/write/trim.
> I didn't think so or we wouldn't need "td->this_io_bytes[]" for
> DDIR_WRITE, DDIR_READ, and DDIR_TRIM at the same time.
>
> > while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
> > (!flist_empty(&td->trim_list)) || !io_bytes_exceeded(td) ||
>
> I've gotten a bit hung up on what io_bytes_exceeded() uses (
> td->this_io_bytes[]), bytes_done[], and bytes_issued. Seems like we
> have three different mechanisms to do essentially the same thing. Can
> I rip two of them out and just count "issued IO" for WRITE/READ/TRIM?
this_io_bytes[] is per loop.
>
> "io_bytes_exceeded()" is the only place I would expect this logic to
> live in this loop. Having additional "break" in the middle of the loop
> implies we've broken the original design.
The fix is a bit of a hack, but the original commit was a hack too. It
would be a lot better to track this explicitly. For now it's just an
upper cap, normally we expect other conditions to terminate it.
So I'd welcome a cleanup in this area for sure.
--
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
