Re: [PATCH 1/2] btrfs-progs: avoid checking wrong RAID5/6 P/Q data

[David Sterba <dsterba@suse.cz>]

On Tue, Mar 29, 2022 at 05:44:25PM +0800, Qu Wenruo wrote:
> [BUG]
> "btrfs check --check-data-csum" is causing tons of false alerts for
> RAID56 systems:
> 
>   # mkfs.btrfs -f $dev1 $dev2 $dev3 -m raid1 -d raid5
>   # mount $dev1 $mnt
>   # xfs_io -f -c "pwrite 0 16k" $mnt/file
>   # umount $mnt
>   # btrfs check --check-data-csum $dev1
>   ...
>   [5/7] checking csums against data
>   mirror 2 bytenr 389152768 csum 0x02ca4e98 expected csum 0x98757625
>   mirror 2 bytenr 389156864 csum 0x02ca4e98 expected csum 0x98757625
>   mirror 2 bytenr 389160960 csum 0x02ca4e98 expected csum 0x98757625
>   mirror 2 bytenr 389165056 csum 0x02ca4e98 expected csum 0x98757625
>   ERROR: errors found in csum tree
>   [6/7] checking root refs
> 
> But scrub is completely fine, and manually inspecting the on-disk data
> shows nothing wrong.
> 
> [CAUSE]
> Btrfs-progs only implemented the RAID56 write support, mostly for
> metadata.
> 
> It doesn't have RAID56 repair ability at all.
> (Btrfs-fuse has the ability ready to be contribued to progs though).
> 
> In read_extent_data(), it always read data from the first stripe,
> without any RAID56 rebuild.
> 
> [WORKAROUND]
> It will take a while to port RAID56 repair into btrfs-progs.
> Just reduce the btrfs_num_copies() report for RAID56 to 1, so that we
> won't even try to rebuild using P/Q.
> 
> Also add a warning message for open_ctree() of btrfs-progs, so
> explicitly show the lack of RAID56 rebuild ability.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  kernel-shared/disk-io.c |  7 +++++++
>  kernel-shared/volumes.c | 10 ++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel-shared/disk-io.c b/kernel-shared/disk-io.c
> index 4964cd3827e4..426fe40b53d6 100644
> --- a/kernel-shared/disk-io.c
> +++ b/kernel-shared/disk-io.c
> @@ -997,6 +997,13 @@ int btrfs_check_fs_compatibility(struct btrfs_super_block *sb,
>  		btrfs_set_super_incompat_flags(sb, features);
>  	}
>  
> +	/*
> +	 * We don't have the ability to repair from P/Q yet, give some warning
> +	 * about this
> +	 */
> +	if (features & BTRFS_FEATURE_INCOMPAT_RAID56)
> +		printf("WARNING: repairing using RAID56 P/Q is not supported yet\n");

There's a helper for warnings, warning()

> +
>  	features = btrfs_super_compat_ro_flags(sb);
>  	if (flags & OPEN_CTREE_WRITES) {
>  		if (flags & OPEN_CTREE_INVALIDATE_FST) {
> diff --git a/kernel-shared/volumes.c b/kernel-shared/volumes.c
> index e24428db8412..5845a4383d5f 100644
> --- a/kernel-shared/volumes.c
> +++ b/kernel-shared/volumes.c
> @@ -1645,10 +1645,12 @@ int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
>  		ret = map->num_stripes;
>  	else if (map->type & BTRFS_BLOCK_GROUP_RAID10)
>  		ret = map->sub_stripes;
> -	else if (map->type & BTRFS_BLOCK_GROUP_RAID5)
> -		ret = 2;
> -	else if (map->type & BTRFS_BLOCK_GROUP_RAID6)
> -		ret = 3;
> +	else if (map->type & BTRFS_BLOCK_GROUP_RAID56_MASK)
> +		/*
> +		 * In btrfs-progs we don't yet have the ability to rebuild
> +		 * from P/Q, thus currently it can only provide one mirror.
> +		 */
> +		ret = 1;
>  	else
>  		ret = 1;
>  	return ret;
> -- 
> 2.35.1