On 10/3/22 2:34 PM, Vincent Fu wrote: > For randtrimwrite, we decide whether to issue a trim or a write based on > whether the end offsets for the most recent trim and write commands > match. If they don't match that means we just issued a new trim and the > next operation should be a write. If they *do* match that means we just > completed a trim + write pair and the next command should be a trim. > > This works fine for sequential workloads but for random workloads it's > possible to complete a trim + write pair and then have the randomly > generated offset for the next trim command match the previous offset. If > that happens we need to alter the offset for the last write operation in > order to ensure that we issue a write operation the next time through. > > It feels dirty to change the meaning of last_pos[DDIR_WRITE] in this way > but hopefully the long comment in the code will be sufficient warning. > > Signed-off-by: Vincent Fu <vincent.fu@xxxxxxxxxxx> > --- > io_u.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/io_u.c b/io_u.c > index 3f6c60ee..0a70f72a 100644 > --- a/io_u.c > +++ b/io_u.c > @@ -513,6 +513,24 @@ static int get_next_offset(struct thread_data *td, struct io_u *io_u, > return 1; > } > > + /* > + * For randtrimwrite, we decide whether to issue a trim or a write > + * based on whether the offsets for the most recent trim and write > + * operations match. If they don't match that means we just issued a > + * new trim and the next operation should be a write. If they *do* > + * match that means we just completed a trim+write pair and the next > + * command should be a trim. > + * > + * This works fine for sequential workloads but for random workloads > + * it's possible to complete a trim+write pair and then have the next > + * randomly generated offset match the previous offset. If that happens > + * we need to alter the offset for the last write operation in order > + * to ensure that we issue a write operation the next time through. > + */ > + if ((td_randtrimwrite(td) && ddir == DDIR_TRIM) && > + fio_unlikely(f->last_start[DDIR_TRIM] == io_u->offset)) > + f->last_pos[DDIR_WRITE]--; Minor nit - the unlikely bracketing doesn't work well like this, it has to cover the whole branch. For this case, I'd probably just skip it. Outside of that, series looks good to me, thanks a lot for taking a closer look at this! -- Jens Axboe
