| Message ID | 20240516181651.2879778-2-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] ext4: Reduce stack usage in ext4_mpage_readpages() | expand |
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
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.
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; } /**
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(-)