On Thu, Jun 08, 2023 at 05:48:41AM +0000, Shinichiro Kawasaki wrote:
> On Jun 07, 2023 / 13:15, Niklas Cassel wrote:
> > On Wed, Jun 07, 2023 at 05:32:27PM +0900, Shin'ichiro Kawasaki wrote:
(snip)
> > > @@ -404,9 +431,6 @@ static int zbd_reset_zones(struct thread_data *td, struct fio_file *f,
> > > continue;
> > >
> > > zone_lock(td, f, z);
> > > - pthread_mutex_lock(&f->zbd_info->mutex);
> > > - zbd_write_zone_put(td, f, z);
> > > - pthread_mutex_unlock(&f->zbd_info->mutex);
> > >
> > > if (z->wp != z->start) {
> > > dprint(FD_ZBD, "%s: resetting zone %u\n",
> > > @@ -2048,7 +2072,7 @@ retry:
> > > */
> > > io_u_quiesce(td);
> > > zb->reset_zone = 0;
> > > - if (zbd_reset_zone(td, f, zb) < 0)
> > > + if (__zbd_reset_zone(td, f, zb) < 0)
> >
> > Since you remove the zbd_write_zone_put() call, which I think is good,
> > I think that you should continue to call zbd_reset_zone() here.
>
> The two hunks above are difficult for review. The 1st hunk which removes the
> zbd_write_zone_put() is a change for zbd_reset_zones(). zbd_reset_zones() no
> longer needs to call zbd_write_zone_put(), since zbd_reset_zone() (no s) calls
> zbd_write_zone_put().
>
> The 2nd hunk is a change in zbd_adjust_block(). The zone reset here is done
> just before the write command issue to the zone, so zbd_write_zone_put() should
> not be called. Then zbd_reset_zone() should be replaced with __zbd_reset_zone().
You are absolutely correct!
I confused the hunk at:
@@ retry:
For being for zbd_reset_zones(), but it really is for zbd_adjust_block().
(And zbd_reset_zones() still calls zbd_reset_zone() as it should.)
I really hate the patch format for using labels instead the actual function
name... Especially since you can have the same label in different functions.
Sorry for the noise.
Kind regards,
Niklas
