[PATCH 1/2] ext4: Reduce stack usage in ext4_mpage_readpages()

[PATCH 1/2] ext4: Reduce stack usage in ext4_mpage_readpages()

[Matthew Wilcox (Oracle)]

This function is very similar to do_mpage_readpage() and a similar
approach to that taken in commit 12ac5a65cb56 will work.  As in
do_mpage_readpage(), we only use this array for checking block contiguity
and we can do that more efficiently with a little arithmetic.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ext4/readpage.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index 21e8f0aebb3c..fe29fb23a1d0 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -221,7 +221,7 @@ int ext4_mpage_readpages(struct inode *inode,
 	sector_t block_in_file;
 	sector_t last_block;
 	sector_t last_block_in_file;
-	sector_t blocks[MAX_BUF_PER_PAGE];
+	sector_t first_block;
 	unsigned page_block;
 	struct block_device *bdev = inode->i_sb->s_bdev;
 	int length;
@@ -263,6 +263,7 @@ int ext4_mpage_readpages(struct inode *inode,
 			unsigned map_offset = block_in_file - map.m_lblk;
 			unsigned last = map.m_len - map_offset;
 
+			first_block = map.m_pblk + map_offset;
 			for (relative_block = 0; ; relative_block++) {
 				if (relative_block == last) {
 					/* needed? */
@@ -271,8 +272,6 @@ int ext4_mpage_readpages(struct inode *inode,
 				}
 				if (page_block == blocks_per_page)
 					break;
-				blocks[page_block] = map.m_pblk + map_offset +
-					relative_block;
 				page_block++;
 				block_in_file++;
 			}
@@ -308,7 +307,9 @@ int ext4_mpage_readpages(struct inode *inode,
 				goto confused;		/* hole -> non-hole */
 
 			/* Contiguous blocks? */
-			if (page_block && blocks[page_block-1] != map.m_pblk-1)
+			if (!page_block)
+			       	first_block = map.m_pblk;
+			else if (first_block + page_block != map.m_pblk)
 				goto confused;
 			for (relative_block = 0; ; relative_block++) {
 				if (relative_block == map.m_len) {
@@ -317,7 +318,6 @@ int ext4_mpage_readpages(struct inode *inode,
 					break;
 				} else if (page_block == blocks_per_page)
 					break;
-				blocks[page_block] = map.m_pblk+relative_block;
 				page_block++;
 				block_in_file++;
 			}
@@ -340,7 +340,7 @@ int ext4_mpage_readpages(struct inode *inode,
 		 * This folio will go to BIO.  Do we need to send this
 		 * BIO off first?
 		 */
-		if (bio && (last_block_in_bio != blocks[0] - 1 ||
+		if (bio && (last_block_in_bio != first_block - 1 ||
 			    !fscrypt_mergeable_bio(bio, inode, next_block))) {
 		submit_and_realloc:
 			submit_bio(bio);
@@ -356,7 +356,7 @@ int ext4_mpage_readpages(struct inode *inode,
 			fscrypt_set_bio_crypt_ctx(bio, inode, next_block,
 						  GFP_KERNEL);
 			ext4_set_bio_post_read_ctx(bio, inode, folio->index);
-			bio->bi_iter.bi_sector = blocks[0] << (blkbits - 9);
+			bio->bi_iter.bi_sector = first_block << (blkbits - 9);
 			bio->bi_end_io = mpage_end_io;
 			if (rac)
 				bio->bi_opf |= REQ_RAHEAD;
@@ -372,7 +372,7 @@ int ext4_mpage_readpages(struct inode *inode,
 			submit_bio(bio);
 			bio = NULL;
 		} else
-			last_block_in_bio = blocks[blocks_per_page - 1];
+			last_block_in_bio = first_block + blocks_per_page - 1;
 		continue;
 	confused:
 		if (bio) {
-- 
2.43.0

[PATCH 2/2] ext4: Remove array of buffer_heads from mext_page_mkuptodate()

[Matthew Wilcox (Oracle)]

The current code iterates over the list of buffer_heads, finds the
!uptodate ones and reads them, waiting for each one before submitting
the next one.  Instead, submit all the read requests before waiting
for each of the needed ones.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ext4/move_extent.c | 45 ++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
index 7cd4afa4de1d..529061fa3fbf 100644
--- a/fs/ext4/move_extent.c
+++ b/fs/ext4/move_extent.c
@@ -166,15 +166,16 @@ mext_folio_double_lock(struct inode *inode1, struct inode *inode2,
 	return 0;
 }
 
-/* Force page buffers uptodate w/o dropping page's lock */
-static int
-mext_page_mkuptodate(struct folio *folio, unsigned from, unsigned to)
+/* Force folio buffers uptodate w/o dropping folio's lock */
+static int mext_page_mkuptodate(struct folio *folio, size_t from, size_t to)
 {
 	struct inode *inode = folio->mapping->host;
 	sector_t block;
-	struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE];
+	struct buffer_head *bh, *head;
 	unsigned int blocksize, block_start, block_end;
-	int i, err,  nr = 0, partial = 0;
+	int err = 0,  nr = 0;
+	bool partial = false;
+
 	BUG_ON(!folio_test_locked(folio));
 	BUG_ON(folio_test_writeback(folio));
 
@@ -186,13 +187,14 @@ mext_page_mkuptodate(struct folio *folio, unsigned from, unsigned to)
 	if (!head)
 		head = create_empty_buffers(folio, blocksize, 0);
 
-	block = (sector_t)folio->index << (PAGE_SHIFT - inode->i_blkbits);
-	for (bh = head, block_start = 0; bh != head || !block_start;
-	     block++, block_start = block_end, bh = bh->b_this_page) {
+	block = folio_pos(folio) >> inode->i_blkbits;
+	block_end = 0;
+	for (bh = head; bh != head; block++, bh = bh->b_this_page) {
+		block_start = block_end;
 		block_end = block_start + blocksize;
 		if (block_end <= from || block_start >= to) {
 			if (!buffer_uptodate(bh))
-				partial = 1;
+				partial = true;
 			continue;
 		}
 		if (buffer_uptodate(bh))
@@ -209,25 +211,28 @@ mext_page_mkuptodate(struct folio *folio, unsigned from, unsigned to)
 				continue;
 			}
 		}
-		BUG_ON(nr >= MAX_BUF_PER_PAGE);
-		arr[nr++] = bh;
+		ext4_read_bh_nowait(bh, 0, NULL);
+		nr++;
 	}
 	/* No io required */
 	if (!nr)
 		goto out;
 
-	for (i = 0; i < nr; i++) {
-		bh = arr[i];
-		if (!bh_uptodate_or_lock(bh)) {
-			err = ext4_read_bh(bh, 0, NULL);
-			if (err)
-				return err;
-		}
+	for (bh = head; bh != head; bh = bh->b_this_page) {
+		if (bh_offset(bh) + blocksize <= from)
+			continue;
+		if (bh_offset(bh) > to)
+			break;
+		wait_on_buffer(bh);
+		if (buffer_uptodate(bh))
+			continue;
+		err = -EIO;
+		break;
 	}
 out:
-	if (!partial)
+	if (!err && !partial)
 		folio_mark_uptodate(folio);
-	return 0;
+	return err;
 }
 
 /**
-- 
2.43.0

Re: [PATCH 2/2] ext4: Remove array of buffer_heads from mext_page_mkuptodate()

[Theodore Ts'o]

On Thu, May 16, 2024 at 07:16:51PM +0100, Matthew Wilcox (Oracle) wrote:
> The current code iterates over the list of buffer_heads, finds the
> !uptodate ones and reads them, waiting for each one before submitting
> the next one.  Instead, submit all the read requests before waiting
> for each of the needed ones.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Hey Willy,

This patch is causing ext4/020 (which tests the EXT4_IOC_MOVE_EXT
ioctl used by e4defrag).  This can be easily reproduced via:
"kvm-xfstests -c ext4/4k ext4/020".  From
/results/ext4/results-4k/ext4/020.out.bad:

   QA output created by 020
   wrote 1048576/1048576 bytes at offset 0
   XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
   wrote 1023/1023 bytes at offset 0
   XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
   md5sum: WARNING: 1 computed checksum did NOT match
   SCRATCH_MNT/020.orig: FAILED

I'm going to drop both this patch and the preceeding patch in this
series ("ext4: reduce stack usage in ext4_mpage_readpages()") pending
further investigation.

							- Ted

Re: [PATCH 2/2] ext4: Remove array of buffer_heads from mext_page_mkuptodate()

[Matthew Wilcox]

On Thu, Jun 27, 2024 at 04:20:22PM -0400, Theodore Ts'o wrote:
> This patch is causing ext4/020 (which tests the EXT4_IOC_MOVE_EXT
> ioctl used by e4defrag).  This can be easily reproduced via:
> "kvm-xfstests -c ext4/4k ext4/020".  From
> /results/ext4/results-4k/ext4/020.out.bad:
> 
>    QA output created by 020
>    wrote 1048576/1048576 bytes at offset 0
>    XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>    wrote 1023/1023 bytes at offset 0
>    XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>    md5sum: WARNING: 1 computed checksum did NOT match
>    SCRATCH_MNT/020.orig: FAILED
> 
> I'm going to drop both this patch and the preceeding patch in this
> series ("ext4: reduce stack usage in ext4_mpage_readpages()") pending
> further investigation.

Thanks.  I couldn't get kvm-xfstests to work, but I was able to run
ext4/020 using Kent's testsuite.  I found two bugs and fixed them.
I split this second patch into three patches for v2.