From: Denis V. Lunev <[email protected]> block_copy_reset_unallocated()'s up-front scan already queries the source, and the same query reports zero-ness. The copy loop re-queries per task and, having no answer up front, can only size a task by the copy buffer, so a mostly-zero image is cut into thousands of pieces that each turn out to read as zero.
Cache the answer in zero_bitmap, published by zero_bitmap_valid once the scan is done. block_copy_task_create() then picks the method from the bitmap and ends the task where the answer changes, so a zero task never reaches into data and a copy task never swallows a zero run. The per-task query stays as the fallback until the flag is set, since CBW intercepts guest writes while the scan is still running. The scan and the query it replaces must resolve BDRV_BLOCK_ZERO against the same part of the chain, so the scan moves from bdrv_co_is_allocated() to bdrv_co_block_status_above() and the base selection is factored out into block_copy_status_base(). zero_bitmap is a plain HBitmap, not a BdrvDirtyBitmap: an internal cache has no business in query-named-block-nodes. One writer and readers gated by the flag need no mutex, but the publish needs ordering, hence store-release and load-acquire. It is allocated on demand, so sync=none and a standalone copy-before-write filter do not pay for it. Signed-off-by: Denis V. Lunev <[email protected]> CC: Vladimir Sementsov-Ogievskiy <[email protected]> CC: John Snow <[email protected]> CC: Andrey Drobyshev <[email protected]> --- block/backup.c | 1 + block/block-copy.c | 108 ++++++++++++++++++++++++++++++++----- include/block/block-copy.h | 1 + 3 files changed, 98 insertions(+), 12 deletions(-) diff --git a/block/backup.c b/block/backup.c index d4713fa1cd..11d70243e2 100644 --- a/block/backup.c +++ b/block/backup.c @@ -280,6 +280,7 @@ static int coroutine_fn backup_run(Job *job, Error **errp) offset += count; } block_copy_set_skip_unallocated(s->bcs, false); + block_copy_set_zero_bitmap_valid(s->bcs); } if (s->sync_mode == MIRROR_SYNC_MODE_NONE) { diff --git a/block/block-copy.c b/block/block-copy.c index d71d070dbd..94d4f3dd69 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -20,6 +20,8 @@ #include "block/block_int-io.h" #include "block/dirty-bitmap.h" #include "block/reqlist.h" +#include "qemu/hbitmap.h" +#include "qemu/host-utils.h" #include "system/block-backend.h" #include "qemu/units.h" #include "qemu/co-shared-resource.h" @@ -157,6 +159,10 @@ typedef struct BlockCopyState { bool skip_unallocated; /* atomic */ /* State fields that use a thread-safe API */ BdrvDirtyBitmap *copy_bitmap; + /* Clusters reading as zero; allocated on demand, frozen once valid. */ + HBitmap *zero_bitmap; + /* Published only after the scan, with skip_unallocated already false. */ + bool zero_bitmap_valid; /* atomic, store-release/load-acquire */ ProgressMeter *progress; SharedResource *mem; RateLimit rate_limit; @@ -190,6 +196,7 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state, int64_t offset, int64_t bytes) { BlockCopyTask *task; + BlockCopyMethod method; int64_t max_chunk; QEMU_LOCK_GUARD(&s->lock); @@ -201,6 +208,27 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state, return NULL; } + method = s->method; + + /* + * The scan already knows how this range reads: pick the method here and + * stop the task where the answer changes. + */ + if (qatomic_load_acquire(&s->zero_bitmap_valid)) { + int64_t boundary; + + if (hbitmap_get(s->zero_bitmap, offset)) { + method = COPY_WRITE_ZEROES; + boundary = hbitmap_next_zero(s->zero_bitmap, offset, bytes); + } else { + boundary = hbitmap_next_dirty(s->zero_bitmap, offset, bytes); + } + + if (boundary >= 0) { + bytes = boundary - offset; + } + } + assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); bytes = QEMU_ALIGN_UP(bytes, s->cluster_size); @@ -215,7 +243,7 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state, .task.func = block_copy_task_entry, .s = s, .call_state = call_state, - .method = s->method, + .method = method, }; reqlist_init_req(&s->reqs, &task->req, offset, bytes); @@ -271,6 +299,9 @@ void block_copy_state_free(BlockCopyState *s) ratelimit_destroy(&s->rate_limit); bdrv_release_dirty_bitmap(s->copy_bitmap); + if (s->zero_bitmap) { + hbitmap_free(s->zero_bitmap); + } shres_destroy(s->mem); g_free(s); } @@ -624,20 +655,24 @@ static coroutine_fn int block_copy_task_entry(AioTask *task) return ret; } +/* The scan and the per-task query must resolve BDRV_BLOCK_ZERO alike. */ +static GRAPH_RDLOCK BlockDriverState *block_copy_status_base(BlockCopyState *s) +{ + if (qatomic_read(&s->skip_unallocated)) { + return bdrv_backing_chain_next(s->source->bs); + } + + return NULL; +} + static coroutine_fn GRAPH_RDLOCK int block_copy_block_status(BlockCopyState *s, int64_t offset, int64_t bytes, int64_t *pnum) { int64_t num; - BlockDriverState *base; + BlockDriverState *base = block_copy_status_base(s); int ret; - if (qatomic_read(&s->skip_unallocated)) { - base = bdrv_backing_chain_next(s->source->bs); - } else { - base = NULL; - } - ret = bdrv_co_block_status_above(s->source->bs, base, offset, bytes, &num, NULL, NULL); if (ret < 0 || num < s->cluster_size) { @@ -657,16 +692,41 @@ int block_copy_block_status(BlockCopyState *s, int64_t offset, int64_t bytes, return ret; } +/* Only the scan allocates, and it runs before zero_bitmap_valid. */ +static HBitmap *block_copy_zero_bitmap(BlockCopyState *s) +{ + if (!s->zero_bitmap) { + s->zero_bitmap = hbitmap_alloc(s->len, ctz32(s->cluster_size)); + } + + return s->zero_bitmap; +} + +static void block_copy_mark_zero_prefix(BlockCopyState *s, int64_t offset, + int64_t zero_count) +{ + int64_t zero_bytes = QEMU_ALIGN_DOWN(zero_count, s->cluster_size); + + if (zero_bytes > 0) { + hbitmap_set(block_copy_zero_bitmap(s), offset, zero_bytes); + } +} + /* * Check if the cluster starting at offset is allocated or not. * return via pnum the number of contiguous clusters sharing this allocation. + * Also marks the zero prefix of the range in zero_bitmap. */ static int coroutine_fn GRAPH_RDLOCK block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset, int64_t *pnum) { BlockDriverState *bs = s->source->bs; + BlockDriverState *base = block_copy_status_base(s); + int64_t orig_offset = offset; int64_t count, total_count = 0; + int64_t zero_count = 0; + bool zero_broken = false; int64_t bytes = s->len - offset; int ret; @@ -674,25 +734,37 @@ block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset, while (true) { /* protected in backup_run() */ - ret = bdrv_co_is_allocated(bs, offset, bytes, &count); + ret = bdrv_co_block_status_above(bs, base, offset, bytes, &count, + NULL, NULL); if (ret < 0) { return ret; } + if (!zero_broken) { + if (ret & BDRV_BLOCK_ZERO) { + zero_count += count; + } else { + zero_broken = true; + } + } + total_count += count; - if (ret || count == 0) { + if ((ret & BDRV_BLOCK_ALLOCATED) || count == 0) { /* - * ret: partial segment(s) are considered allocated. + * BDRV_BLOCK_ALLOCATED: partial segment(s) are considered + * allocated. * otherwise: unallocated tail is treated as an entire segment. */ *pnum = DIV_ROUND_UP(total_count, s->cluster_size); - return ret; + block_copy_mark_zero_prefix(s, orig_offset, zero_count); + return !!(ret & BDRV_BLOCK_ALLOCATED); } /* Unallocated segment(s) with uncertain following segment(s) */ if (total_count >= s->cluster_size) { *pnum = total_count / s->cluster_size; + block_copy_mark_zero_prefix(s, orig_offset, zero_count); return 0; } @@ -752,6 +824,12 @@ block_copy_set_task_method(BlockCopyState *s, BlockCopyTask *task) int ret; int64_t status_bytes; + /* block_copy_task_create() already decided, from zero_bitmap. */ + if (qatomic_load_acquire(&s->zero_bitmap_valid)) { + return true; + } + + /* CBW filter could call this early. */ ret = block_copy_block_status(s, task->req.offset, task->req.bytes, &status_bytes); assert(ret >= 0); /* never fail */ @@ -1081,6 +1159,12 @@ void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip) qatomic_set(&s->skip_unallocated, skip); } +void block_copy_set_zero_bitmap_valid(BlockCopyState *s) +{ + block_copy_zero_bitmap(s); + qatomic_store_release(&s->zero_bitmap_valid, true); +} + void block_copy_set_speed(BlockCopyState *s, uint64_t speed) { ratelimit_set_speed(&s->rate_limit, speed, BLOCK_COPY_SLICE_TIME); diff --git a/include/block/block-copy.h b/include/block/block-copy.h index 0df2771181..e4e5b56753 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -101,5 +101,6 @@ void block_copy_call_cancel(BlockCopyCallState *call_state); BdrvDirtyBitmap *block_copy_dirty_bitmap(BlockCopyState *s); int64_t block_copy_cluster_size(BlockCopyState *s); void block_copy_set_skip_unallocated(BlockCopyState *s, bool skip); +void block_copy_set_zero_bitmap_valid(BlockCopyState *s); #endif /* BLOCK_COPY_H */ -- 2.53.0
