On Nov 01, 2022 / 16:08, Vincent Fu wrote: > > -----Original Message----- > > From: Shin'ichiro Kawasaki [mailto:shinichiro.kawasaki@xxxxxxx] > > Sent: Friday, October 28, 2022 7:54 AM > > To: fio@xxxxxxxxxxxxxxx; Jens Axboe <axboe@xxxxxxxxx>; Vincent Fu > > <vincentfu@xxxxxxxxx> > > Cc: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>; Dmitry > > Fomichev <Dmitry.Fomichev@xxxxxxx>; Niklas Cassel > > <niklas.cassel@xxxxxxx>; Shin'ichiro Kawasaki > > <shinichiro.kawasaki@xxxxxxx> > > Subject: [PATCH v3 06/13] zbd: verify before zone reset for > > zone_reset_threshold option > > > > When verify option is specified together with zonemode=zbd and > > zone_reset_threshold option, zone reset happens to zones not full. This > > erases data for verify and causes verify failure. Current implementation > > avoids this scenario by assert. > > > > To allow zone_reset_threshold option together with zonemode=zbd and > > verify option, do verify before the zone reset. When zone reset is > > required to an open zone with verify data, call get_next_verify() to > > get verify read unit and return it from zbd_adjust_block(). > > > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> > > Tested-by: Dmitry Fomichev <dmitry.fomichev@xxxxxxx> > > Reviewed-by: Dmitry Fomichev <dmitry.fomichev@xxxxxxx> > > --- > > zbd.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/zbd.c b/zbd.c > > index 6f4e5ab2..af761c17 100644 > > --- a/zbd.c > > +++ b/zbd.c > > @@ -2032,7 +2032,10 @@ retry: > > > > /* Reset the zone pointer if necessary */ > > if (zb->reset_zone || zbd_zone_full(f, zb, min_bs)) { > > - assert(td->o.verify == VERIFY_NONE); > > + if (td->o.verify != VERIFY_NONE && > > + !get_next_verify(td, io_u)) > > + goto accept; > > + > > /* > > * Since previous write requests may have been > > submitted > > * asynchronously and since we will submit the > > zone > > -- > > 2.37.1 > > The new test case 59 assesses the above functionality with a sequential write > workload. But when I try a random write workload, it seems like randomly > generated write offsets may no longer be changed to a zone write pointer. > > Am I missing something? No, you caught a bug. Thank you! I noticed that get_next_verify() does nothing to io_u when io-u>file is set. So the hunk above does not return verify read io_u from zbd_adjust_block(). Just returns the original write io_u without adjustment to write pointer. Hence the weird symptom you observed. (I checked verify debug option output, but overlooked the returned io_u was not read but write. My bad.) I modified the hunk to unset io_u->file before calling get_next_verify(), then observed the verify read happens as expected. I also found "goto accept" in the hunk was not good either, since it does not unlock the current selected zone. The hunk [1] should work good. On top of the workloads write and randwrite, I also tried rw and randrw. They reported another failure symptom "rand_seed verify check failure". It happened because random seed was not reset for the verify read when zone_reset_threshold option is specified since all write operations have not yet completed. This is same story as verify_backlog option. I created another hunk [2] which skips the rand_seed check in same manner as verify_backlog. It avoided the failure. With the changes [1] and [2], the zone_reset_threshold option looks working good with verify. After some more testing, I will post v4 with the changes. I will also improve the test case #59 to cover the various workloads, write, randwrite, rw and randrw. [1] diff --git a/zbd.c b/zbd.c index 6f4e5ab2..fadeb458 100644 --- a/zbd.c +++ b/zbd.c @@ -2032,7 +2032,19 @@ retry: /* Reset the zone pointer if necessary */ if (zb->reset_zone || zbd_zone_full(f, zb, min_bs)) { - assert(td->o.verify == VERIFY_NONE); + if (td->o.verify != VERIFY_NONE) { + /* + * Unset io-u->file to tell get_next_verify() + * that this IO is not requeue. + */ + io_u->file = NULL; + if (!get_next_verify(td, io_u)) { + zone_unlock(zb); + return io_u_accept; + } + io_u->file = f; + } + /* * Since previous write requests may have been submitted * asynchronously and since we will submit the zone [2] diff --git a/verify.c b/verify.c index d6a229ca..ddfadcc8 100644 --- a/verify.c +++ b/verify.c @@ -917,9 +917,11 @@ int verify_io_u(struct thread_data *td, struct io_u **io_u_ptr) hdr = p; /* - * Make rand_seed check pass when have verify_backlog. + * Make rand_seed check pass when have verify_backlog or + * zone reset frequency for zonemode=zbd. */ - if (!td_rw(td) || (td->flags & TD_F_VER_BACKLOG)) + if (!td_rw(td) || (td->flags & TD_F_VER_BACKLOG) || + td->o.zrf.u.f) io_u->rand_seed = hdr->rand_seed; if (td->o.verify != VERIFY_PATTERN_NO_HDR) { -- Shin'ichiro Kawasaki
