Re: [PATCH] iomap: release the folio batch on iomap callback failures

On Tue, Jul 28, 2026 at 11:30 AM Brian Foster <[email protected]> wrote:
>
> 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]>
> ---
>
> As noted here[1], I'm aware this conflicts with the outstanding iomap
> iter rework. I'm happy to rebase onto that if that is ultimately
> preferred. I've got at least one vote to get this in sooner, so this
> version is based on 7.2-rc5.
>

Hi Brian,

As I understand your analysis of the bug in [1], given that it's an
unlikely / second-order scenario nobody is hitting in reality, maybe
it'd be easiest if I fold this patch into the iomap iter rework series
as a preparatory patch (keeping your authorship and Fixes: tag), make
the needed changes in the iomap iter series to be compatible with your
patch, and then submit everything together to the vfs-7.3.iomap branch
as v5 of the series? I think that avoids the nontrivial merge conflict
Christian/Stephen would have to deal with.

Alternatively, if you prefer to have this as part of 7.2, I can send a
v5 of the series to try minimizing the conflict, and then send
Christian or Stephen a diff for how to resolve the merge when they hit
it.

Thanks,
Joanne

[1] https://lore.kernel.org/linux-fsdevel/amjztG-DisHYbV9W@bfoster/