On 3/3/23 8:53 AM, Darrick J. Wong wrote: > > > 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 If IOMAP_NOWAIT is set, and the allocation fails, we should return -EAGAIN, so the write request is retried in the slow path. --Stefan
