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

[Stefan Roesch <shr@meta.com>]

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