[PATCH] mirror: Optimize mirroring for zero blocks in mirror_read_complete

luzhipeng posted 1 patch 6 days, 3 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250922113657.391-2-luzhipeng@cestc.cn
Maintainers: John Snow <jsnow@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
block/mirror.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
[PATCH] mirror: Optimize mirroring for zero blocks in mirror_read_complete
Posted by luzhipeng 6 days, 3 hours ago
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 <luzhipeng@cestc.cn>
---
 block/mirror.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

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)) {
+            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);
+    } else {
+        /* Normal data write path */
+        ret = blk_co_pwritev(s->target, op->offset,
+                           op->qiov.size, &op->qiov, 0);
+    }
+
     ret = blk_co_pwritev(s->target, op->offset, op->qiov.size, &op->qiov, 0);
     mirror_write_complete(op, ret);
 }
-- 
2.45.1.windows.1
Re: [PATCH] mirror: Optimize mirroring for zero blocks in mirror_read_complete
Posted by Eric Blake 6 days, 2 hours ago
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 <luzhipeng@cestc.cn>
> ---
>  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