Re: [RESEND PATCH v9 04/14] iomap: Add flags parameter to iomap_page_create()

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

On Fri, Mar 03, 2023 at 04:51:10AM +0000, Matthew Wilcox wrote:
> On Thu, Jun 23, 2022 at 10:51:47AM -0700, Stefan Roesch wrote:
> > Add the kiocb flags parameter to the function iomap_page_create().
> > Depending on the value of the flags parameter it enables different gfp
> > flags.
> > 
> > No intended functional changes in this patch.
> 
> [...]
> 
> > @@ -226,7 +234,7 @@ static int iomap_read_inline_data(const struct iomap_iter *iter,
> >  	if (WARN_ON_ONCE(size > iomap->length))
> >  		return -EIO;
> >  	if (offset > 0)
> > -		iop = iomap_page_create(iter->inode, folio);
> > +		iop = iomap_page_create(iter->inode, folio, iter->flags);
> >  	else
> >  		iop = to_iomap_page(folio);
> 
> I really don't like what this change has done to this file.  I'm
> modifying this function, and I start thinking "Well, hang on, if
> flags has IOMAP_NOWAIT set, then GFP_NOWAIT can fail, and iop
> will be NULL, so we'll end up marking the entire folio uptodate
> when really we should only be marking some blocks uptodate, so
> we should really be failing the entire read if the allocation
> failed, but maybe it's OK because IOMAP_NOWAIT is never set in
> this path".
> 
> I don't know how we fix this.  Maybe return ERR_PTR(-ENOMEM) or
> -EAGAIN if the memory allocation fails (leaving the NULL return
> for "we don't need an iop").  Thoughts?

I don't see any problem with that, aside from being pre-coffee and on
vacation for the rest of today. ;)

--D