[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 6/7] block/backup: teach backup_cow_with_boun
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 6/7] block/backup: teach backup_cow_with_bounce_buffer to copy more at once |
Date: |
Fri, 9 Aug 2019 10:59:53 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
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 <address@hidden>
> ---
> 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
[Qemu-devel] [PATCH v2 3/7] block/io: handle alignment and max_transfer for copy_range, Vladimir Sementsov-Ogievskiy, 2019/08/09
[Qemu-devel] [PATCH v2 7/7] block/backup: merge duplicated logic into backup_do_cow, Vladimir Sementsov-Ogievskiy, 2019/08/09
[Qemu-devel] [PATCH v2 4/7] block/backup: drop handling of max_transfer for copy_range, Vladimir Sementsov-Ogievskiy, 2019/08/09
[Qemu-devel] [PATCH v2 2/7] block/backup: refactor write_flags, Vladimir Sementsov-Ogievskiy, 2019/08/09
Re: [Qemu-devel] [PATCH v2 0/7] backup improvements, Vladimir Sementsov-Ogievskiy, 2019/08/09