From 13bd691421bc191a402d2e0d3da5f248d170a632 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 17 Oct 2020 08:31:29 -0600 Subject: mm: mark async iocb read as NOWAIT once some data has been copied 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. Instead make sure we return the copied amount, and let the caller retry, instead of returning -EIOCBQUEUED for a new page. This should only be possible with read-ahead disabled on the below device, and multiple threads racing on the same file. Haven't been able to reproduce on anything else. Cc: stable@vger.kernel.org # v5.9 Fixes: 1a0a7853b901 ("mm: support async buffered reads in generic_file_buffered_read()") Reported-by: Kent Overstreet Signed-off-by: Jens Axboe --- mm/filemap.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'mm') diff --git a/mm/filemap.c b/mm/filemap.c index 1a6beaf69f49..e4101b5bfa82 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -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; + for (;;) { struct page *page; pgoff_t end_index; -- cgit From 324bcf54c449c7b5b7024c9fa4549fbaaae1935d Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 17 Oct 2020 09:25:52 -0600 Subject: mm: use limited read-ahead to satisfy read For the case where read-ahead is disabled on the file, or if the cgroup is congested, ensure that we can at least do 1 page of read-ahead to make progress on the read in an async fashion. This could potentially be larger, but it's not needed in terms of functionality, so let's error on the side of caution as larger counts of pages may run into reclaim issues (particularly if we're congested). This makes sure we're not hitting the potentially sync ->readpage() path for IO that is marked IOCB_WAITQ, which could cause us to block. It also means we'll use the same path for IO, regardless of whether or not read-ahead happens to be disabled on the lower level device. Acked-by: Johannes Weiner Reported-by: Matthew Wilcox (Oracle) Reported-by: Hao_Xu [axboe: updated for new ractl API] Signed-off-by: Jens Axboe --- mm/readahead.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'mm') diff --git a/mm/readahead.c b/mm/readahead.c index c6ffb76827da..c5b0457415be 100644 --- a/mm/readahead.c +++ b/mm/readahead.c @@ -552,15 +552,23 @@ readit: void page_cache_sync_ra(struct readahead_control *ractl, struct file_ra_state *ra, unsigned long req_count) { - /* no read-ahead */ - if (!ra->ra_pages) - return; + bool do_forced_ra = ractl->file && (ractl->file->f_mode & FMODE_RANDOM); - if (blk_cgroup_congested()) - return; + /* + * Even if read-ahead is disabled, issue this request as read-ahead + * as we'll need it to satisfy the requested range. The forced + * read-ahead will do the right thing and limit the read to just the + * requested range, which we'll set to 1 page for this case. + */ + if (!ra->ra_pages || blk_cgroup_congested()) { + if (!ractl->file) + return; + req_count = 1; + do_forced_ra = true; + } /* be dumb */ - if (ractl->file && (ractl->file->f_mode & FMODE_RANDOM)) { + if (do_forced_ra) { force_page_cache_ra(ractl, ra, req_count); return; } -- cgit