On 8/9/19 10:32 AM, Vladimir Sementsov-Ogievskiy wrote: > backup_cow_with_offload can transfer more than on cluster. Let
s/on/one/ > backup_cow_with_bounce_buffer behave similarly. It reduces number > of IO and there are no needs to copy cluster by cluster. It reduces the number of IO requests, since there is no need to copy cluster by cluster. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > block/backup.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index d482d93458..155e21d0a3 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -104,22 +104,25 @@ static int coroutine_fn > backup_cow_with_bounce_buffer(BackupBlockJob *job, > int64_t start, > int64_t end, > bool is_write_notifier, > - bool *error_is_read, > - void **bounce_buffer) > + bool *error_is_read) Why is this signature changing? > { > int ret; > BlockBackend *blk = job->common.blk; > int nbytes; > int read_flags = is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0; > + void *bounce_buffer; > > assert(QEMU_IS_ALIGNED(start, job->cluster_size)); > - bdrv_reset_dirty_bitmap(job->copy_bitmap, start, job->cluster_size); > - nbytes = MIN(job->cluster_size, job->len - start); > - if (!*bounce_buffer) { > - *bounce_buffer = blk_blockalign(blk, job->cluster_size); Pre-patch, you allocate the bounce_buffer at most once (but limited to a cluster size), post-patch, you are now allocating and freeing a bounce buffer every iteration through. There may be fewer calls because you are allocating something bigger, but still, isn't it a good goal to try and allocate the bounce buffer as few times as possible and reuse it, rather than getting a fresh one each iteration? > + > + nbytes = MIN(end - start, job->len - start); What is the largest this will be? Will it exhaust memory on a 32-bit or otherwise memory-constrained system, where you should have some maximum cap for how large the bounce buffer will be while still getting better performance than one cluster at a time and not slowing things down by over-sizing malloc? 64M, maybe? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature