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

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?

> +		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);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1