public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH RFC] mm: fix the sync buffered read to the old way
@ 2020-10-22 13:58 Hao Xu
  2020-10-22 14:10 ` Matthew Wilcox
  0 siblings, 1 reply; 3+ messages in thread
From: Hao Xu @ 2020-10-22 13:58 UTC (permalink / raw)
  To: Jens Axboe, io-uring; +Cc: linux-mm, Johannes Weiner, Andrew Morton

The commit 324bcf54c449 changed the code path of async buffered reads
to go with the page_cache_sync_readahead() way when readahead is
disabled, meanwhile the sync buffered reads are forced to do IO in the
above way as well, which makes it go to a more complex code path.

Fixes: 324bcf54c449 ("mm: use limited read-ahead to satisfy read")
Signed-off-by: Hao Xu <[email protected]>
---

Hi Jens,
I see it from the commit 324bcf54c449 ("mm: use limited read-ahead to
satisfy read") that we have forced normal sync buffered reads go with
the page_cache_sync_readahead() when readahead is disabled. I'm not
sure if this is what you expected. Here I changed the sync buffered
reads to go with the old code path(a_ops->readpage()), and tested the
performance of them, the results of IOPS and cpu time are similar. I
need your opinion on this.

 mm/filemap.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index e4101b5bfa82..0b2a0f633c01 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2224,9 +2224,14 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb,
 		if (!page) {
 			if (iocb->ki_flags & IOCB_NOIO)
 				goto would_block;
-			page_cache_sync_readahead(mapping,
-					ra, filp,
-					index, last_index - index);
+			/*
+			 * when readahead is disabled and IOCB_WAITQ isn't set
+			 * we should go with the readpage() way.
+			 */
+			if (ra->ra_pages || (iocb->ki_flags & IOCB_WAITQ))
+				page_cache_sync_readahead(mapping,
+						ra, filp,
+						index, last_index - index);
 			page = find_get_page(mapping, index);
 			if (unlikely(page == NULL))
 				goto no_cached_page;
-- 
1.8.3.1


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

* Re: [PATCH RFC] mm: fix the sync buffered read to the old way
  2020-10-22 13:58 [PATCH RFC] mm: fix the sync buffered read to the old way Hao Xu
@ 2020-10-22 14:10 ` Matthew Wilcox
  2020-10-22 14:55   ` Hao_Xu
  0 siblings, 1 reply; 3+ messages in thread
From: Matthew Wilcox @ 2020-10-22 14:10 UTC (permalink / raw)
  To: Hao Xu; +Cc: Jens Axboe, io-uring, linux-mm, Johannes Weiner, Andrew Morton

On Thu, Oct 22, 2020 at 09:58:34PM +0800, Hao Xu wrote:
> The commit 324bcf54c449 changed the code path of async buffered reads
> to go with the page_cache_sync_readahead() way when readahead is
> disabled, meanwhile the sync buffered reads are forced to do IO in the
> above way as well, which makes it go to a more complex code path.

But ->readpage is (increasingly) synchronous while readahead should be
used to start async I/Os.  I'm pretty sure Jens meant to do exactly what
he did.


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

* Re: [PATCH RFC] mm: fix the sync buffered read to the old way
  2020-10-22 14:10 ` Matthew Wilcox
@ 2020-10-22 14:55   ` Hao_Xu
  0 siblings, 0 replies; 3+ messages in thread
From: Hao_Xu @ 2020-10-22 14:55 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Jens Axboe, io-uring, linux-mm, Johannes Weiner, Andrew Morton

在 2020/10/22 下午10:10, Matthew Wilcox 写道:
> On Thu, Oct 22, 2020 at 09:58:34PM +0800, Hao Xu wrote:
>> The commit 324bcf54c449 changed the code path of async buffered reads
>> to go with the page_cache_sync_readahead() way when readahead is
>> disabled, meanwhile the sync buffered reads are forced to do IO in the
>> above way as well, which makes it go to a more complex code path.
> 
> But ->readpage is (increasingly) synchronous while readahead should be
> used to start async I/Os.  I'm pretty sure Jens meant to do exactly what
> he did.
Yes, we should start async I/Os with readahead, but why should we do the 
sync I/O like syscall read() in this way too when ra->ra_pages is 0?
> 

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

end of thread, other threads:[~2020-10-22 14:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-22 13:58 [PATCH RFC] mm: fix the sync buffered read to the old way Hao Xu
2020-10-22 14:10 ` Matthew Wilcox
2020-10-22 14:55   ` Hao_Xu

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