Re: [RFC PATCH 1/5] md/raid1: balance reads across non-rotational disks

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The use of a global atomic counter (`atomic_inc_return(&conf->read_rr)`) in the read fast-path introduces severe cacheline contention on high-concurrency NVMe arrays, heavily throttling peak read IOPS scalability.
--

--- Patch [1]: [RFC PATCH 1/5] md/raid1: balance reads across non-rotational disks ---
commit fe7aa5a93e1157781321a0273156730cd901aaec
Author: Chen Cheng <[email protected]>
md/raid1: balance reads across non-rotational disks

This commit optimizes read balancing for non-rotational disks in RAID1 by
distributing reads more evenly when pending IOs are equal. It introduces a
round-robin start slot rotation across disks to prevent low-depth random
reads from always sticking to the first slot.

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index f0646fb24371f..319b24bcab5b7 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -799,6 +806,10 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
>  		.sequential_disk	= -1,
>  	};
>  
> +	if (has_nonrot)
> +		rr_start = (unsigned int)atomic_inc_return(&conf->read_rr) %
> +			   conf->raid_disks;

[Severity: Medium]
Will updating a global atomic counter on every read operation introduce
severe cacheline bouncing across CPU cores?

For high-concurrency NVMe arrays processing millions of IOPS, could this
global atomic increment in the read fast-path become a major scalability
bottleneck due to exclusive cacheline ownership?

> +
>  	for (disk = 0 ; disk < conf->raid_disks * 2 ; disk++) {
>  		struct md_rdev *rdev;
>  		sector_t dist;

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