From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Nikolay Borisov <nborisov@suse.com>, Qu Wenruo <wqu@suse.com>,
linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v5 00/12] btrfs: Enhancement to tree block validation
Date: Fri, 15 Feb 2019 18:19:58 +0100 [thread overview]
Message-ID: <20190215171958.GG9874@twin.jikos.cz> (raw)
In-Reply-To: <18beb5f9-48f5-e519-6abf-08bd1509cde5@gmx.com>
On Fri, Feb 15, 2019 at 09:18:03PM +0800, Qu Wenruo wrote:
>
>
> On 2019/2/15 下午9:10, Nikolay Borisov wrote:
> >
> >
> > On 15.02.19 г. 12:50 ч., Qu Wenruo wrote:
> >> Patchset can be fetched from github:
> >> https://github.com/adam900710/linux/tree/write_time_tree_checker
> >> Which is based on v5.0-rc1 tag.
> >> Also there is no conflict rebasing the patchset to misc-next.
> >>
> >> This patchset has the following 3 features:
> >> - Tree block validation output enhancement
> >> * Output validation failure timing (write time or read time)
> >> * Always output tree block level/key mismatch error message
> >> This part is already submitted and reviewed.
> >>
> >> - Write time tree block validation check
> >> To catch memory corruption either from hardware or kernel.
> >> Example output would be:
> >>
> >> BTRFS critical (device dm-3): corrupt leaf: root=2 block=1350630375424 slot=68, bad key order, prev (10510212874240 169 0) current (1714119868416 169 0)
> >> BTRFS error (device dm-3): write time tree block corruption detected
> > This is not good. Those two error messages should be collapsed into
> > one. Otherwise it's hard to actually match them up.
>
> That shouldn't be a problem, since the error won't happen so frequently
> there is no other error message that could interrupt these 2 lines.
>
> > Better output will
> > be "Corrupt leaf detected during writing: root=..." and eliminate "write
> > time tree block corruption detected" line. Is that feasible?
>
> Feasible, currently tree checker only get called in 3 locations:
> 1) read time full checker
> 2) mark dirty time basic checker
> 3) write time full checker
>
> And they all have different internal bool to indicate the timing, so
> it's possible to output the timing.
>
> But that needs to pass the internal bool down a long long way, for all
> the output help to accept an extra string.
> I'm not a big fan for that, and prefer a timing neutral tree checker.
I'd rather not merge the error messages, as we'll possibly add more
sanity checks to various functions so there could be a list of problems
and there's one final note about when it happened (read time/write
time).
Matching the lines together is desirable though, so if the block number
could be part of all messages, I hope this makes it usable for analysis.
Reading btree_readpage_end_io_hook, the message should be under the err:
label, as there are 3 other possible messages printed (bad block start,
fsid and level).
next prev parent reply other threads:[~2019-02-15 17:18 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-15 10:50 [PATCH v5 00/12] btrfs: Enhancement to tree block validation Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 01/12] btrfs: Always output error message when key/level verification fails Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 02/12] btrfs: extent_io: Kill the forward declaration of flush_write_bio() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 03/12] btrfs: disk-io: Show the timing of corrupted tree block explicitly Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 04/12] btrfs: extent_io: Move the BUG_ON() in flush_write_bio() one level up Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 05/12] btrfs: extent_io: Handle error better in extent_write_full_page() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 06/12] btrfs: extent_io: Handle error better in btree_write_cache_pages() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 07/12] btrfs: extent_io: Kill the dead branch in extent_write_cache_pages() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 08/12] btrfs: extent_io: Handle error better in extent_write_locked_range() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 09/12] btrfs: extent_io: Kill the BUG_ON() in lock_extent_buffer_for_io() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 10/12] btrfs: extent_io: Kill the BUG_ON() in extent_write_cache_pages() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 11/12] btrfs: extent_io: Handle error better in extent_writepages() Qu Wenruo
2019-02-15 10:50 ` [PATCH v5 12/12] btrfs: Do mandatory tree block check before submitting bio Qu Wenruo
2019-02-15 13:10 ` [PATCH v5 00/12] btrfs: Enhancement to tree block validation Nikolay Borisov
2019-02-15 13:18 ` Qu Wenruo
2019-02-15 17:19 ` David Sterba [this message]
2019-02-16 6:49 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190215171958.GG9874@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=nborisov@suse.com \
--cc=quwenruo.btrfs@gmx.com \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).