lkml.org 
[lkml]   [2022]   [Dec]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [6.2][regression] after commit 947a629988f191807d2d22ba63ae18259bb645c5 btrfs volume periodical forced switch to readonly after a lot of disk writes
From


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,
QuFrom 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
\
 
 \ /
  Last update: 2023-03-26 23:21    [W:0.064 / U:0.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site