On Tue, Aug 11, 2020 at 02:01:27PM -0700, Darrick J. Wong wrote:
> On Tue, Jul 28, 2020 at 06:32:14PM +0100, Matthew Wilcox (Oracle) wrote:
> > The iomap_iter provides a convenient way to package up and maintain
> > all the arguments to the various mapping and operation functions.
> > iomi_advance() moves the iterator forward to the next chunk of the file.
> > This approach has only one callback to the filesystem -- the iomap_next_t
> > instead of the separate iomap_begin / iomap_end calls. This slightly
> > complicates the filesystems, but is more efficient. The next function
>
> How much more efficient? I've wondered since the start of
I haven't done any performance measurements. Not entirely sure what
I'd do to measure it, to be honest. I suppose I should also note the
decreased stack depth here, although I do mention that in the next patch.
> > +/**
> > + * struct iomap_iter - Iterate through a range of a file.
> > + * @inode: Set at the start of the iteration and should not change.
> > + * @pos: The current file position we are operating on. It is updated by
> > + * calls to iomap_iter(). Treat as read-only in the body.
> > + * @len: The remaining length of the file segment we're operating on.
> > + * It is updated at the same time as @pos.
> > + * @ret: What we want our declaring function to return. It is initialised
> > + * to zero and is the cumulative number of bytes processed so far.
> > + * It is updated at the same time as @pos.
> > + * @copied: The number of bytes operated on by the body in the most
> > + * recent iteration. If no bytes were operated on, it may be set to
> > + * a negative errno. 0 is treated as -EIO.
> > + * @flags: Zero or more of the iomap_begin flags above.
> > + * @iomap: ...
> > + * @srcma:s ...
>
> ...? :)
I ran out of tuits. If this approach makes people happy, I can finish it
off.
> > + */
> > +struct iomap_iter {
> > + struct inode *inode;
> > + loff_t pos;
> > + u64 len;
>
> Thanks for leaving this u64 :)
>
> > + loff_t ret;
> > + ssize_t copied;
>
> Is this going to be sufficient for SEEK_{HOLE,DATA} when it wants to
> jump ahead by more than 2GB?
That's a good point. loff_t, I guess. Even though the current users
of iomap don't support extents larger than 2GB ;-)
> > + unsigned flags;
> > + struct iomap iomap;
> > + struct iomap srcmap;
> > +};
> > +
> > +#define IOMAP_ITER(name, _inode, _pos, _len, _flags) \
> > + struct iomap_iter name = { \
> > + .inode = _inode, \
> > + .pos = _pos, \
> > + .len = _len, \
> > + .flags = _flags, \
> > + }
> > +
> > +typedef int (*iomap_next_t)(const struct iomap_iter *,
> > + struct iomap *iomap, struct iomap *srcmap);
>
> I muttered in my other reply how these probably should be called
> iomap_iter_advance_t since they actually do the upper level work of
> advancing the iterator to the next mapping.
nono, the iterator is const! They don't move the iterator at all,
they just get the next iomap/srcmap.
