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

[Eric Blake]

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