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

[Kevin Wolf <kwolf@redhat.com>]

Am 22.09.2025 um 14:53 hat Eric Blake geschrieben:
> 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.

We already have the detect-zeroes=on|off|unmap option, which is used in
bdrv_aligned_pwritev() to enable this kind of optimisation. I think we
should reuse this code in some way. We also should configure the mirror
job in a similar way, adding a new detect-zeroes=on|off|unmap option to
blockdev-mirror (and off will stay the default).

One option is, as you say, that we introduce BDRV_REQ_* flags that could
take effect even when the BlockDriverState isn't configured to detect
zeroes. For a three-way option, we'd need a combination of two flags,
though, or a two-bit field. I'm not sure how much I like this.

The other option is that you just set detect-zeroes on the target node
that mirror points to. What makes this a bit harder is that mirror only
owns the BlockBackend, but no node on the target side (on the source
side, we have the mirror_top filter), and suddenly inserting new nodes
isn't great either. So this is probably something that mirror can't do
for you, but you as the user would have to configure the target node
explicitly this way.

Kevin