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



On 2022/12/27 07:52, Mikhail Gavrilov wrote:
> On Tue, Dec 27, 2022 at 4:36 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>> 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.
>>
> 
> The kernel failed to built with v2 patch.
> 
> Here list of compilatore error messages:
> fs/btrfs/disk-io.c: In function ‘btrfs_submit_metadata_bio’:
> fs/btrfs/disk-io.c:5413:2: error: unterminated argument list invoking
> macro "WARN_ON"
>   5413 | }
>        |  ^
>    CC      net/ipv4/icmp.o
> fs/btrfs/disk-io.c:851:17: error: ‘WARN_ON’ undeclared (first use in
> this function)
>    851 |                 WARN_ON(!memcmp(&check, &bbio->parent_check,
>        |                 ^~~~~~~
> fs/btrfs/disk-io.c:851:17: note: each undeclared identifier is
> reported only once for each function it appears in
> fs/btrfs/disk-io.c:851:24: error: expected ‘;’ at end of input
>    851 |                 WARN_ON(!memcmp(&check, &bbio->parent_check,
>        |                        ^
>        |                        ;
> ......
> fs/btrfs/disk-io.c:851:17: error: expected declaration or statement at
> end of input
>    851 |                 WARN_ON(!memcmp(&check, &bbio->parent_check,
>        |                 ^~~~~~~
> fs/btrfs/disk-io.c:851:17: error: expected declaration or statement at
> end of input
> fs/btrfs/disk-io.c:845:22: warning: unused variable ‘ret’ [-Wunused-variable]
>    845 |         blk_status_t ret;
>        |                      ^~~
> fs/btrfs/disk-io.c:844:40: warning: unused variable ‘check’ [-Wunused-variable]
>    844 |         struct btrfs_tree_parent_check check = {0};
>        |                                        ^~~~~
> fs/btrfs/disk-io.c:842:31: warning: unused variable ‘fs_info’
> [-Wunused-variable]
>    842 |         struct btrfs_fs_info *fs_info = inode->root->fs_info;
>        |                               ^~~~~~~
>    HDRTEST usr/include/linux/loadpin.h
> fs/btrfs/disk-io.c: At top level:
> fs/btrfs/disk-io.c:63:13: warning: ‘btrfs_destroy_ordered_extents’
> declared ‘static’ but never defined [-Wunused-function]
>     63 | static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
>        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:64:12: warning: ‘btrfs_destroy_delayed_refs’
> declared ‘static’ but never defined [-Wunused-function]
>     64 | static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>        |            ^~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:66:13: warning: ‘btrfs_destroy_delalloc_inodes’
> declared ‘static’ but never defined [-Wunused-function]
>     66 | static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root);
>        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:67:12: warning: ‘btrfs_destroy_marked_extents’
> declared ‘static’ but never defined [-Wunused-function]
>     67 | static int btrfs_destroy_marked_extents(struct btrfs_fs_info *fs_info,
>        |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:70:12: warning: ‘btrfs_destroy_pinned_extent’
> declared ‘static’ but never defined [-Wunused-function]
>     70 | static int btrfs_destroy_pinned_extent(struct btrfs_fs_info *fs_info,
>        |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:72:12: warning: ‘btrfs_cleanup_transaction’
> declared ‘static’ but never defined [-Wunused-function]
>     72 | static int btrfs_cleanup_transaction(struct btrfs_fs_info *fs_info);
>        |            ^~~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:73:13: warning: ‘btrfs_error_commit_super’ declared
> ‘static’ but never defined [-Wunused-function]
>     73 | static void btrfs_error_commit_super(struct btrfs_fs_info *fs_info);
>        |             ^~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:828:13: warning: ‘should_async_write’ defined but
> not used [-Wunused-function]
>    828 | static bool should_async_write(struct btrfs_fs_info *fs_info,
>        |             ^~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:161:13: warning: ‘btrfs_supported_super_csum’
> defined but not used [-Wunused-function]
>    161 | static bool btrfs_supported_super_csum(u16 csum_type)
>        |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:130:12: warning: ‘verify_parent_transid’ defined
> but not used [-Wunused-function]
>    130 | static int verify_parent_transid(struct extent_io_tree *io_tree,
>        |            ^~~~~~~~~~~~~~~~~~~~~
> fs/btrfs/disk-io.c:75:13: warning: ‘btrfs_free_csum_hash’ defined but
> not used [-Wunused-function]
>     75 | static void btrfs_free_csum_hash(struct btrfs_fs_info *fs_info)
>        |             ^~~~~~~~~~~~~~~~~~~~
>    CC      kernel/notifier.o
> make[3]: *** [scripts/Makefile.build:252: fs/btrfs/disk-io.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
> 
> 

My bad, missing a ")" in the WARN_ON() line.

Fixed with v3 patch.

Thanks,
Qu

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

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