Re: [PATCH 1/2] md/raid5: submit a window of stripes during resync/recovery

Hi,

在 2026/7/10 21:36, [email protected] 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The batched sync loop can greedily drain the stripe cache without yielding, causing severe latency spikes and potentially starving user I/O.
> --
>
> --- Patch [1]: [PATCH 1/2] md/raid5: submit a window of stripes during resync/recovery ---
> commit 2b3cdb45d82bdea6112cba5dad14b59ec3acb23a
> Author: Hiroshi Nishida <[email protected]>
>
> md/raid5: submit a window of stripes during resync/recovery
>
> This patch updates the RAID5 sync path to dispatch a batch of up to 32
> stripes per call instead of just one. It throttles only on the first stripe,
> allowing subsequent stripes to be submitted without blocking to improve
> resync performance.
>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 0c5c9fb0606ee..574880e4f23fe 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
> [ ... ]
>> @@ -6640,13 +6632,42 @@ static inline sector_t raid5_sync_request(struct mddev *mddev, sector_t sector_n
> [ ... ]
>> +	/* Submit remaining stripes in the window non-blocking.  Stop early
>> +	 * if the stripe cache is full: the disk queue is already saturated.
>> +	 * Bound by resync_max so a user- or cluster-imposed sync ceiling is
>> +	 * not overshot.
>> +	 */
>> +	win_sector = sector_nr + RAID5_STRIPE_SECTORS(conf);
>> +	for (submitted = 1;
>> +	     submitted < RAID5_SYNC_WINDOW && win_sector < max_sector &&
>> +	     win_sector < mddev->resync_max;
>> +	     submitted++, win_sector += RAID5_STRIPE_SECTORS(conf)) {
>> +		sh = raid5_get_active_stripe(conf, NULL, win_sector,
>> +					     R5_GAS_NOBLOCK);
>> +		if (!sh)
>> +			break;
> [Severity: High]
> Can this batched sync loop greedily drain the stripe cache without yielding to
> user I/O?
>
> During array resync or recovery, this loop acquires up to 32 free stripes
> sequentially. Could this starve user I/O threads that are blocked on the
> conf->wait_for_stripe waitqueue, since it doesn't check if user I/O is waiting
> (e.g., via waitqueue_active(&conf->wait_for_stripe)) before getting the next
> stripe?

This make sense, always submit 32 stripes at a time will against the throttling rules
from sync_io_depth. Please check this in sync_io_within_limit(), and do not exceed
the limit.

>
>> +		md_bitmap_start_sync(mddev, win_sector, &sync_blocks,
>> +				     still_degraded);
>> +		set_bit(STRIPE_SYNC_REQUESTED, &sh->state);
>> +		set_bit(STRIPE_HANDLE, &sh->state);
>> +		raid5_release_stripe(sh);
>> +	}
>> +
>> +	return submitted * RAID5_STRIPE_SECTORS(conf);
>>   }

-- 
Thanks,
Kuai