Re: [PATCH] qcow2: Reduce write_zeroes size in handle_alloc_space()

[Eric Blake <eblake@redhat.com>]

On 6/9/20 9:28 AM, Vladimir Sementsov-Ogievskiy wrote:
> 09.06.2020 17:08, Kevin Wolf wrote:
>> Since commit c8bb23cbdbe, handle_alloc_space() is called for newly
>> allocated clusters to efficiently initialise the COW areas with zeros if
>> necessary. It skips the whole operation if both start_cow nor end_cow
>> are empty. However, it requests zeroing the whole request size (possibly
>> multiple megabytes) even if only one end of the request actually needs
>> this.
>>
>> This patch reduces the write_zeroes request size in this case so that we
>> don't unnecessarily zero-initialise a region that we're going to
>> overwrite immediately.
>>

> 
> Hmm, I'm afraid, that this may make things worse in some cases, as with 
> one big write-zero request
> we preallocate data-region in the protocol file, so we have better 
> locality for the clusters we
> are going to write. And, in the same time, with BDRV_REQ_NO_FALLBACK 
> flag write-zero must be
> fast anyway (especially in comparison with the following write request).
> 
>>           /*
>>            * instead of writing zero COW buffers,
>>            * efficiently zero out the whole clusters
>>            */
>> -        ret = qcow2_pre_write_overlap_check(bs, 0, m->alloc_offset,
>> -                                            m->nb_clusters * 
>> s->cluster_size,
>> -                                            true);
>> +        ret = qcow2_pre_write_overlap_check(bs, 0, start, len, true);
>>           if (ret < 0) {
>>               return ret;
>>           }
>>           BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_SPACE);
>> -        ret = bdrv_co_pwrite_zeroes(s->data_file, m->alloc_offset,
>> -                                    m->nb_clusters * s->cluster_size,
>> +        ret = bdrv_co_pwrite_zeroes(s->data_file, start, len,
>>                                       BDRV_REQ_NO_FALLBACK);

Good point.  If we weren't using BDRV_REQ_NO_FALLBACK, then avoiding a 
pre-zero pass over the middle is essential.  But since we are insisting 
that the pre-zero pass be fast or else immediately fail, the time spent 
in pre-zeroing should not be a concern.  Do you have benchmark numbers 
stating otherwise?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org