On Mon, Sep 22, 2025 at 07:36:55PM +0800, luzhipeng wrote:
> When mirroring data blocks, detect if the read data consists entirely of
> zeros. If so, use blk_co_pwrite_zeroes() instead of regular write to
> improve performance.
>
> Signed-off-by: luzhipeng <[email protected]>
> ---
> block/mirror.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
First, some minor observations:
>
> diff --git a/block/mirror.c b/block/mirror.c
> index b344182c74..535112f65d 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -269,6 +269,33 @@ static void coroutine_fn mirror_read_complete(MirrorOp
> *op, int ret)
> return;
> }
>
> + /* Check if the read data is all zeros */
> + bool is_zero = true;
> + for (int i = 0; i < op->qiov.niov; i++) {
> + if (!buffer_is_zero(op->qiov.iov[i].iov_base,
> + op->qiov.iov[i].iov_len)) {
Indentation looks off here.
> + is_zero = false;
> + break;
> + }
> + }
> +
> + /* Write to target - optimized path for zero blocks */
> + if (is_zero) {
> + /*
> + * Use zero-writing interface which may:
> + * 1. Avoid actual data transfer
> + * 2. Enable storage-level optimizations
> + * 3. Potentially unmap blocks (if supported)
> + */
> + ret = blk_co_pwrite_zeroes(s->target, op->offset,
> + op->qiov.size,
> + BDRV_REQ_MAY_UNMAP);
...and here.
> + } else {
> + /* Normal data write path */
> + ret = blk_co_pwritev(s->target, op->offset,
> + op->qiov.size, &op->qiov, 0);
...here too.
> + }
> +
> ret = blk_co_pwritev(s->target, op->offset, op->qiov.size, &op->qiov, 0);
> mirror_write_complete(op, ret);
> }
Then a higher-level question. Should we teach blk_co_pwritev() to
have a flag that ANY caller can set to request write-zero
optimizations, rather than having to open-code a check and call to
blk_co_pwrite_zeroes() at every call-site that might benefit?
Optimizing to a write zero is often nice, but using BDRV_REQ_MAY_UNMAP
may break use cases that have explicitly requested full allocation.
The more we can consolidate all of the decisions about whether or not
to use write zeroes, and whether or not to allow unmap in that
attempt, into a single common function rather than duplicated out at
every call site that might benefit, the easier it will be to maintain
the code down the road.
Thus, I'm wondering if it might be better to introduce a new BDRV_REQ_
flag for passing to blk_co_pwritev for deciding to trigger potential
write-zero optimizations.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
