Re: [PATCH v3 5/8] iomap: simplify when reads can be skipped for writes

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Nov 04, 2025 at 12:51:16PM -0800, Joanne Koong wrote:
> Currently, the logic for skipping the read range for a write is
> 
> if (!(iter->flags & IOMAP_UNSHARE) &&
>     (from <= poff || from >= poff + plen) &&
>     (to <= poff || to >= poff + plen))
> 
> which breaks down to skipping the read if any of these are true:
> a) from <= poff && to <= poff
> b) from <= poff && to >= poff + plen
> c) from >= poff + plen && to <= poff
> d) from >= poff + plen && to >= poff + plen
> 
> This can be simplified to
> if (!(iter->flags & IOMAP_UNSHARE) && from <= poff && to >= poff + plen)
> 
> from the following reasoning:
> 
> a) from <= poff && to <= poff
> This reduces to 'to <= poff' since it is guaranteed that 'from <= to'
> (since to = from + len). It is not possible for 'from <= to' to be true
> here because we only reach here if plen > 0 (thanks to the preceding 'if
> (plen == 0)' check that would break us out of the loop). If 'to <=
> poff', plen would have to be 0 since poff and plen get adjusted in
> lockstep for uptodate blocks. This means we can eliminate this check.
> 
> c) from >= poff + plen && to <= poff
> This is not possible since 'from <= to' and 'plen > 0'. We can eliminate
> this check.
> 
> d) from >= poff + plen && to >= poff + plen
> This reduces to 'from >= poff + plen' since 'from <= to'.
> It is not possible for 'from >= poff + plen' to be true here. We only
> reach here if plen > 0 and for writes, poff and plen will always be
> block-aligned, which means poff <= from < poff + plen. We can eliminate
> this check.
> 
> The only valid check is b) from <= poff && to >= poff + plen.
> 
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Heh yeah, makes sense to me
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/iomap/buffered-io.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 0f14d2a91f49..c02d33bff3d0 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -752,9 +752,12 @@ static int __iomap_write_begin(const struct iomap_iter *iter,
>  		if (plen == 0)
>  			break;
>  
> -		if (!(iter->flags & IOMAP_UNSHARE) &&
> -		    (from <= poff || from >= poff + plen) &&
> -		    (to <= poff || to >= poff + plen))
> +		/*
> +		 * If the read range will be entirely overwritten by the write,
> +		 * we can skip having to zero/read it in.
> +		 */
> +		if (!(iter->flags & IOMAP_UNSHARE) && from <= poff &&
> +		    to >= poff + plen)
>  			continue;
>  
>  		if (iomap_block_needs_zeroing(iter, block_start)) {
> -- 
> 2.47.3
> 
>