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

[Johannes Weiner <hannes@cmpxchg.org>]

On Sat, Oct 17, 2020 at 02:07:26PM -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.

It could be useful to elaborate on the (user-visible) failure scenario
here a bit, as I don't think it's obvious.

> Instead make sure we return the copied amount, and let the caller
> retry, instead of returning -EIOCBQUEUED for a new page.

We *wouldn't* return -EIOCBQUEUED, though, would we? We'd do the async
path, put the caller on a waitqueue, but then return `written' instead
of letting it know.

> @@ -2199,6 +2199,14 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb,
>  	last_index = (*ppos + iter->count + PAGE_SIZE-1) >> PAGE_SHIFT;
>  	offset = *ppos & ~PAGE_MASK;
>  
> +	/*
> +	 * If we've already successfully copied some data, then we
> +	 * can no longer safely return -EIOCBQUEUED. Hence mark
> +	 * an async read NOWAIT at that point.
> +	 */
> +	if (written && (iocb->ki_flags & IOCB_WAITQ))
> +		iocb->ki_flags |= IOCB_NOWAIT;

That looks correct to me, FWIW. It took a second to verify with all
spaghetti in this function :-) But the ra/!uptodate path already has
its own guard, and this is needed for the readpage fallback.