Re: [PATCH 1/2] iomap: simplify iomap_readpage_actor

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

On Thu, Jul 22, 2021 at 07:42:55AM +0200, Christoph Hellwig wrote:
> Now that the outstanding reads are counted in bytes, there is no need
> to use the low-level __bio_try_merge_page API, we can switch back to
> always using bio_add_page and simply iomap_readpage_actor again.

s/simply/simplify/ ?

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>

With that corrected,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/iomap/buffered-io.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 0597f5c186a33f..7898c1c47370e6 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -245,7 +245,6 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>  	struct iomap_readpage_ctx *ctx = data;
>  	struct page *page = ctx->cur_page;
>  	struct iomap_page *iop;
> -	bool same_page = false, is_contig = false;
>  	loff_t orig_pos = pos;
>  	unsigned poff, plen;
>  	sector_t sector;
> @@ -269,16 +268,10 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>  	if (iop)
>  		atomic_add(plen, &iop->read_bytes_pending);
>  
> -	/* Try to merge into a previous segment if we can */
>  	sector = iomap_sector(iomap, pos);
> -	if (ctx->bio && bio_end_sector(ctx->bio) == sector) {
> -		if (__bio_try_merge_page(ctx->bio, page, plen, poff,
> -				&same_page))
> -			goto done;
> -		is_contig = true;
> -	}
> -
> -	if (!is_contig || bio_full(ctx->bio, plen)) {
> +	if (!ctx->bio ||
> +	    bio_end_sector(ctx->bio) != sector ||
> +	    bio_add_page(ctx->bio, page, plen, poff) != plen) {
>  		gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
>  		gfp_t orig_gfp = gfp;
>  		unsigned int nr_vecs = DIV_ROUND_UP(length, PAGE_SIZE);
> @@ -302,9 +295,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
>  		ctx->bio->bi_iter.bi_sector = sector;
>  		bio_set_dev(ctx->bio, iomap->bdev);
>  		ctx->bio->bi_end_io = iomap_read_end_io;
> +		__bio_add_page(ctx->bio, page, plen, poff);
>  	}
> -
> -	bio_add_page(ctx->bio, page, plen, poff);
>  done:
>  	/*
>  	 * Move the caller beyond our range so that it keeps making progress.
> -- 
> 2.30.2
>