Re: [PATCH v2 02/18] xfs: convert iomap ops to ->iomap_next()
Joanne Koong <[email protected]>
| Newsgroups | org.kernel.vger.linux-xfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAJnrk1bjA+Y5V-87AhNGR5-ss7gUJWwLkjouwic1Mckd2QRH7Q@mail.gmail.com> |
On Fri, Jul 17, 2026 at 10:55 AM Darrick J. Wong <[email protected]> wrote: > > On Fri, Jul 17, 2026 at 09:49:30AM -0700, Joanne Koong wrote: > > On Thu, Jul 2, 2026 at 6:21 PM Joanne Koong <[email protected]> wrote: > > > > > > Ahh okay, I think I see the point you were trying to make. The > > > callsite functions are like: > > > > > > ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from, > > > iomap_next_fn iomap_next, > > > const struct iomap_write_ops *write_ops, void *private); > > > > > > int iomap_fsverity_write(struct file *file, loff_t pos, size_t length, > > > const void *buf, iomap_next_fn iomap_next, > > > const struct iomap_write_ops *write_ops); > > > > > > void iomap_read_folio(iomap_next_fn iomap_next, > > > struct iomap_read_folio_ctx *ctx, void *private); > > > > > > void iomap_readahead(iomap_next_fn iomap_next, > > > struct iomap_read_folio_ctx *ctx, void *private); > > > > > > etc, so it's not clear from the "iomap_next_fn iomap_next" naming that > > > it handles per-iteration logic, whereas renaming it to "iomap_iter_fn > > > iomap_iter" makes it more explicit. Am I interpreting this correctly? > > > > Going back to this... "iomap_iter_fn iomap_iter" doesn't feel quite > > right. I think there's too much naming collision with iomap_iter() and > > struct iomap_iter, and I like how in general ->next() matches the > > orthodox naming scheme for advancing an iterator. I think > > "iomap_iter_next_fn next" would be the best of both worlds - I'll > > change it to this for v3. But if you don't agree with this, please let > > me know. > > Fine by me :) > > --D > > > > > Thanks, > > Joanne > > > > > > > > > > > +{ > > > > > > + return iomap_process(iter, iomap, srcmap, xfs_direct_write_iomap_begin, > > > > > > + NULL); > > > > > > > > > > But then "iomap_iter" sets up a new problem: should iomap_process have a > > > > > new name that goes along with that? iomap_iter is already taken for the I'm starting not to like "iomap_process()" either, it seems too vague... I think it'd be better renamed to "iomap_iter_next()". The callsites would then just look something like: static int xfs_read_iomap_next( const struct iomap_iter *iter, struct iomap *iomap, struct iomap *srcmap) { return iomap_iter_next(iter, iomap, srcmap, xfs_read_iomap_begin, NULL); } static int xfs_seek_iomap_next( const struct iomap_iter *iter, struct iomap *iomap, struct iomap *srcmap) { return iomap_iter_next(iter, iomap, srcmap, xfs_seek_iomap_begin, NULL); } I'll make this change for v3. But if you don't like this, please let me know :) Thanks, Joanne > > > > > legacy path. If we were emulating python I'd suggest iomap_iter_yield > > > > > but this is C so we get to reinvent everything from scratch so who > > > > > knows. > > > > > > > > > > --D > > > > > > >