public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH] io_uring: support async buffered reads when readahead is disabled
@ 2020-09-29 12:00 Hao Xu
  2020-09-29 13:51 ` Jens Axboe
  0 siblings, 1 reply; 2+ messages in thread
From: Hao Xu @ 2020-09-29 12:00 UTC (permalink / raw)
  To: io-uring, axboe; +Cc: viro, akpm, hannes, linux-fsdevel, linux-mm, 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 <[email protected]>
---
 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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] io_uring: support async buffered reads when readahead is disabled
  2020-09-29 12:00 [PATCH] io_uring: support async buffered reads when readahead is disabled Hao Xu
@ 2020-09-29 13:51 ` Jens Axboe
  0 siblings, 0 replies; 2+ messages in thread
From: Jens Axboe @ 2020-09-29 13:51 UTC (permalink / raw)
  To: Hao Xu, io-uring; +Cc: viro, akpm, hannes, linux-fsdevel, linux-mm

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-29 13:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-29 12:00 [PATCH] io_uring: support async buffered reads when readahead is disabled Hao Xu
2020-09-29 13:51 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox