Re: [PATCH v2 03/12] iomap: convert misc simple ops to incremental advance

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Feb 19, 2025 at 02:24:28PM -0800, Darrick J. Wong wrote:
> On Wed, Feb 19, 2025 at 12:50:41PM -0500, Brian Foster wrote:
> > Update several of the remaining iomap operations to advance the iter
> > directly rather than via return value. This includes page faults,
> > fiemap, seek data/hole and swapfile activation.
> > 
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > ---
> >  fs/iomap/buffered-io.c |  2 +-
> >  fs/iomap/fiemap.c      | 18 +++++++++---------
> >  fs/iomap/seek.c        | 12 ++++++------
> >  fs/iomap/swapfile.c    |  7 +++++--
> >  4 files changed, 21 insertions(+), 18 deletions(-)
> > 
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index 215866ba264d..ddc82dab6bb5 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -1480,7 +1480,7 @@ static loff_t iomap_folio_mkwrite_iter(struct iomap_iter *iter,
> >  		folio_mark_dirty(folio);
> >  	}
> >  
> > -	return length;
> > +	return iomap_iter_advance(iter, &length);
> 
> Same dorky question here -- doesn't iomap_iter_advance return int, so
> all these functions can now return int instead of loff_t?

Same dorky answer too!
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> 
> --D
> 
> >  }
> >  
> >  vm_fault_t iomap_page_mkwrite(struct vm_fault *vmf, const struct iomap_ops *ops)
> > diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
> > index 610ca6f1ec9b..8a0d8b034218 100644
> > --- a/fs/iomap/fiemap.c
> > +++ b/fs/iomap/fiemap.c
> > @@ -39,24 +39,24 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
> >  			iomap->length, flags);
> >  }
> >  
> > -static loff_t iomap_fiemap_iter(const struct iomap_iter *iter,
> > +static loff_t iomap_fiemap_iter(struct iomap_iter *iter,
> >  		struct fiemap_extent_info *fi, struct iomap *prev)
> >  {
> > +	u64 length = iomap_length(iter);
> >  	int ret;
> >  
> >  	if (iter->iomap.type == IOMAP_HOLE)
> > -		return iomap_length(iter);
> > +		goto advance;
> >  
> >  	ret = iomap_to_fiemap(fi, prev, 0);
> >  	*prev = iter->iomap;
> > -	switch (ret) {
> > -	case 0:		/* success */
> > -		return iomap_length(iter);
> > -	case 1:		/* extent array full */
> > -		return 0;
> > -	default:	/* error */
> > +	if (ret < 0)
> >  		return ret;
> > -	}
> > +	if (ret == 1)	/* extent array full */
> > +		return 0;
> > +
> > +advance:
> > +	return iomap_iter_advance(iter, &length);
> >  }
> >  
> >  int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
> > diff --git a/fs/iomap/seek.c b/fs/iomap/seek.c
> > index a845c012b50c..83c687d6ccc0 100644
> > --- a/fs/iomap/seek.c
> > +++ b/fs/iomap/seek.c
> > @@ -10,7 +10,7 @@
> >  #include <linux/pagemap.h>
> >  #include <linux/pagevec.h>
> >  
> > -static loff_t iomap_seek_hole_iter(const struct iomap_iter *iter,
> > +static loff_t iomap_seek_hole_iter(struct iomap_iter *iter,
> >  		loff_t *hole_pos)
> >  {
> >  	loff_t length = iomap_length(iter);
> > @@ -20,13 +20,13 @@ static loff_t iomap_seek_hole_iter(const struct iomap_iter *iter,
> >  		*hole_pos = mapping_seek_hole_data(iter->inode->i_mapping,
> >  				iter->pos, iter->pos + length, SEEK_HOLE);
> >  		if (*hole_pos == iter->pos + length)
> > -			return length;
> > +			return iomap_iter_advance(iter, &length);
> >  		return 0;
> >  	case IOMAP_HOLE:
> >  		*hole_pos = iter->pos;
> >  		return 0;
> >  	default:
> > -		return length;
> > +		return iomap_iter_advance(iter, &length);
> >  	}
> >  }
> >  
> > @@ -56,19 +56,19 @@ iomap_seek_hole(struct inode *inode, loff_t pos, const struct iomap_ops *ops)
> >  }
> >  EXPORT_SYMBOL_GPL(iomap_seek_hole);
> >  
> > -static loff_t iomap_seek_data_iter(const struct iomap_iter *iter,
> > +static loff_t iomap_seek_data_iter(struct iomap_iter *iter,
> >  		loff_t *hole_pos)
> >  {
> >  	loff_t length = iomap_length(iter);
> >  
> >  	switch (iter->iomap.type) {
> >  	case IOMAP_HOLE:
> > -		return length;
> > +		return iomap_iter_advance(iter, &length);
> >  	case IOMAP_UNWRITTEN:
> >  		*hole_pos = mapping_seek_hole_data(iter->inode->i_mapping,
> >  				iter->pos, iter->pos + length, SEEK_DATA);
> >  		if (*hole_pos < 0)
> > -			return length;
> > +			return iomap_iter_advance(iter, &length);
> >  		return 0;
> >  	default:
> >  		*hole_pos = iter->pos;
> > diff --git a/fs/iomap/swapfile.c b/fs/iomap/swapfile.c
> > index b90d0eda9e51..4395e46a4dc7 100644
> > --- a/fs/iomap/swapfile.c
> > +++ b/fs/iomap/swapfile.c
> > @@ -94,9 +94,11 @@ static int iomap_swapfile_fail(struct iomap_swapfile_info *isi, const char *str)
> >   * swap only cares about contiguous page-aligned physical extents and makes no
> >   * distinction between written and unwritten extents.
> >   */
> > -static loff_t iomap_swapfile_iter(const struct iomap_iter *iter,
> > +static loff_t iomap_swapfile_iter(struct iomap_iter *iter,
> >  		struct iomap *iomap, struct iomap_swapfile_info *isi)
> >  {
> > +	u64 length = iomap_length(iter);
> > +
> >  	switch (iomap->type) {
> >  	case IOMAP_MAPPED:
> >  	case IOMAP_UNWRITTEN:
> > @@ -132,7 +134,8 @@ static loff_t iomap_swapfile_iter(const struct iomap_iter *iter,
> >  			return error;
> >  		memcpy(&isi->iomap, iomap, sizeof(isi->iomap));
> >  	}
> > -	return iomap_length(iter);
> > +
> > +	return iomap_iter_advance(iter, &length);
> >  }
> >  
> >  /*
> > -- 
> > 2.48.1
> > 
> > 
>