Re: [PATCH v5 01/22] iomap: release the folio batch on iomap callback failures

On Wed, Jul 29, 2026 at 12:27:16PM -0700, Joanne Koong wrote:
> From: Brian Foster <[email protected]>
> 
> A sashiko review of an unrelated patch points out that the folio
> batch mechanism used for iomap zero range fails to release the batch
> in a couple error scenarios. If either calls to ->iomap_end() or
> ->iomap_begin() fail, the direct return paths bypass the batch
> cleanup.
> 
> The ->iomap_end() case is not a practical issue at the moment
> because there is no user of the mechanism that returns an error from
> this path. The ->iomap_begin() case is theoretically possible
> because XFS can invoke the fill helper and error out at various
> points thereafter. This subtly complicates things because XFS does
> not transfer iomap_flags to the iomap data structure in the error
> path.
> 
> To deal with both of these issues, first make sure to invoke the
> cleanup helper in the error path for either fs callback. Second,
> update the helper to clear the flag unconditionally and release the
> batch so long as it is populated. This more clearly delineates the
> purpose of the flag to control the I/O path and not necessarily the
> status of the fbatch, so add a comment around this as well.
> 
> Reported-by: Sashiko <[email protected]>
> Assisted-by: LLM
> Fixes: 395ed1ef0012 ("iomap: optional zero range dirty folio processing")
> Signed-off-by: Brian Foster <[email protected]>

Seems reasonable to me,

Cc: <[email protected]> # v6.19
Reviewed-by: "Darrick J. Wong" <[email protected]>

--D

> ---
>  fs/iomap/iter.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c
> index e4a29829591a..63617ec48250 100644
> --- a/fs/iomap/iter.c
> +++ b/fs/iomap/iter.c
> @@ -6,12 +6,18 @@
>  #include <linux/iomap.h>
>  #include "trace.h"
>  
> +/*
> + * Release the iter folio batch. Note that the iomap flag is meant to control
> + * the I/O path for the mapping and may not be set in error situations.
> + */
>  static inline void iomap_iter_clean_fbatch(struct iomap_iter *iter)
>  {
> -	if (iter->iomap.flags & IOMAP_F_FOLIO_BATCH) {
> +	if (!iter->fbatch)
> +		return;
> +	iter->iomap.flags &= ~IOMAP_F_FOLIO_BATCH;
> +	if (folio_batch_count(iter->fbatch)) {
>  		folio_batch_release(iter->fbatch);
>  		folio_batch_reinit(iter->fbatch);
> -		iter->iomap.flags &= ~IOMAP_F_FOLIO_BATCH;
>  	}
>  }
>  
> @@ -79,7 +85,7 @@ int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
>  						  olen),
>  				advanced, iter->flags, &iter->iomap);
>  		if (ret < 0 && !advanced)
> -			return ret;
> +			goto error;
>  	}
>  
>  	/* detect old return semantics where this would advance */
> @@ -110,7 +116,11 @@ int iomap_iter(struct iomap_iter *iter, const struct iomap_ops *ops)
>  	ret = ops->iomap_begin(iter->inode, iter->pos, iter->len, iter->flags,
>  			       &iter->iomap, &iter->srcmap);
>  	if (ret < 0)
> -		return ret;
> +		goto error;
>  	iomap_iter_done(iter);
>  	return 1;
> +
> +error:
> +	iomap_iter_clean_fbatch(iter);
> +	return ret;
>  }
> -- 
> 2.52.0
> 
>