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

On Tue, Jul 28, 2026 at 12:12 PM Brian Foster <[email protected]> wrote:
>
> 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.

I'll wait a day for anyone to object, and if no one does, I'll send v5
tomorrow with your patch folded in.

Thanks,
Joanne

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