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...
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.
--
Damien Le Moal
Western Digital Research
