Re: [6.2][regression] after commit 947a629988f191807d2d22ba63ae18259bb645c5 btrfs volume periodical forced switch to readonly after a lot of disk writes

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

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



On 2022/12/26 20:15, Mikhail Gavrilov wrote:
> On Mon, Dec 26, 2022 at 5:11 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>> While at the caller, the structure is properly passed in.
>>
>> So there is something wrong between the endio function and the check.
>>
>> I have created the v2 version patch to debug, please apply without any
>> previous debug patch.
>>
>> Meanwhile this really looks like a race, thus I'm not 100% sure if my
>> debug patch would reduce the possibility to reproduce.
>>
>> Your bisect should be the determining evidence, for the worst case we
>> can revert the offending patch.
>>
>> Thank you very much for all of the testing, it really helps a lot.
>> Qu
>>
> 
> Looks like v2 version patch is missed in last message
> 
Oh, my bad.

Please be aware that, this newer version may cause extra kernel 
warnings, if it found any uninitialized btrfs_tree_parent_check 
structure at submit time.

But there are completely valid cases we can do that, thus it may cause 
much larger dmesg output, and you should not rely on the kernel warning 
to determine if the bug is triggered.

Thanks,
Qu

[-- Attachment #2: 0001-btrfs-add-extra-debug-for-level-mismatch.patch --]
[-- Type: text/x-patch, Size: 3780 bytes --]

From 273968610a4427559f74958ed456a00ea0f5fc74 Mon Sep 17 00:00:00 2001
Message-Id: <273968610a4427559f74958ed456a00ea0f5fc74.1672097559.git.wqu@suse.com>
From: Qu Wenruo <wqu@suse.com>
Date: Mon, 26 Dec 2022 16:44:08 +0800
Subject: [PATCH v2] btrfs: add extra debug for level mismatch

Currently I assume there is some race or uninitialized value for
check::level.

The extra output are for two locations:

- validate_extent_buffer()
  Output the error message for read error and the members of check.

- read_extent_buffer_pages()
  This will dump the stack for us to catch the offender.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v2:
- Extra submission time output
  This would greately enlarge the dmesg size

- Extra warning when submitting a metadata bio
  If we have an uninitialized check structure, do a warning and stack
  dump to show the offending call trace.
---
 fs/btrfs/disk-io.c   | 19 +++++++++++++++++--
 fs/btrfs/extent_io.c | 13 ++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f8b5955f003f..5023f213aa52 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -530,6 +530,10 @@ static int validate_extent_buffer(struct extent_buffer *eb,
 	}
 
 	if (found_level != check->level) {
+		btrfs_err(eb->fs_info,
+"level verify failed on logical %llu mirror %u wanted %u found %u",
+			  eb->start, eb->read_mirror, check->level,
+			  found_level);
 		ret = -EIO;
 		goto out;
 	}
@@ -581,13 +585,20 @@ static int validate_extent_buffer(struct extent_buffer *eb,
 	if (found_level > 0 && btrfs_check_node(eb))
 		ret = -EIO;
 
+out:
 	if (!ret)
 		set_extent_buffer_uptodate(eb);
-	else
+	else {
 		btrfs_err(fs_info,
 		"read time tree block corruption detected on logical %llu mirror %u",
 			  eb->start, eb->read_mirror);
-out:
+		btrfs_err(eb->fs_info,
+"check owner_root=%llu transid=%llu first_key=(%llu %u %llu) has_first_key=%d level=%u",
+			  check->owner_root,
+			  check->transid, check->first_key.objectid,
+			  check->first_key.type, check->first_key.offset,
+			  check->has_first_key, check->level);
+	}
 	return ret;
 }
 
@@ -652,6 +663,7 @@ int btrfs_validate_metadata_buffer(struct btrfs_bio *bbio,
 	int reads_done;
 
 	ASSERT(page->private);
+	WARN_ON(!bbio->is_metadata);
 
 	if (btrfs_sb(page->mapping->host->i_sb)->nodesize < PAGE_SIZE)
 		return validate_subpage_buffer(page, start, end, mirror,
@@ -833,12 +845,15 @@ void btrfs_submit_metadata_bio(struct btrfs_inode *inode, struct bio *bio, int m
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct btrfs_bio *bbio = btrfs_bio(bio);
+	struct btrfs_tree_parent_check check = {0};
 	blk_status_t ret;
 
 	bio->bi_opf |= REQ_META;
 	bbio->is_metadata = 1;
 
 	if (btrfs_op(bio) != BTRFS_MAP_WRITE) {
+		WARN_ON(!memcmp(&check, &bbio->parent_check,
+			sizeof(struct btrfs_tree_parent_check));
 		btrfs_submit_bio(fs_info, bio, mirror_num);
 		return;
 	}
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 83dd3aa59663..c94eb036dde4 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -5005,8 +5005,19 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int wait, int mirror_num,
 	for (i = 0; i < num_pages; i++) {
 		page = eb->pages[i];
 		wait_on_page_locked(page);
-		if (!PageUptodate(page))
+		if (!PageUptodate(page)) {
 			ret = -EIO;
+			btrfs_err(eb->fs_info,
+"read failed, bytenr=%llu check owner_root=%llu transid=%llu has_first_key=%d first_key=(%llu %u %llu) level=%u",
+				  eb->start,
+				  check->owner_root, check->transid,
+				  check->has_first_key,
+				  check->first_key.objectid,
+				  check->first_key.type,
+				  check->first_key.offset,
+				  check->level);
+			dump_stack();
+		}
 	}
 
 	return ret;
-- 
2.39.0