On May 27, 2020 / 03:50, Damien Le Moal wrote:
> On 2020/05/27 10:20, Shin'ichiro Kawasaki wrote:
> > Commit 219c662d3b12 ("zbd: introduce per job maximum open zones limit")
> > introduced job_max_open_zones option which limits the number of open
> > zones per job. It has similar role as max_open_zones option which limits
> > the number of open zones for all jobs. It was intended that these two
> > options both work, but the commit replaced some checks for max_open_zones
> > simply with checks for job_max_open_zones. Because of this, when
> > max_open_zones is set and job_max_open_zones is not set, fio fails to
> > limit the number of open zones. This resulted in test case #29 failure
> > of t/zbd/test-zbd-support script for regular null_blk devices.
> >
> > To fix the failure, modify the checks to target both job_max_open_zones
> > and max_open_zones.
> >
> > Fixes: 219c662d3b12 ("zbd: introduce per job maximum open zones limit")
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> > ---
> > zbd.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/zbd.c b/zbd.c
> > index 72352db0..2a725c1f 100644
> > --- a/zbd.c
> > +++ b/zbd.c
> > @@ -997,7 +997,7 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
> >
> > assert(is_valid_offset(f, io_u->offset));
> >
> > - if (td->o.job_max_open_zones) {
> > + if (td->o.max_open_zones || td->o.job_max_open_zones) {
> > /*
> > * This statement accesses f->zbd_info->open_zones[] on purpose
> > * without locking.
> > @@ -1026,7 +1026,7 @@ static struct fio_zone_info *zbd_convert_to_open_zone(struct thread_data *td,
> >
> > zone_lock(td, f, z);
> > pthread_mutex_lock(&f->zbd_info->mutex);
> > - if (td->o.job_max_open_zones == 0)
> > + if (td->o.max_open_zones == 0 && td->o.job_max_open_zones == 0)
> > goto examine_zone;
> > if (f->zbd_info->num_open_zones == 0) {
> > pthread_mutex_unlock(&f->zbd_info->mutex);
> > @@ -1082,7 +1082,7 @@ examine_zone:
> > }
> > dprint(FD_ZBD, "%s(%s): closing zone %d\n", __func__, f->file_name,
> > zone_idx);
> > - if (td->o.job_max_open_zones)
> > + if (td->o.max_open_zones || td->o.job_max_open_zones)
> > zbd_close_zone(td, f, open_zone_idx);
> > pthread_mutex_unlock(&f->zbd_info->mutex);
> >
> >
>
> Looks good.
>
> Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
Jens,
Could you consider to pick up this fix for upstream? I think it is good to keep
the max_open_zones option behavior same as before to avoid confusions.
--
Best Regards,
Shin'ichiro Kawasaki
