On Fri, May 01, 2020 at 01:44:26AM +0000, Damien Le Moal wrote:
> On 2020/04/30 21:41, Alexey Dobriyan wrote:
> > Another mutex is coming!
>
> That is not very descriptive at all.
> What the patch does is to make the per zone mutex initialization code common
> between init_zone_info() and parse_zone_info(), moving it to create_zone_info().
> > @@ -546,11 +532,27 @@ static int zbd_create_zone_info(struct thread_data *td, struct fio_file *f)
> > return -EINVAL;
> > }
> >
> > - if (ret == 0) {
> > - f->zbd_info->model = zbd_model;
> > - f->zbd_info->max_open_zones = td->o.global_max_open_zones;
> > + if (ret)
> > + return ret;
> > +
> > + zbd = f->zbd_info;
> > + zbd->model = zbd_model;
> > + zbd->max_open_zones = td->o.global_max_open_zones;
> > + {
> > + pthread_mutexattr_t attr;
>
> If you move this declaration to the beginning of zbd_create_zone_info(), you can
> avoid this block code, which does not look pretty.
It is done on purpose, to move declaration closer to usage.
fio is built with -std=gnu99 _and_ -Wdeclaration-after-statement which
prevents
a = b;
int c;
c = d;
modern style of declaration.
> > + pthread_mutexattr_init(&attr);
> > + pthread_mutexattr_setpshared(&attr, true);
> > + pthread_mutex_init(&zbd->mutex, &attr);
> > + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
> > + for (uint32_t z = 0; z < zbd->nr_zones; z++) {
> > + struct fio_zone_info *zi = &zbd->zone_info[z];
> > +
> > + pthread_mutex_init(&zi->mutex, &attr);
> > + }
> > + pthread_mutexattr_destroy(&attr);
> > }
> > - return ret;
> > + return 0;
> > }
> >
> > void zbd_free_zone_info(struct fio_file *f)
> >
>
> With the nits above fixed, Looks good to me.
>
> Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
