From: Denis V. Lunev <[email protected]> zero_bitmap_valid says more than its name: a task created once it is set skips its own block-status query, the sync=top skipping included, because the scan has already settled what to copy. And the end of the scan was announced twice, clearing skip_unallocated and then publishing zero_bitmap_valid, with an ordering rule in a comment tying the two.
Rename the flag to scan_done, and let block_copy_set_scan_done() clear skip_unallocated before publishing it, so backup_scan_source() makes one call and the order is kept in one place. skip_unallocated is left to mean that a sync=top scan is in progress. No behavior change. Suggested-by: Vladimir Sementsov-Ogievskiy <[email protected]> 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 | 6 +----- block/block-copy.c | 19 ++++++++++++------- include/block/block-copy.h | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/block/backup.c b/block/backup.c index 60c00fca67..1bf6cbedac 100644 --- a/block/backup.c +++ b/block/backup.c @@ -254,7 +254,6 @@ static void backup_init_bcs_bitmap(BackupBlockJob *job) static int coroutine_fn backup_scan_source(BackupBlockJob *s) { Job *job = &s->common.job; - bool top = s->sync_mode == MIRROR_SYNC_MODE_TOP; int64_t offset, cached_end = 0, count; int ret = 0; @@ -279,10 +278,7 @@ static int coroutine_fn backup_scan_source(BackupBlockJob *s) } } - if (top) { - block_copy_set_skip_unallocated(s->bcs, false); - } - block_copy_set_zero_bitmap_valid(s->bcs); + block_copy_set_scan_done(s->bcs); return 0; } diff --git a/block/block-copy.c b/block/block-copy.c index 78cc855b5b..3b7e282e5b 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -161,8 +161,12 @@ typedef struct BlockCopyState { 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 */ + /* + * Set when the scan is over: zero_bitmap is complete, and for sync=top + * copy_bitmap holds no unallocated clusters, so a task needs no + * block-status query of its own. + */ + bool scan_done; /* atomic, store-release/load-acquire */ ProgressMeter *progress; SharedResource *mem; RateLimit rate_limit; @@ -214,7 +218,7 @@ block_copy_task_create(BlockCopyState *s, BlockCopyCallState *call_state, * 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)) { + if (qatomic_load_acquire(&s->scan_done)) { int64_t boundary; if (hbitmap_get(s->zero_bitmap, offset)) { @@ -687,7 +691,7 @@ 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. */ +/* Only the scan allocates, and it runs before scan_done. */ static HBitmap *block_copy_zero_bitmap(BlockCopyState *s) { if (!s->zero_bitmap) { @@ -845,7 +849,7 @@ block_copy_set_task_method(BlockCopyState *s, BlockCopyTask *task) int64_t status_bytes; /* block_copy_task_create() already decided, from zero_bitmap. */ - if (qatomic_load_acquire(&s->zero_bitmap_valid)) { + if (qatomic_load_acquire(&s->scan_done)) { return true; } @@ -1179,10 +1183,11 @@ 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) +void block_copy_set_scan_done(BlockCopyState *s) { block_copy_zero_bitmap(s); - qatomic_store_release(&s->zero_bitmap_valid, true); + qatomic_set(&s->skip_unallocated, false); + qatomic_store_release(&s->scan_done, true); } void block_copy_set_speed(BlockCopyState *s, uint64_t speed) diff --git a/include/block/block-copy.h b/include/block/block-copy.h index dc51316fcf..e82c96586e 100644 --- a/include/block/block-copy.h +++ b/include/block/block-copy.h @@ -102,6 +102,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); +void block_copy_set_scan_done(BlockCopyState *s); #endif /* BLOCK_COPY_H */ -- 2.53.0
