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.
