Message ID | cover.1710409033.git.wqu@suse.com (mailing list archive) |
---|---|
Headers | show |
Series | btrfs: scrub: refine the error messages | expand |
On Thu, Mar 14, 2024 at 08:20:13PM +1030, Qu Wenruo wrote: > During some support sessions, I found older kernels are always report > very unuseful scrub error messages like: > > BTRFS error (device dm-0): bdev /dev/mapper/sys-rootlv errs: wr 0, rd 0, flush 0, corrupt 2823, gen 0 > BTRFS error (device dm-0): unable to fixup (regular) error at logical 1563504640 on dev /dev/mapper/sys-rootlv > BTRFS error (device dm-0): bdev /dev/mapper/sys-rootlv errs: wr 0, rd 0, flush 0, corrupt 2824, gen 0 > BTRFS error (device dm-0): unable to fixup (regular) error at logical 1579016192 on dev /dev/mapper/sys-rootlv > > There are two problems: > - No proper details about the corruption > No metadata or data indication, nor the filename or the tree id. > Even the involved kernel (and newer kernels after the scrub rework) > has the ability to do backref walk and find out the file path or the > tree backref. > > My guess is, for data sometimes the corrupted sector is no longer > referred by any data extent. > > - Too noisy and useless error message from > btrfs_dev_stat_inc_and_print() > I'd argue we should not output any error message just for > btrfs_dev_stat_inc_and_print(). > > After the series, the error message would look like this: > > BTRFS warning (device dm-2): checksum error at inode 5/257(file1) fileoff 16384, logical 13647872(1) physical 1(/dev/mapper/test-scratch1)13647872 Stoked on this series, thanks for doing it! qq: would it be helpful to also include the actual/expected csum? I think it's particularly helpful when one or the other is either zeros or the checksum of the zero block. > > This involves the following enhancement: > > - Single line > And we ensure we output at least one line for the error we hit. > No more unrelated btrfs_dev_stat_inc_and_print() output. > > - Proper fall backs > We have the following different fall back options > * Repaired > Just a line of something repaired for logical/physical address. > > * Detailed data info > Including the following elements (if possible), and if higher > priority ones can not be fetched, it would be skipped and try > lower priority items: > + file path (can have multiple ones) > + root/inode number and offset > + logical/physical address (always output) > > * Detaile metadata info > The priority list is: > + root ref/level > + logical/physical address (always output) > > For the worst case of data corruption, we would still have some like: > > BTRFS warning (device dm-2): checksum error at data, logical 13647872(1) physical 1(/dev/mapper/test-scratch1)13647872 > > And similar ones for metadata: > > BTRFS warning (device dm-2): checksum error at meta, logical 13647872(1) physical 1(/dev/mapper/test-scratch1)13647872 > > The first patch is fixing a regression in the error message, which leads > to bad logical/physical bytenr. > The second one would reduce the log level for > btrfs_dev_stat_inc_and_print(). > > Path 3~4 are cleanups to remove unnecessary parameters. > > The remaining reworks the format and refine the error message frequency. > > Qu Wenruo (7): > btrfs: scrub: fix incorrectly reported logical/physical address > btrfs: reduce the log level for btrfs_dev_stat_inc_and_print() > btrfs: scrub: remove unused is_super parameter from > scrub_print_common_warning() > btrfs: scrub: remove unnecessary dev/physical lookup for > scrub_stripe_report_errors() > btrfs: scrub: simplify the inode iteration output > btrfs: scrub: unify and shorten the error message > btrfs: scrub: fix the frequency of error messages > > fs/btrfs/scrub.c | 185 ++++++++++++++++----------------------------- > fs/btrfs/volumes.c | 2 +- > 2 files changed, 66 insertions(+), 121 deletions(-) > > -- > 2.44.0 >
在 2024/3/15 04:05, Boris Burkov 写道: > On Thu, Mar 14, 2024 at 08:20:13PM +1030, Qu Wenruo wrote: >> During some support sessions, I found older kernels are always report >> very unuseful scrub error messages like: >> >> BTRFS error (device dm-0): bdev /dev/mapper/sys-rootlv errs: wr 0, rd 0, flush 0, corrupt 2823, gen 0 >> BTRFS error (device dm-0): unable to fixup (regular) error at logical 1563504640 on dev /dev/mapper/sys-rootlv >> BTRFS error (device dm-0): bdev /dev/mapper/sys-rootlv errs: wr 0, rd 0, flush 0, corrupt 2824, gen 0 >> BTRFS error (device dm-0): unable to fixup (regular) error at logical 1579016192 on dev /dev/mapper/sys-rootlv >> >> There are two problems: >> - No proper details about the corruption >> No metadata or data indication, nor the filename or the tree id. >> Even the involved kernel (and newer kernels after the scrub rework) >> has the ability to do backref walk and find out the file path or the >> tree backref. >> >> My guess is, for data sometimes the corrupted sector is no longer >> referred by any data extent. >> >> - Too noisy and useless error message from >> btrfs_dev_stat_inc_and_print() >> I'd argue we should not output any error message just for >> btrfs_dev_stat_inc_and_print(). >> >> After the series, the error message would look like this: >> >> BTRFS warning (device dm-2): checksum error at inode 5/257(file1) fileoff 16384, logical 13647872(1) physical 1(/dev/mapper/test-scratch1)13647872 > > Stoked on this series, thanks for doing it! > > qq: would it be helpful to also include the actual/expected csum? I > think it's particularly helpful when one or the other is either zeros or > the checksum of the zero block. It's a little too long to add (especially considering SHA256). And even with CRC32C, I have difficulty to expose any all zero/one checksum. (Maybe I did too few support recently?) Thanks, Qu > >> >> This involves the following enhancement: >> >> - Single line >> And we ensure we output at least one line for the error we hit. >> No more unrelated btrfs_dev_stat_inc_and_print() output. >> >> - Proper fall backs >> We have the following different fall back options >> * Repaired >> Just a line of something repaired for logical/physical address. >> >> * Detailed data info >> Including the following elements (if possible), and if higher >> priority ones can not be fetched, it would be skipped and try >> lower priority items: >> + file path (can have multiple ones) >> + root/inode number and offset >> + logical/physical address (always output) >> >> * Detaile metadata info >> The priority list is: >> + root ref/level >> + logical/physical address (always output) >> >> For the worst case of data corruption, we would still have some like: >> >> BTRFS warning (device dm-2): checksum error at data, logical 13647872(1) physical 1(/dev/mapper/test-scratch1)13647872 >> >> And similar ones for metadata: >> >> BTRFS warning (device dm-2): checksum error at meta, logical 13647872(1) physical 1(/dev/mapper/test-scratch1)13647872 >> >> The first patch is fixing a regression in the error message, which leads >> to bad logical/physical bytenr. >> The second one would reduce the log level for >> btrfs_dev_stat_inc_and_print(). >> >> Path 3~4 are cleanups to remove unnecessary parameters. >> >> The remaining reworks the format and refine the error message frequency. >> >> Qu Wenruo (7): >> btrfs: scrub: fix incorrectly reported logical/physical address >> btrfs: reduce the log level for btrfs_dev_stat_inc_and_print() >> btrfs: scrub: remove unused is_super parameter from >> scrub_print_common_warning() >> btrfs: scrub: remove unnecessary dev/physical lookup for >> scrub_stripe_report_errors() >> btrfs: scrub: simplify the inode iteration output >> btrfs: scrub: unify and shorten the error message >> btrfs: scrub: fix the frequency of error messages >> >> fs/btrfs/scrub.c | 185 ++++++++++++++++----------------------------- >> fs/btrfs/volumes.c | 2 +- >> 2 files changed, 66 insertions(+), 121 deletions(-) >> >> -- >> 2.44.0 >> >