[PATCH] io_uring: support async buffered reads when readahead is disabled

[PATCH] io_uring: support async buffered reads when readahead is disabled

[Hao Xu]

The async buffered reads feature is not working when readahead is
turned off. There are two things to concern:

- when doing retry in io_read, not only the IOCB_WAITQ flag but also
  the IOCB_NOWAIT flag is still set, which makes it goes to would_block
  phase in generic_file_buffered_read() and then return -EAGAIN. After
  that, the io-wq thread work is queued, and later doing the async
  reads in the old way.

- even if we remove IOCB_NOWAIT when doing retry, the feature is still
  not running properly, since in generic_file_buffered_read() it goes to
  lock_page_killable() after calling mapping->a_ops->readpage() to do
  IO, and thus causing process to sleep.

Fixes: 1a0a7853b901 ("mm: support async buffered reads in generic_file_buffered_read()")
Fixes: 3b2a4439e0ae ("io_uring: get rid of kiocb_wait_page_queue_init()")
Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
 fs/io_uring.c | 1 +
 mm/filemap.c  | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 556e4a2ead07..e7e8ea58274e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3106,6 +3106,7 @@ static bool io_rw_should_retry(struct io_kiocb *req)
 	wait->wait.flags = 0;
 	INIT_LIST_HEAD(&wait->wait.entry);
 	kiocb->ki_flags |= IOCB_WAITQ;
+	kiocb->ki_flags &= ~IOCB_NOWAIT;
 	kiocb->ki_waitq = wait;
 
 	io_get_req_task(req);
diff --git a/mm/filemap.c b/mm/filemap.c
index 1aaea26556cc..ea383478fc22 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2267,7 +2267,11 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb,
 		}
 
 		if (!PageUptodate(page)) {
-			error = lock_page_killable(page);
+			if (iocb->ki_flags & IOCB_WAITQ)
+				error = lock_page_async(page, iocb->ki_waitq);
+			else
+				error = lock_page_killable(page);
+
 			if (unlikely(error))
 				goto readpage_error;
 			if (!PageUptodate(page)) {
-- 
1.8.3.1

Re: [PATCH] io_uring: support async buffered reads when readahead is disabled

[Jens Axboe]

On 9/29/20 6:00 AM, Hao Xu wrote:
> The async buffered reads feature is not working when readahead is
> turned off. There are two things to concern:
> 
> - when doing retry in io_read, not only the IOCB_WAITQ flag but also
>   the IOCB_NOWAIT flag is still set, which makes it goes to would_block
>   phase in generic_file_buffered_read() and then return -EAGAIN. After
>   that, the io-wq thread work is queued, and later doing the async
>   reads in the old way.
> 
> - even if we remove IOCB_NOWAIT when doing retry, the feature is still
>   not running properly, since in generic_file_buffered_read() it goes to
>   lock_page_killable() after calling mapping->a_ops->readpage() to do
>   IO, and thus causing process to sleep.

Thanks, this looks great, and avoids cases of io-wq punt where we don't
need it. I'm going to run this through full testing, but I think this
looks good to me.

-- 
Jens Axboe