[07/13] mm: refactor generic_file_buffered_read_get_pages

Commit Message

Christoph Hellwig Oct. 31, 2020, 8:59 a.m. UTC

Move the call to filemap_make_page_uptodate for a newly allocated page
into filemap_new_page, which turns the new vs lookup decision into a
plain if / else statement, rename two identifier to be more obvious
and the function itself to filemap_read_pages which describes
it a little better while being much shorter.

Signed-off-by: Christoph Hellwig <[e-mail]>
---
 mm/filemap.c | 52 ++++++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

Comments

Matthew Wilcox Nov. 1, 2020, 11:18 a.m. UTC | #1

On Sat, Oct 31, 2020 at 09:59:58AM +0100, Christoph Hellwig wrote:
> Move the call to filemap_make_page_uptodate for a newly allocated page
> into filemap_new_page, which turns the new vs lookup decision into a
> plain if / else statement, rename two identifier to be more obvious
> and the function itself to filemap_read_pages which describes
> it a little better while being much shorter.

We don't need to do this -- filemap_read_page waits for the page to
become unlocked, so we already know that it's uptodate (or an error!)
and we know it isn't going to have the readahead bit set.

Patch

diff --git a/mm/filemap.c b/mm/filemap.c
index 0af7ddaa0fe7ba..96855299247c56 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2320,7 +2320,10 @@  static int filemap_new_page(struct kiocb *iocb, struct iov_iter *iter,
 		return error;
 	}
 
-	return filemap_readpage(iocb, *page);
+	error = filemap_readpage(iocb, *page);
+	if (error)
+		return error;
+	return filemap_make_page_uptodate(iocb, iter, *page, index, true);
 }
 
 static int filemap_find_get_pages(struct kiocb *iocb, struct iov_iter *iter,
@@ -2344,40 +2347,38 @@  static int filemap_find_get_pages(struct kiocb *iocb, struct iov_iter *iter,
 	return find_get_pages_contig(mapping, index, nr, pages);
 }
 
-static int generic_file_buffered_read_get_pages(struct kiocb *iocb,
-						struct iov_iter *iter,
-						struct page **pages,
-						unsigned int nr)
+static int filemap_read_pages(struct kiocb *iocb, struct iov_iter *iter,
+		struct page **pages, unsigned int nr)
 {
 	pgoff_t index = iocb->ki_pos >> PAGE_SHIFT;
-	int i, j, nr_got, err = 0;
+	int nr_pages, err = 0, i, j;
 
-find_page:
+retry:
 	if (fatal_signal_pending(current))
 		return -EINTR;
 
-	nr_got = filemap_find_get_pages(iocb, iter, pages, nr);
-	if (!nr_got) {
+	nr_pages = filemap_find_get_pages(iocb, iter, pages, nr);
+	if (nr_pages) {
+		for (i = 0; i < nr_pages; i++) {
+			err = filemap_make_page_uptodate(iocb, iter, pages[i],
+					index + i, i == 0);
+			if (err) {
+				for (j = i + 1; j < nr_pages; j++)
+					put_page(pages[j]);
+				nr_pages = i;
+				break;
+			}
+		}
+	} else {
 		err = filemap_new_page(iocb, iter, &pages[0]);
 		if (!err)
-			nr_got = 1;
+			nr_pages = 1;
 	}
 
-	for (i = 0; i < nr_got; i++) {
-		err = filemap_make_page_uptodate(iocb, iter, pages[i],
-				index + i, i == 0);
-		if (err) {
-			for (j = i + 1; j < nr_got; j++)
-				put_page(pages[j]);
-			nr_got = i;
-			break;
-		}
-	}
-
-	if (likely(nr_got))
-		return nr_got;
+	if (likely(nr_pages))
+		return nr_pages;
 	if (err == -EEXIST || err == AOP_TRUNCATED_PAGE)
-		goto find_page;
+		goto retry;
 	return err;
 }
 
@@ -2436,8 +2437,7 @@  ssize_t generic_file_buffered_read(struct kiocb *iocb,
 			iocb->ki_flags |= IOCB_NOWAIT;
 
 		i = 0;
-		pg_nr = generic_file_buffered_read_get_pages(iocb, iter,
-							     pages, nr_pages);
+		pg_nr = filemap_read_pages(iocb, iter, pages, nr_pages);
 		if (pg_nr < 0) {
 			error = pg_nr;
 			break;