Re: [PATCH] btrfs: scrub: report the failing sector's address, not the stripe base

在 2026/8/15 01:54, James C. Owens 写道:
> scrub_stripe_report_errors() iterates over the sectors of a stripe, but
> every message it emits passes stripe->logical, the address of the first
> sector of the 64KiB stripe, rather than the address of the sector being
> reported. The physical address is likewise computed once, before the
> loop, from stripe->logical.
> 
> This matters because scrub_print_common_warning() uses that logical
> address for the backref walk which produces the "root %llu inode %llu
> offset %llu ... (path: ...)" part of the message. As the address is
> always the stripe base, the reported root/inode/offset/path can identify
> a different file from the one whose sector actually failed.
> 
> A 64KiB stripe routinely spans several extents belonging to unrelated
> files. On the machine where this was found, the stripe at logical
> 0x17D9380000 holds four sectors of /usr/share/plasma/emoji/bg.dict, then
> a file inside a docker volume, then sectors referenced only by
> snapshots. Every error anywhere in that stripe is attributed to bg.dict.
> 
> The effect is visible statistically: across ten months and four kernel
> series that machine logged 81 distinct flagged logical addresses, and
> every one of them is exactly 64KiB aligned. Since BTRFS_STRIPE_LEN is
> 64KiB and stripe->logical is stripe aligned by construction, real
> failures distributed across sectors could not produce that.
> 
> Report the address of the sector actually being examined. Adding the
> sector offset to the physical address is valid because BTRFS_STRIPE_LEN
> is the unit contiguous on a single device for every profile, so a stripe
> never crosses a device boundary.
> 
> Signed-off-by: James C. Owens <[email protected]>

Fixes: 0096580713ff ("btrfs: scrub: introduce error reporting 
functionality for scrub_stripe")

> ---
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -1045,6 +1045,10 @@

The patch format doesn't looks correct.
How did you generate the patch?

Normally the lines should look like:

---
  fs/btrfs/dir-item.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index 30ddafaf8d3d..a6bd69be522b 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -149,8 +149,6 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle 
*trans,

Which should include a summary of modified files, the hash, a simple 
context.

Otherwise the patch looks good to me, and still applies.

Reviewed-by: Qu Wenruo <[email protected]>

Thanks,
Qu

>   
>   skip:
>   	for_each_set_bit(sector_nr, &extent_bitmap, stripe->nr_sectors) {
> +		const u64 sector_logical = stripe->logical +
> +			((u64)sector_nr << fs_info->sectorsize_bits);
> +		const u64 sector_physical = physical +
> +			((u64)sector_nr << fs_info->sectorsize_bits);
>   		bool repaired = false;
>   
>   		if (scrub_bitmap_test_bit_is_metadata(stripe, sector_nr)) {
> @@ -1073,12 +1077,12 @@
>   			if (dev) {
>   				btrfs_err_rl(fs_info,
>   		"scrub: fixed up error at logical %llu on dev %s physical %llu",
> -					    stripe->logical, btrfs_dev_name(dev),
> -					    physical);
> +					    sector_logical, btrfs_dev_name(dev),
> +					    sector_physical);
>   			} else {
>   				btrfs_err_rl(fs_info,
>   			   "scrub: fixed up error at logical %llu on mirror %u",
> -					    stripe->logical, stripe->mirror_num);
> +					    sector_logical, stripe->mirror_num);
>   			}
>   			continue;
>   		}
> @@ -1087,30 +1091,30 @@
>   		if (dev) {
>   			btrfs_err_rl(fs_info,
>   "scrub: unable to fixup (regular) error at logical %llu on dev %s physical %llu",
> -					    stripe->logical, btrfs_dev_name(dev),
> -					    physical);
> +					    sector_logical, btrfs_dev_name(dev),
> +					    sector_physical);
>   		} else {
>   			btrfs_err_rl(fs_info,
>   	  "scrub: unable to fixup (regular) error at logical %llu on mirror %u",
> -					    stripe->logical, stripe->mirror_num);
> +					    sector_logical, stripe->mirror_num);
>   		}
>   
>   		if (scrub_bitmap_test_bit_io_error(stripe, sector_nr))
>   			if (__ratelimit(&rs) && dev)
>   				scrub_print_common_warning("i/o error", dev, false,
> -						     stripe->logical, physical);
> +						     sector_logical, sector_physical);
>   		if (scrub_bitmap_test_bit_csum_error(stripe, sector_nr))
>   			if (__ratelimit(&rs) && dev)
>   				scrub_print_common_warning("checksum error", dev, false,
> -						     stripe->logical, physical);
> +						     sector_logical, sector_physical);
>   		if (scrub_bitmap_test_bit_meta_error(stripe, sector_nr))
>   			if (__ratelimit(&rs) && dev)
>   				scrub_print_common_warning("header error", dev, false,
> -						     stripe->logical, physical);
> +						     sector_logical, sector_physical);
>   		if (scrub_bitmap_test_bit_meta_gen_error(stripe, sector_nr))
>   			if (__ratelimit(&rs) && dev)
>   				scrub_print_common_warning("generation error", dev, false,
> -						     stripe->logical, physical);
> +						     sector_logical, sector_physical);
>   	}
>   
>   	/* Update the device stats. */