On 9/1/26 15:37, Andrey Drobyshev wrote: >> Task size was capped to block_copy_chunk_size() regardless of method, >> which sizes a task by the copy buffer. A write-zeroes task carries no >> buffer, so that cap only splits one long known-zero run into a crowd of >> small COPY_WRITE_ZEROES tasks, each with its own request against the >> target. >> >> block_copy_task_create() already decides from zero_bitmap how far a task >> may run, so add block_copy_widen_zero_area() to that decision: for a >> write-zeroes task, re-search the dirty area under >> BLOCK_COPY_MAX_ZERO_CHUNK instead of the buffer chunk size, and let the >> existing clamp cut it back to where the zero run ends. Widening is safe: >> an overlapping caller waits on the task's BlockReq via >> reqlist_wait_one() rather than observing it mid-flight. >> >> The 256M cap stays well under BDRV_REQUEST_MAX_BYTES, where >> block_copy_do_copy() would trip assert(nbytes < INT_MAX), keeps the rate >> limiter usable, leaves enough tasks to fill BLOCK_COPY_MAX_WORKERS and >> bounds how long a cancel waits on one request. It is aligned at the use >> site rather than in the constant, since nothing bounds cluster_size. >> >> Signed-off-by: Denis V. Lunev <[email protected]> >> CC: Vladimir Sementsov-Ogievskiy <[email protected]> >> CC: John Snow <[email protected]> >> CC: Andrey Drobyshev <[email protected]> >> >> diff --git a/block/block-copy.c b/block/block-copy.c >> index c0a33599384..16b76484cbd 100644 >> --- a/block/block-copy.c >> +++ b/block/block-copy.c >> @@ -37,6 +37,8 @@ >> #define BLOCK_COPY_MAX_WORKERS 64 >> #define BLOCK_COPY_SLICE_TIME 100000000ULL /* ns */ >> #define BLOCK_COPY_CLUSTER_SIZE_DEFAULT (1 << 16) >> +/* Bounded by BDRV_REQUEST_MAX_BYTES, rate limiting and cancel latency. */ >> +#define BLOCK_COPY_MAX_ZERO_CHUNK (256 * MiB) > AFAIU with this new cap we enlarge the range, a guest IO to which will > trigger CBW. It used to be 1M or 16M depending on copy method, now it's > 256M. If the target is qcow2 - then write_zeroes() is size-independent > and is a simple metadata write. But what if target doesn't support fast > write_zeroes() operation? Then guest IO will be stalled for the entire > CBW, and the latency increases. Please correct me if I'm wrong. > > If it's true - shouldn't we probe for write_zeroes support adding > BDRV_REQ_NO_FALLBACK to the flags? And if -ENOTSUP is returned - don't > widen zero area. > > Andrey > v2 probes as you suggest. CBW itself cannot grow, though: block_copy_task_create() searches within the range of the block_copy() call it serves, so a copy driven by a guest write covers that write and nothing more. The wait comes the other way round, a guest write landing on a widened background task and waiting through reqlist_wait_one().
supported_zero_flags rules out the targets which cannot zero cheaply at all, qcow2 v2 and iscsi among them, but is optimistic for the rest: file-posix advertises NO_FALLBACK at open and finds out from the first fallocate that fails. So the first widened request carries the flag. A target which would write the zeroes out fails it without writing, that range then goes out in buffer-sized chunks, and widening stays off for the run. The cap is therefore BDRV_REQUEST_MAX_BYTES rather than 256M, aligned down to the cluster size. On a 16G source holding 1G of data, sync=full: 1088 block-copy tasks for qcow2 v3 and raw targets, 16384 for qcow2 v2, every target identical to the source. Thank you for your time, Den
