diff mbox series

[DEBUG] md/raid1: check recovery_offset in raid1_check_read_range

Message ID 20240728103634.208234-1-mat.jonczyk@o2.pl (mailing list archive)
State RFC
Headers show
Series [DEBUG] md/raid1: check recovery_offset in raid1_check_read_range | expand

Checks

Context Check Description
mdraidci/vmtest-md-6_11-VM_Test-0 success Logs for build-kernel
mdraidci/vmtest-md-6_11-PR success PR summary

Commit Message

Mateusz Jończyk July 28, 2024, 10:36 a.m. UTC
This should fix the filesystem corruption during RAID resync.

Checking this condition in raid1_check_read_range is not ideal, but this
is only a debug patch.

Link: https://lore.kernel.org/lkml/20240724141906.10b4fc4e@peluse-desk5/T/#m671d6d3a7eda44d39d0882864a98824f52c52917
Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
Cc: Yu Kuai <yukuai3@huawei.com>
Cc: Song Liu <song@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/md/raid1-10.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Yu Kuai July 29, 2024, 1:30 a.m. UTC | #1
Hi,

在 2024/07/28 18:36, Mateusz Jończyk 写道:
> This should fix the filesystem corruption during RAID resync.
> 
> Checking this condition in raid1_check_read_range is not ideal, but this
> is only a debug patch.
> 
> Link: https://lore.kernel.org/lkml/20240724141906.10b4fc4e@peluse-desk5/T/#m671d6d3a7eda44d39d0882864a98824f52c52917
> Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl>
> Cc: Yu Kuai <yukuai3@huawei.com>
> Cc: Song Liu <song@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>   drivers/md/raid1-10.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
> index 2ea1710a3b70..4ab896e8cb12 100644
> --- a/drivers/md/raid1-10.c
> +++ b/drivers/md/raid1-10.c
> @@ -252,6 +252,10 @@ static inline int raid1_check_read_range(struct md_rdev *rdev,
>   	sector_t first_bad;
>   	int bad_sectors;
>   
> +	if (!test_bit(In_sync, &rdev->flags) &&
> +	    rdev->recovery_offset < this_sector + *len)
> +		return 0;
> +

It's right this check is necessary, which means should not read from
rdev that the array is still in recovery and the read range is not
recoveried yet.

However, choose_first_rdev() is called during array resync, hence
the array will not be in recovery, and this is just dead code and not
needed.

And choose_bb_rdev() will only be called when the read range has bb,
which means the read range msut be accessed before and IO error must
occur first, and which indicates this rdev can't still be in recovery
for the read range.

So I think this check is only needed for slow disks.

BTW, looks like we don't have much tests for the case slow disks in
raid1.

Thanks,
Kuai

>   	/* no bad block overlap */
>   	if (!is_badblock(rdev, this_sector, *len, &first_bad, &bad_sectors))
>   		return *len;
>
diff mbox series

Patch

diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index 2ea1710a3b70..4ab896e8cb12 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -252,6 +252,10 @@  static inline int raid1_check_read_range(struct md_rdev *rdev,
 	sector_t first_bad;
 	int bad_sectors;
 
+	if (!test_bit(In_sync, &rdev->flags) &&
+	    rdev->recovery_offset < this_sector + *len)
+		return 0;
+
 	/* no bad block overlap */
 	if (!is_badblock(rdev, this_sector, *len, &first_bad, &bad_sectors))
 		return *len;