Re: [PATCH 0/6] btrfs: scrub: refine the error messages

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 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
>>
>