Re: [PATCH v1 1/1] iomap: don't mark folio uptodate if read IO has bytes pending

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

On Wed, Feb 18, 2026 at 04:39:11PM -0800, Joanne Koong wrote:
> If a folio has ifs metadata attached to it and the folio is partially
> read in through an async IO helper with the rest of it then being read
> in through post-EOF zeroing or as inline data, and the helper
> successfully finishes the read first, then post-EOF zeroing / reading
> inline will mark the folio as uptodate in iomap_set_range_uptodate().
> 
> This is a problem because when the read completion path later calls
> iomap_read_end(), it will call folio_end_read(), which sets the uptodate
> bit using XOR semantics. Calling folio_end_read() on a folio that was
> already marked uptodate clears the uptodate bit.

Aha, I wondered if that xor thing was going to come back to bite us.

> Fix this by not marking the folio as uptodate if the read IO has bytes
> pending. The folio uptodate state will be set in the read completion
> path through iomap_end_read() -> folio_end_read().
> 
> Reported-by: Wei Gao <wegao@suse.com>
> Suggested-by: Sasha Levin <sashal@kernel.org>
> Tested-by: Wei Gao <wegao@suse.com>
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> Fixes: b2f35ac4146d ("iomap: add caller-provided callbacks for read and readahead")

I would add:

Link: https://lore.kernel.org/linux-fsdevel/aYbmy8JdgXwsGaPP@autotest-wegao.qe.prg2.suse.org/
Cc: <stable@vger.kernel.org> # v6.19

since the recent discussion around this was sort of buried in a
different thread, and the original patch is now in a released kernel.

> ---
>  fs/iomap/buffered-io.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 58887513b894..4fc5ce963feb 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -80,18 +80,27 @@ static void iomap_set_range_uptodate(struct folio *folio, size_t off,
>  {
>  	struct iomap_folio_state *ifs = folio->private;
>  	unsigned long flags;
> -	bool uptodate = true;
> +	bool mark_uptodate = true;
>  
>  	if (folio_test_uptodate(folio))
>  		return;
>  
>  	if (ifs) {
>  		spin_lock_irqsave(&ifs->state_lock, flags);
> -		uptodate = ifs_set_range_uptodate(folio, ifs, off, len);
> +		/*
> +		 * If a read with bytes pending is in progress, we must not call
> +		 * folio_mark_uptodate(). The read completion path
> +		 * (iomap_read_end()) will call folio_end_read(), which uses XOR
> +		 * semantics to set the uptodate bit. If we set it here, the XOR
> +		 * in folio_end_read() will clear it, leaving the folio not
> +		 * uptodate.

Yeah, that makes sense.  How difficult is this to write up as an fstest?

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> +		 */
> +		mark_uptodate = ifs_set_range_uptodate(folio, ifs, off, len) &&
> +				!ifs->read_bytes_pending;
>  		spin_unlock_irqrestore(&ifs->state_lock, flags);
>  	}
>  
> -	if (uptodate)
> +	if (mark_uptodate)
>  		folio_mark_uptodate(folio);
>  }
>  
> -- 
> 2.47.3
> 
>