linux-kernel - Re: [PATCH] md/raid1: skip recovery of already synced areas

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <eaae71c1-8723-8f49-5bd8-01a1e67152be@huaweicloud.com>
Date: Thu, 11 Sep 2025 14:10:18 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: linan666@...weicloud.com, song@...nel.org
Cc: linux-raid@...r.kernel.org, linux-kernel@...r.kernel.org,
 yangerkun@...wei.com, yi.zhang@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH] md/raid1: skip recovery of already synced areas

Hi,

在 2025/09/10 16:25, linan666@...weicloud.com 写道:
> From: Li Nan <linan122@...wei.com>
> 
> When a new disk is added during running recovery, the kernel may
> restart recovery from the beginning of the device and submit write
> io to ranges that have already been synchronized.
> 
> Reproduce:
>    mdadm -CR /dev/md0 -l1 -n3 /dev/sda missing missing
>    mdadm --add /dev/md0 /dev/sdb
>    sleep 10
>    cat /proc/mdstat	# partially synchronized
>    mdadm --add /dev/md0 /dev/sdc
>    cat /proc/mdstat	# start from 0
>    iostat 1 sdb sdc	# sdb has io, too
> 
> If 'rdev->recovery_offset' is ahead of the current recovery sector,
> read from that device instead of issuing a write. It prevents
> unnecessary writes while still preserving the chance to back up data
> if it is the last copy.
> 
> Signed-off-by: Li Nan <linan122@...wei.com>
> ---
>   drivers/md/raid1.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 3e422854cafb..ac5a9b73157a 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2894,7 +2894,8 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
>   		    test_bit(Faulty, &rdev->flags)) {
>   			if (i < conf->raid_disks)
>   				still_degraded = true;
> -		} else if (!test_bit(In_sync, &rdev->flags)) {
> +		} else if (!test_bit(In_sync, &rdev->flags) &&
> +			   rdev->recovery_offset <= sector_nr) {
>   			bio->bi_opf = REQ_OP_WRITE;
>   			bio->bi_end_io = end_sync_write;
>   			write_targets ++;
> @@ -2903,6 +2904,9 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
>   			sector_t first_bad = MaxSector;
>   			sector_t bad_sectors;
>   
> +			if (!test_bit(In_sync, &rdev->flags))
> +				good_sectors = min(rdev->recovery_offset - sector_nr,
> +						   (u64)good_sectors);
>   			if (is_badblock(rdev, sector_nr, good_sectors,
>   					&first_bad, &bad_sectors)) {
>   				if (first_bad > sector_nr)
> 

This patch looks correct, however, I took a long time to go through all
the details, and there is still the same problem in the case new disk is
added during resync.

Perhaps this is a good time to cleanup raid1_sync_request() now, just
don't mess resync and recover together in the same code path with lots
of special handling.

Thanks,
Kuai