Re: [PATCH] mm: mark async iocb read as NOWAIT once some data has been, copied

[Jens Axboe <axboe@kernel.dk>]

On 10/17/20 1:13 PM, Kent Overstreet wrote:
> On Sat, Oct 17, 2020 at 11:30:47AM -0600, Jens Axboe wrote:
>> On 10/17/20 10:28 AM, Matthew Wilcox wrote:
>>> On Sat, Oct 17, 2020 at 09:30:59AM -0600, Jens Axboe wrote:
>>>> Once we've copied some data for an iocb that is marked with IOCB_WAITQ,
>>>> we should no longer attempt to async lock a new page. Instead make sure
>>>> we return the copied amount, and let the caller retry, instead of
>>>> returning -EIOCBQUEUED for a new page.
>>>>
>>>> This should only be possible with read-ahead disabled on the below
>>>> device, and multiple threads racing on the same file. Haven't been able
>>>> to reproduce on anything else.
>>>
>>> Wouldn't this do the job just as well?
>>>
>>> @@ -2211,6 +2211,8 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb,
>>>  
>>>                 put_page(page);
>>>                 written += ret;
>>> +               if (iocb->ki_flags & IOCB_WAITQ)
>>> +                       iocb->ki_flags |= IOCB_NOWAIT;
>>>                 if (!iov_iter_count(iter))
>>>                         goto out;
>>>                 if (ret < nr) {
>>
>> Indeed, that's cleaner. Let me re-test with that just to be on the safe
>> side, and I'll post a new one.
> 
> generic_file_buffered_read() has written passed in, which can be nonzero (DIO
> fallback) - we probably want the check to be at the top of the loop.

Yeah good point. That calling convention is... miserable. I'll move it up top.

-- 
Jens Axboe