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

On Tue, Jul 28, 2026 at 12:07:10PM -0700, Joanne Koong wrote:
> 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.
> 

That's perfectly fine with me if you're Ok with doing that and nobody
otherwise objects.

Brian

> 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/
>