On Thu, Jul 02, 2026 at 05:17:02PM -0700, Joanne Koong wrote:
> On Thu, Jul 2, 2026 at 9:58 AM Darrick J. Wong <[email protected]> wrote:
> >
> > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > > index 3f0932e46fd6..0aa8abc438c1 100644
> > > --- a/fs/iomap/buffered-io.c
> > > +++ b/fs/iomap/buffered-io.c
> > > @@ -626,7 +626,7 @@ static int iomap_read_folio_iter(struct iomap_iter
> > > *iter,
> > > return 0;
> > > }
> > >
> > > -void iomap_read_folio(const struct iomap_ops *ops,
> > > +void iomap_read_folio(iomap_next_fn iomap_next,
> >
> > If you took my earlier suggestion to rename the typedef to
> > iomap_iter_fn, then this parameter ought to be named iter_fn.
>
> Hmm... maybe at that point, it's self-explanatory enough that the arg
> could just be called "iter" instead of "iter_fn"?
Dunno. Seeing as we already have variables named "iter" that are the
actual iteration state object, I think it's clearer to leave the
iteration function as "iter_fn".
> >
> > > struct iomap_read_folio_ctx *ctx, void *private)
> > > {
> > > struct folio *folio = ctx->cur_folio;
> > > @@ -650,7 +650,7 @@ void iomap_read_folio(const struct iomap_ops *ops,
> > > fsverity_readahead(ctx->vi, folio->index,
> > > folio_nr_pages(folio));
> > >
> > > - while ((ret = iomap_iter(&iter, ops)) > 0) {
> > > + while ((ret = iomap_iter(&iter, iomap_next)) > 0) {
> > > iter.status = iomap_read_folio_iter(&iter, ctx,
> > > &bytes_submitted);
> > > iomap_read_submit(&iter, ctx);
> > > @@ -688,22 +688,22 @@ static int iomap_readahead_iter(struct iomap_iter
> > > *iter,
> > >
> > > /**
> > > * iomap_readahead - Attempt to read pages from a file.
> > > - * @ops: The operations vector for the filesystem.
> > > + * @iomap_next: The iomap_next callback for the filesystem.
> >
> > "The iomap iteration function for the filesystem" ?
> >
> > Using the term "iomap_next" in the definition for iomap_next isn't that
> > helpful.
>
> Agreed, I'll replace this with your suggestion.
<nod>
> >
> > > return ret;
> > > @@ -824,16 +824,16 @@ xfs_file_dio_write_atomic(
> > > unsigned int iolock = XFS_IOLOCK_SHARED;
> > > ssize_t ret, ocount = iov_iter_count(from);
> > > unsigned int dio_flags = 0;
> > > - const struct iomap_ops *dops;
> > > + iomap_next_fn dops;
> > >
> > > /*
> > > * HW offload should be faster, so try that first if it is already
> > > * known that the write length is not too large.
> > > */
> > > if (ocount > xfs_inode_buftarg(ip)->bt_awu_max)
> > > - dops = &xfs_atomic_write_cow_iomap_ops;
> > > + dops = xfs_atomic_write_cow_iomap_next;
> > > else
> > > - dops = &xfs_direct_write_iomap_ops;
> > > + dops = xfs_direct_write_iomap_next;
> >
> > Probably ought to be called iter_fn, or at least something that isn't
> > "dops".
>
> Nice spotting, I'll rename this in the next version.
<nod>
--D
> Thanks,
> Joanne
>
