Re: [PATCH 1/4] zbd: move zone finish operation to zbd_convert_to_write_zone()

On Apr 14, 2025 / 09:29, Damien Le Moal wrote:
> On 4/11/25 8:12 PM, Shin'ichiro Kawasaki wrote:
> > Currently, when a write target zone has fewer remainder sectors than
> > the block size, fio finishes the zone to make the zone inactive (not
> > open), so that another zone can be open and used as a write target zone.
> > This zone finish operation is implemented in zbd_adjust_block().
> > However, this placement is less ideal because zbd_adjust_block() manages
> > not just write requests but also read and trim requests.
> > 
> > Since the zone finish operation is exclusively necessary for write
> > requests, implement it into zbd_convert_to_write_zone(). While at it,
> > improve the function comment.
> > 
> > Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
> 
> A few nits below. But otherwise, nice cleanup !
> 
> Reviewed-by: Damien Le Moal <[email protected]>
> 
> > ---
> >  zbd.c | 84 ++++++++++++++++++++++++++++++++---------------------------
> >  1 file changed, 45 insertions(+), 39 deletions(-)
> > 
> > diff --git a/zbd.c b/zbd.c
> > index ee095b1d..5e0f7f4e 100644
> > --- a/zbd.c
> > +++ b/zbd.c
> > @@ -1396,16 +1396,23 @@ static bool any_io_in_flight(void)
> >  	return false;
> >  }
> >  
> > -/*
> > +/**
> > + * zbd_convert_to_write_zone - Convert the write request to writable zone
> 
> Maybe: Convert the target zone of an io_u to a writable zone
> ?

Thanks, will reflect to v2.

> 
> > + * @td: The fio thread data
> > + * @io_u: The I/O unit write request to convert
> 
> We do not convert the io_u, but rather, the zones it targets, no ?

Agreed, that will be more accurate description. I will modify the
descrption as follows in v2:

        @io_u: The I/O unit that targets the zone to convert

> 
> > + * @zb: The zone selected at the beginning of the function call. The caller must
> > + *      hold zb->mutex.
> > + *
> >   * Modify the offset of an I/O unit that does not refer to a zone such that
> > - * in write target zones array. Add a zone to or remove a zone from the lsit if
> > + * in write target zones array. Add a zone to or remove a zone from the list if
> 
> Nit: s/list/array

Thanks, will reflect to v2.