Re: [PATCH] iomap: Do not use GFP_NORETRY to allocate BIOs

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Mon, Mar 23, 2020 at 06:20:52AM -0700, Christoph Hellwig wrote:
> On Mon, Mar 23, 2020 at 06:12:44AM -0700, Matthew Wilcox wrote:
> > From: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
> > 
> > If we use GFP_NORETRY, we have to be able to handle failures, and it's
> > tricky to handle failure here.  Other implementations of ->readpages
> > do not attempt to handle BIO allocation failures, so this is no worse.
> 
> do_mpage_readpage tries to use it, I guess that is wher I copied it
> from..

Oh, I see that now.  It uses readahead_gfp_mask(), and I was grepping for
GFP_NORETRY so I didn't spot it.  It falls back to block_read_full_page()
which we can't do.  That will allocate smaller BIOs, so there's an argument
that we should do the same.  How about this:

+++ b/fs/iomap/buffered-io.c
@@ -302,6 +302,7 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 
        if (!ctx->bio || !is_contig || bio_full(ctx->bio, plen)) {
                gfp_t gfp = mapping_gfp_constraint(page->mapping, GFP_KERNEL);
+               gfp_t orig_gfp = gfp;
                int nr_vecs = (length + PAGE_SIZE - 1) >> PAGE_SHIFT;
 
                if (ctx->bio)
@@ -310,6 +311,8 @@ iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
                if (ctx->is_readahead) /* same as readahead_gfp_mask */
                        gfp |= __GFP_NORETRY | __GFP_NOWARN;
                ctx->bio = bio_alloc(gfp, min(BIO_MAX_PAGES, nr_vecs));
+               if (!ctx->bio)
+                       ctx->bio = bio_alloc(orig_gfp, 1);
                ctx->bio->bi_opf = REQ_OP_READ;
                if (ctx->is_readahead)
                        ctx->bio->bi_opf |= REQ_RAHEAD;