Re: [PATCH v1] ext4: fix a assertion failure due to ungranted bh dirting

[Jan Kara <jack@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

Hello,

On Fri 11-10-24 12:08:58, Baolin Liu wrote:
> Greetings，
> 
> This problem is reproduced by our customer using their own testing tool
> “run_bug”. When I consulted with a client, the testing tool “run_bug”
> used a variety of background programs to benchmark (including memory
> pressure, cpu pressure, file cycle manipulation, fsstress Stress testing
> tool, postmark program，and so on).
> 
> The recurrence probability is relatively low.

OK, thanks for asking!

> In response to your query, in ext4_block_write_begin, the new state will
> be clear before get block, and the bh that failed get_block will not be
> set to new. However, when the page size is greater than the block size, a
> page will contain multiple bh. 

True. I wanted to argue that the buffer_new bit should be either cleared in
ext4_block_write_begin() (in case of error) or in
ext4_journalled_write_end() (in case of success) but actually
ext4_journalled_write_end() misses the clearing. So I think the better
solution is like the attached patch. I'll submit it once testing finishes
but it would be great if you could test that it fixes your problems as
well. Thanks!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[-- Attachment #2: 0001-ext4-Make-sure-BH_New-bit-is-cleared-in-write_end-ha.patch --]
[-- Type: text/x-patch, Size: 1853 bytes --]

From e93cba22bf3b576e6ac481d034b677188134bd7d Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Wed, 16 Oct 2024 12:24:03 +0200
Subject: [PATCH] ext4: Make sure BH_New bit is cleared in ->write_end handler

Currently we clear BH_New bit in case of error and also in the standard
ext4_write_end() handler (in block_commit_write()). However
ext4_journalled_write_end() misses this clearing and thus we are leaving
stale BH_New bits behind. Generally ext4_block_write_begin() clears
these bits before any harm can be done but in case blocksize < pagesize
and we hit some error when processing a page with these stale bits,
we'll try to zero buffers with these stale BH_New bits and jbd2 will
complain (as buffers were not prepared for writing in this transaction).
Fix the problem by clearing BH_New bits in ext4_journalled_write_end()
and WARN if ext4_block_write_begin() sees stale BH_New bits.

Reported-by: Baolin Liu <liubaolin12138@163.com>
Reported-by: Zhi Long <longzhi@sangfor.com.cn>
Fixes: 3910b513fcdf ("ext4: persist the new uptodate buffers in ext4_journalled_zero_new_buffers")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 54bdd4884fe6..aa56af4a92ad 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1049,7 +1049,7 @@ int ext4_block_write_begin(handle_t *handle, struct folio *folio,
 			}
 			continue;
 		}
-		if (buffer_new(bh))
+		if (WARN_ON_ONCE(buffer_new(bh)))
 			clear_buffer_new(bh);
 		if (!buffer_mapped(bh)) {
 			WARN_ON(bh->b_size != blocksize);
@@ -1265,6 +1265,7 @@ static int write_end_fn(handle_t *handle, struct inode *inode,
 	ret = ext4_dirty_journalled_data(handle, bh);
 	clear_buffer_meta(bh);
 	clear_buffer_prio(bh);
+	clear_buffer_new(bh);
 	return ret;
 }
 
-- 
2.35.3