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
