On Wed, May 31, 2023 at 04:50:42AM -0700, Johannes Thumshirn wrote:
> When the iomap buffered-io code can't add a folio to a bio, it allocates a
> new bio and adds the folio to that one. This is done using bio_add_folio(),
> but doesn't check for errors.
>
> As adding a folio to a newly created bio can't fail, use the newly
> introduced bio_add_folio_nofail() function.
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> ---
> fs/iomap/buffered-io.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 063133ec77f4..0edab9deae2a 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -312,7 +312,7 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter,
> ctx->bio->bi_opf |= REQ_RAHEAD;
> ctx->bio->bi_iter.bi_sector = sector;
> ctx->bio->bi_end_io = iomap_read_end_io;
> - bio_add_folio(ctx->bio, folio, plen, poff);
> + bio_add_folio_nofail(ctx->bio, folio, plen, poff);
> }
>
> done:
> @@ -539,7 +539,7 @@ static int iomap_read_folio_sync(loff_t block_start, struct folio *folio,
>
> bio_init(&bio, iomap->bdev, &bvec, 1, REQ_OP_READ);
> bio.bi_iter.bi_sector = iomap_sector(iomap, block_start);
> - bio_add_folio(&bio, folio, plen, poff);
> + bio_add_folio_nofail(&bio, folio, plen, poff);
> return submit_bio_wait(&bio);
> }
>
> @@ -1582,7 +1582,7 @@ iomap_add_to_ioend(struct inode *inode, loff_t pos, struct folio *folio,
>
> if (!bio_add_folio(wpc->ioend->io_bio, folio, len, poff)) {
> wpc->ioend->io_bio = iomap_chain_bio(wpc->ioend->io_bio);
> - bio_add_folio(wpc->ioend->io_bio, folio, len, poff);
> + bio_add_folio_nofail(wpc->ioend->io_bio, folio, len, poff);
> }
We lose adjacent page merging with this change.
We've had performance regressions in the past that have been
attributed to either the page allocator not handing out sequential
adjacent pages for sequential writes and/or bios not merging
adjacent pages. Some hardware is much more performant when it only
has to do a single large DMA instead of (potentially) hundreds of
single page DMAs for the same amount of data...
What performance regression testing has been done on this change?
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
