On Wed, Apr 01, 2020 at 08:42:48AM -0700, Christoph Hellwig wrote:
> > +loff_t iomap_iter(struct iomap_iter *iter, loff_t written)
> > +{
> > + const struct iomap_ops *ops = iter->ops;
> > + struct iomap *iomap = &iter->iomap;
> > + struct iomap *srcmap = &iter->srcmap;
>
> I think it makes sense to only have members in the iter structure
> that this function modifies. That is, just pass inode, ops and flags
> as explicit parameters.
One of the annoying things we do when looking at the disassembly is
spend a lot of instructions shuffling arguments around. Passing as many
arguments as possible in a struct minimises that. Ideally we'd pass
the iomap_iter to iomap_begin() and iomap_end(). Agreed passing the
ops there makes no sense, but I'd like to keep inode and flags in
the iomap_iter struct so they don't need to be passed to begin/end
as explicit arguments.
> OTOH the len argument / return value seems like something that would
> seems useful in the iter structure. That would require renaming the
> current len to something like total_len..
I'm inclined to go with seg_len and op_len.
> > +/* Magic value for first call to iterator */
> > +#define IOMAP_FIRST_CALL LLONG_MIN
>
> Can we find a way to make a a zero initialized field the indicatator
> of the first call? That way we don't need any knowledge of magic
> values in the callers. And also don't need any special initializer
> value, but just leave it to the caller to initialize .pos and
> .total_len, and be done with it.
Yeah; this was just a quick hack. I'll do something neater.
