Re: [PATCH 6/9] zbd: consolidate zone mutex initialisation

[Alexey Dobriyan <adobriyan@gmail.com>]

On Thu, May 21, 2020 at 10:02:09PM +0000, Damien Le Moal wrote:
> On 2020/05/22 6:44, Alexey Dobriyan wrote:
> > On Mon, May 11, 2020 at 01:16:04AM +0000, Damien Le Moal wrote:
> >> On 2020/05/06 2:56, Alexey Dobriyan wrote:
> >>> Initialise everything that is not write pointers coming from device or
> >>> faked separatedly.
> >>
> >> s/Initialise/Initialize
> >> s/separatedly/separately
> > 
> >>  	assert(td->o.zone_mode == ZONE_MODE_ZBD);
> >>> @@ -546,11 +533,25 @@ 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.max_open_zones;
> >>> +	if (ret)
> >>> +		return ret;
> >>> +
> >>> +	zbd = f->zbd_info;
> >>> +	zbd->model = zbd_model;
> >>> +	zbd->max_open_zones = td->o.max_open_zones;
> >>> +
> >>> +	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);
> >>>  	}
> >>> -	return ret;
> >>> +	pthread_mutexattr_destroy(&attr);
> >>> +
> >>> +	return 0;
> >>>  }
> >>>  
> >>>  void zbd_free_zone_info(struct fio_file *f)
> >>>
> >>
> >> On a very large drive (e.g. 20TB SMR), we have in excess of 70000 zones. Having
> >> to loop over all of them again will start to be painful and likely slow down (a
> >> little) startup. And going forward with ever increasing disk capacity, this will
> >> get even worse. So not sure if this patch makes much sense.
> > 
> > It separates hw-specific code (reading write pointers) from fio-specific
> > code.
> 
> I appreciate the cleanup the patch does, but that does not address my concern
> about startup time.
> 
> > Iterating can be made faster by going over zone in [->min_zone, ->max_zone)
> > only, but I didn't look at it.
> 
> That would make things very complicated as the zbd_info zone array would end
> being sparse or not all zones initialized in it...

It is more! Don't need to allocate memory for unused trailing zones. :^)

In the future _this_ loop will do more work:

	->mutex
	->write_mutex (divide and conquer writes and appends)
	second ->wp
	per zone seed
	per zone generator state
	write list

> What about replacing the mutex initialization helper doing the entire loop again
> over all zones with one doing a single zone mutex initialization:
> 
> init_zone_mutex(zone, &attr)
> 
> That would cleanup nicely init_zone_info() & parse_zone_info() too.

OK, it is too late here to think.
I'll send what's doesn't raise questions for the release.