[v2,2/8] fs/buffer: remove batching from async read

Commit Message

Luis Chamberlain Feb. 4, 2025, 11:12 p.m. UTC

From: Matthew Wilcox <[e-mail]>

The current implementation of a folio async read in block_read_full_folio()
first batches all buffer-heads which need IOs issued for by putting them on an
array of max size MAX_BUF_PER_PAGE. After collection it locks the batched
buffer-heads and finally submits the pending reads. On systems with CPUs
where the system page size is quite larger like Hexagon with 256 KiB this
batching can lead stack growth warnings so we want to avoid that.

Note the use of folio_end_read() through block_read_full_folio(), its
used either when the folio is determined to be fully uptodate and no
pending read is needed, an IO error happened on get_block(), or an out of
bound read raced against batching collection to make our required reads
uptodate.

We can simplify this logic considerably and remove the stack growth
issues of MAX_BUF_PER_PAGE by just replacing the batched logic with
one which only issues IO for the previous buffer-head keeping in mind
we'll always have one buffer-head (the current one) on the folio with
an async flag, this will prevent any calls to folio_end_read().

So we accomplish two things with this:

 o Avoid large stacks arrays with MAX_BUF_PER_PAGE
 o Make the need for folio_end_read() explicit and easier to read

Suggested-by: Matthew Wilcox <[e-mail]>
Signed-off-by: Luis Chamberlain <[e-mail]>
---
 fs/buffer.c | 51 +++++++++++++++++++++------------------------------
 1 file changed, 21 insertions(+), 30 deletions(-)

Patch

diff --git a/fs/buffer.c b/fs/buffer.c
index b99560e8a142..167fa3e33566 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2361,9 +2361,8 @@  int block_read_full_folio(struct folio *folio, get_block_t *get_block)
 {
 	struct inode *inode = folio->mapping->host;
 	sector_t iblock, lblock;
-	struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
+	struct buffer_head *bh, *head, *prev = NULL;
 	size_t blocksize;
-	int nr, i;
 	int fully_mapped = 1;
 	bool page_error = false;
 	loff_t limit = i_size_read(inode);
@@ -2380,7 +2379,6 @@  int block_read_full_folio(struct folio *folio, get_block_t *get_block)
 	iblock = div_u64(folio_pos(folio), blocksize);
 	lblock = div_u64(limit + blocksize - 1, blocksize);
 	bh = head;
-	nr = 0;
 
 	do {
 		if (buffer_uptodate(bh))
@@ -2410,40 +2408,33 @@  int block_read_full_folio(struct folio *folio, get_block_t *get_block)
 			if (buffer_uptodate(bh))
 				continue;
 		}
-		arr[nr++] = bh;
+
+		lock_buffer(bh);
+		if (buffer_uptodate(bh)) {
+			unlock_buffer(bh);
+			continue;
+		}
+
+		mark_buffer_async_read(bh);
+		if (prev)
+			submit_bh(REQ_OP_READ, prev);
+		prev = bh;
 	} while (iblock++, (bh = bh->b_this_page) != head);
 
 	if (fully_mapped)
 		folio_set_mappedtodisk(folio);
 
-	if (!nr) {
-		/*
-		 * All buffers are uptodate or get_block() returned an
-		 * error when trying to map them - we can finish the read.
-		 */
-		folio_end_read(folio, !page_error);
-		return 0;
-	}
-
-	/* Stage two: lock the buffers */
-	for (i = 0; i < nr; i++) {
-		bh = arr[i];
-		lock_buffer(bh);
-		mark_buffer_async_read(bh);
-	}
-
 	/*
-	 * Stage 3: start the IO.  Check for uptodateness
-	 * inside the buffer lock in case another process reading
-	 * the underlying blockdev brought it uptodate (the sct fix).
+	 * All buffers are uptodate or get_block() returned an error
+	 * when trying to map them - we must finish the read because
+	 * end_buffer_async_read() will never be called on any buffer
+	 * in this folio.
 	 */
-	for (i = 0; i < nr; i++) {
-		bh = arr[i];
-		if (buffer_uptodate(bh))
-			end_buffer_async_read(bh, 1);
-		else
-			submit_bh(REQ_OP_READ, bh);
-	}
+	if (prev)
+		submit_bh(REQ_OP_READ, prev);
+	else
+		folio_end_read(folio, !page_error);
+
 	return 0;
 }
 EXPORT_SYMBOL(block_read_full_folio);