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: 1114 bytes --]



On 2022/12/27 09:35, Mikhail Gavrilov wrote:
> On Tue, Dec 27, 2022 at 5:18 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>> My bad, missing a ")" in the WARN_ON() line.
>>
>> Fixed with v3 patch.
>>
> 
> New kernel logs are ready.
> 

The result doesn't make sense...

A lot of read_block_for_search() and btrfs_read_node_slot() are 
triggering the warning.

But inside both functions, we have just set the numbers before the call:

In read_block_for_search() we got:

         check.has_first_key = true;
         check.level = parent_level - 1;
         check.transid = gen;
         check.owner_root = root->root_key.objectid;

Thus at least check.has_first_key is always true, and the WARN_ON() 
should never get triggered.
The same applies to btrfs_read_node_slot().

It looks like something involved in memory barrier.

Anyway, the latest debug patch has extra mb to be sure.

And despite the possible fix, could you provide extra info of your 
build? Include:

- Hardware (mostly CPU and RAM spec)
- Toolchain used to compile the kernel (include compiler and its
   version)
- Kernel config

Thanks,
Qu

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

From 9d81bfe48291964eaf658c08a71f04c3839592d0 Mon Sep 17 00:00:00 2001
Message-Id: <9d81bfe48291964eaf658c08a71f04c3839592d0.1672117970.git.wqu@suse.com>
From: Qu Wenruo <wqu@suse.com>
Date: Mon, 26 Dec 2022 16:44:08 +0800
Subject: [PATCH v4] 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.

v3:
- Fix a compiling error

v4:
- Add extra mb for test
---
 fs/btrfs/disk-io.c   | 19 +++++++++++++++++--
 fs/btrfs/extent_io.c | 15 ++++++++++++++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index f8b5955f003f..49b077acf359 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..2920642df2a3 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4878,6 +4878,7 @@ static int read_extent_buffer_subpage(struct extent_buffer *eb, int wait,
 		 */
 		atomic_dec(&eb->io_pages);
 	}
+	smp_mb();
 	memcpy(&btrfs_bio(bio_ctrl.bio)->parent_check, check, sizeof(*check));
 	submit_one_bio(&bio_ctrl);
 	if (ret || wait != WAIT_COMPLETE) {
@@ -4996,6 +4997,7 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int wait, int mirror_num,
 		}
 	}
 
+	smp_mb();
 	memcpy(&btrfs_bio(bio_ctrl.bio)->parent_check, check, sizeof(*check));
 	submit_one_bio(&bio_ctrl);
 
@@ -5005,8 +5007,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