[RFC PATCH 0/5] md/raid1: improve choose_best_rdev read balance

[RFC PATCH 0/5] md/raid1: improve choose_best_rdev read balance

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

Prefer a nonrot disk when one can take the read. When pending is
the same, use a real round-robin among the disks we may pick.

fio libaio direct=1, NVMe none, SATA mq-deadline (stock -> this):

- 2x GM9000: 1M QD16 j1 7.031 -> 13.886 GB/s, 50/50; 4k QD1 50/50
- 2x/3x HDWG740: 0.294 -> 0.514 / 0.630 GB/s
- GM9000+HDD seq: 2.468 or 0.280 -> 7.112 GB/s, 100/0 NVMe
- Fanxiang+HDD seq, HDD first: 0.278 -> 0.562 GB/s
- GM9000+HDD 4k QD8/16: clat -47% / -45%


Chen Cheng (5):
  md/raid1: balance reads across non-rotational disks
  md/raid1: do not move nonrot reads onto a rot disk
  md/raid1: do not send random reads to a rot disk
  md/raid1: use rot policy when no nonrot disk is readable
  md/raid1: clarify choose_best_rdev comments

 drivers/md/raid1.c | 122 ++++++++++++++++++++++++++++++++++++++-------
 drivers/md/raid1.h |   1 +
 2 files changed, 104 insertions(+), 19 deletions(-)

-- 
2.55.0

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

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

choose_best_rdev() picks one disk for each read.
min_pending is UINT_MAX; store it as unsigned int, like raid10.

Current:
1. Sequential read: stay on the current disk.
2. Switch only if should_choose_next() is true.
3. should_choose_next() needs bdev_io_opt() > 0.
4. Non-sequential read: pick the disk with the lowest nr_pending.
5. The compare uses strict '>'. Same pending keeps the first disk.

Problem:
1. Many client NVMe set optimal_io_size to 0. Then
   should_choose_next() never runs. One sequential stream stays on
   one disk. Why not use the idle disk?
2. Same for rot-only RAID1. One rot disk takes the whole stream.
   The other rot disk is idle. Why not use it?
3. Low-depth random reads often have the same pending. Why always
   stay on slot 0?

Improve:
1. If a sequential disk already has pending I/O, do not return it
   at once. Let pending pick an idle disk.
2. On nonrot arrays, if pending is the same, rotate a start slot
   (0..raid_disks-1).
3. Only bump read_rr when the array has a nonrot member.

Tested with fio libaio direct=1 (NVMe scheduler none, SATA
scheduler mq-deadline):

- 2x Predator GM9000 (optimal_io_size=0):
  a) 4k randread QD1 jobs=1: 0.084 GB/s, 100/0 -> 0.084 GB/s, 50/50
  b) 1M read QD16 jobs=1: 7.031 -> 13.886 GB/s (+97%), 66/34 -> 50/50
  c) 1M read QD16 jobs=2: 14.22 GB/s, 50/50 both sides

- 4x Intel MEMPEK1J016GA (Optane pmem, optimal_io_size=0):
  1M read QD16 jobs=1: ~0.85 GB/s on one member -> ~3.0+ GB/s,
  ~25% per disk

- RAID1 of two then three TOSHIBA HDWG740:
  2 disks, 1M read QD16 jobs=1: 0.294 GB/s, 100/0 -> 0.514 GB/s, 50/50
  3 disks, 1M read QD16 jobs=1: 0.294 GB/s, 100/0 -> 0.630 GB/s, 33/33/33
  2 and 3 disks, 4k randread QD1 jobs=1: 0.001 GB/s, all on
  one disk (rot-only random does not use read_rr)

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid1.c | 25 +++++++++++++++++++++----
 drivers/md/raid1.h |  1 +
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index f0646fb24371..319b24bcab5b 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -780,27 +780,38 @@ static bool rdev_readable(struct md_rdev *rdev, struct r1bio *r1_bio)
 }
 
 struct read_balance_ctl {
 	sector_t closest_dist;
 	int closest_dist_disk;
-	int min_pending;
+	unsigned int min_pending;
 	int min_pending_disk;
 	int sequential_disk;
 	int readable_disks;
 };
 
+static int raid1_rr_pos(int disk, int start, int n)
+{
+	return ((disk % n) - start + n) % n;
+}
+
 static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 {
 	int disk;
+	int rr_start = 0;
+	bool has_nonrot = READ_ONCE(conf->nonrot_disks);
 	struct read_balance_ctl ctl = {
 		.closest_dist_disk      = -1,
 		.closest_dist           = MaxSector,
 		.min_pending_disk       = -1,
 		.min_pending            = UINT_MAX,
 		.sequential_disk	= -1,
 	};
 
+	if (has_nonrot)
+		rr_start = (unsigned int)atomic_inc_return(&conf->read_rr) %
+			   conf->raid_disks;
+
 	for (disk = 0 ; disk < conf->raid_disks * 2 ; disk++) {
 		struct md_rdev *rdev;
 		sector_t dist;
 		unsigned int pending;
 
@@ -819,11 +830,11 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 		dist = abs(r1_bio->sector -
 			   READ_ONCE(conf->mirrors[disk].head_position));
 
 		/* Don't change to another disk for sequential reads */
 		if (is_sequential(conf, disk, r1_bio)) {
-			if (!should_choose_next(conf, disk))
+			if (!should_choose_next(conf, disk) && !pending)
 				return disk;
 
 			/*
 			 * Add 'pending' to avoid choosing this disk if
 			 * there is other idle disk.
@@ -834,11 +845,16 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 			 * will be chosen.
 			 */
 			ctl.sequential_disk = disk;
 		}
 
-		if (ctl.min_pending > pending) {
+		if (ctl.min_pending > pending ||
+		    (has_nonrot && ctl.min_pending == pending &&
+		     ctl.min_pending_disk >= 0 &&
+		     raid1_rr_pos(disk, rr_start, conf->raid_disks) <
+		     raid1_rr_pos(ctl.min_pending_disk, rr_start,
+				  conf->raid_disks))) {
 			ctl.min_pending = pending;
 			ctl.min_pending_disk = disk;
 		}
 
 		if (ctl.closest_dist > dist) {
@@ -859,11 +875,11 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 	 * non-rotational, choose the disk with less pending request even the
 	 * disk is rotational, which might/might not be optimal for raids with
 	 * mixed ratation/non-rotational disks depending on workload.
 	 */
 	if (ctl.min_pending_disk != -1 &&
-	    (READ_ONCE(conf->nonrot_disks) || ctl.min_pending == 0))
+	    (has_nonrot || ctl.min_pending == 0))
 		return ctl.min_pending_disk;
 	else
 		return ctl.closest_dist_disk;
 }
 
@@ -3091,10 +3107,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 		goto abort;
 
 	err = -EINVAL;
 	spin_lock_init(&conf->device_lock);
 	conf->raid_disks = mddev->raid_disks;
+	atomic_set(&conf->read_rr, -1);
 	rdev_for_each(rdev, mddev) {
 		int disk_idx = rdev->raid_disk;
 
 		if (disk_idx >= conf->raid_disks || disk_idx < 0)
 			continue;
diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
index c98d43a7ae99..d5de976d171d 100644
--- a/drivers/md/raid1.h
+++ b/drivers/md/raid1.h
@@ -54,10 +54,11 @@ struct r1conf {
 	struct raid1_info	*mirrors;	/* twice 'raid_disks' to
 						 * allow for replacements.
 						 */
 	int			raid_disks;
 	int			nonrot_disks;
+	atomic_t		read_rr;
 
 	spinlock_t		device_lock;
 
 	/* list of 'struct r1bio' that need to be processed by raid1d,
 	 * whether to retry a read, writeout a resync or recovery
-- 
2.55.0

[RFC PATCH 2/5] md/raid1: do not move nonrot reads onto a rot disk

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

The previous change: if a sequential disk already has pending I/O,
the next read can go to an idle disk.

Current:
1. A sequential disk can give the next read to an idle peer.
2. On a mixed array that peer can be a rot disk.
3. After a write, every disk has the same head_position.
4. Then every disk looks sequential.

Problem:
1. The first sequential disk in slot order may be a rot disk. Then
   we return it and never see the nonrot disk. We want the nonrot
   disk.
2. When pending is the same, rot and nonrot share one round-robin.
   A rot disk can win. We want the nonrot disk.

Improve:
1. If a nonrot disk is readable, do not stop on a sequential rot
   disk.
2. Remember sequential_disk once. A nonrot disk may replace a rot
   one.
3. If a nonrot disk is readable, do not keep a rot disk as the
   sequential fallback.
4. When pending is the same, prefer nonrot. Rotate only among
   nonrot disks.
5. Rot-only sequential reads still stay on the current disk when
   no peer is idle.

Tested with fio libaio direct=1 (NVMe scheduler none, SATA
scheduler mq-deadline), after a short write so both
head_positions match:

- RAID1 of Predator GM9000 + SATA HDD:
  1M read QD16 jobs=1, nonrot first:
    2.468 GB/s, 92/8 NVMe/HDD -> 7.112 GB/s, 100/0 NVMe
  1M read QD16 jobs=1, rot first:
    0.280 GB/s, 100/0 HDD -> 7.112 GB/s, 100/0 NVMe

- RAID1 of Fanxiang S103Pro + SATA HDD:
  1M read QD16 jobs=1, nonrot first:
    0.562 GB/s, 100/0 Fanxiang both sides
  1M read QD16 jobs=1, rot first:
    0.278 GB/s, 100/0 HDD -> 0.562 GB/s, 100/0 Fanxiang

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid1.c | 42 +++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 319b24bcab5b..36520e48826f 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -784,17 +784,36 @@ struct read_balance_ctl {
 	int closest_dist_disk;
 	unsigned int min_pending;
 	int min_pending_disk;
 	int sequential_disk;
 	int readable_disks;
+	bool min_pending_nonrot;
+	bool sequential_nonrot;
 };
 
 static int raid1_rr_pos(int disk, int start, int n)
 {
 	return ((disk % n) - start + n) % n;
 }
 
+static bool is_better_disk(unsigned int pending, int disk, bool nonrot,
+			   const struct read_balance_ctl *ctl,
+			   int rr_start, int n)
+{
+	if (ctl->min_pending_disk < 0)
+		return true;
+	if (ctl->min_pending < pending)
+		return false;
+	if (ctl->min_pending > pending)
+		return true;
+	if (nonrot && !ctl->min_pending_nonrot)
+		return true;
+	return nonrot && ctl->min_pending_nonrot &&
+	       raid1_rr_pos(disk, rr_start, n) <
+	       raid1_rr_pos(ctl->min_pending_disk, rr_start, n);
+}
+
 static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 {
 	int disk;
 	int rr_start = 0;
 	bool has_nonrot = READ_ONCE(conf->nonrot_disks);
@@ -812,10 +831,11 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 
 	for (disk = 0 ; disk < conf->raid_disks * 2 ; disk++) {
 		struct md_rdev *rdev;
 		sector_t dist;
 		unsigned int pending;
+		bool nonrot;
 
 		if (r1_bio->bios[disk] == IO_BLOCKED)
 			continue;
 
 		rdev = conf->mirrors[disk].rdev;
@@ -827,14 +847,16 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 			set_bit(R1BIO_FailFast, &r1_bio->state);
 
 		pending = atomic_read(&rdev->nr_pending);
 		dist = abs(r1_bio->sector -
 			   READ_ONCE(conf->mirrors[disk].head_position));
+		nonrot = test_bit(Nonrot, &rdev->flags);
 
 		/* Don't change to another disk for sequential reads */
 		if (is_sequential(conf, disk, r1_bio)) {
-			if (!should_choose_next(conf, disk) && !pending)
+			if (!should_choose_next(conf, disk) && !pending &&
+			    (nonrot || !has_nonrot))
 				return disk;
 
 			/*
 			 * Add 'pending' to avoid choosing this disk if
 			 * there is other idle disk.
@@ -842,21 +864,22 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 			pending++;
 			/*
 			 * If there is no other idle disk, this disk
 			 * will be chosen.
 			 */
-			ctl.sequential_disk = disk;
+			if (ctl.sequential_disk < 0 ||
+			    (nonrot && !ctl.sequential_nonrot)) {
+				ctl.sequential_disk = disk;
+				ctl.sequential_nonrot = nonrot;
+			}
 		}
 
-		if (ctl.min_pending > pending ||
-		    (has_nonrot && ctl.min_pending == pending &&
-		     ctl.min_pending_disk >= 0 &&
-		     raid1_rr_pos(disk, rr_start, conf->raid_disks) <
-		     raid1_rr_pos(ctl.min_pending_disk, rr_start,
-				  conf->raid_disks))) {
+		if (is_better_disk(pending, disk, nonrot, &ctl,
+				   rr_start, conf->raid_disks)) {
 			ctl.min_pending = pending;
 			ctl.min_pending_disk = disk;
+			ctl.min_pending_nonrot = nonrot;
 		}
 
 		if (ctl.closest_dist > dist) {
 			ctl.closest_dist = dist;
 			ctl.closest_dist_disk = disk;
@@ -865,11 +888,12 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 
 	/*
 	 * sequential IO size exceeds optimal iosize, however, there is no other
 	 * idle disk, so choose the sequential disk.
 	 */
-	if (ctl.sequential_disk != -1 && ctl.min_pending != 0)
+	if (ctl.sequential_disk != -1 && ctl.min_pending != 0 &&
+	    (ctl.sequential_nonrot || !has_nonrot))
 		return ctl.sequential_disk;
 
 	/*
 	 * If all disks are rotational, choose the closest disk. If any disk is
 	 * non-rotational, choose the disk with less pending request even the
-- 
2.55.0

[RFC PATCH 3/5] md/raid1: do not send random reads to a rot disk

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

The previous change keeps sequential nonrot I/O off a rot disk.

Current:
1. Sequential reads stay on nonrot when a nonrot disk is readable.
2. Random reads still pick the disk with the lowest nr_pending.
3. Rot disks also join that compare.

Problem:
1. A nonrot disk is fast. It can have more pending I/O.
2. A rot disk is slow. It can have fewer pending I/O.
3. Then the next random read goes to the rot disk.
4. Why send a random 4k read to the slow disk?

Improve:
1. If a nonrot disk is readable, do not use rot disks for
   min_pending.
2. Random reads stay on nonrot disks.
3. If no nonrot disk is readable, still pick a rot disk by head
   position.

Tested with fio libaio direct=1 (NVMe scheduler none, SATA
scheduler mq-deadline):

- RAID1 of Predator GM9000 + SATA HDD:
  4k randread QD1 jobs=1: 0.097 GB/s, 100/0 NVMe -> 0.084 GB/s,
    100/0 NVMe. Both disks have pending 0, so stock already stayed
    on NVMe.
  4k randread QD8 jobs=1: 0.363 GB/s, 99.6/0.4 NVMe/HDD,
    clat 87 us -> 0.671 GB/s, 100/0 NVMe, clat 46 us (-47%)
  4k randread QD16 jobs=1: 0.668 GB/s, 99.7/0.3 NVMe/HDD,
    clat 95 us -> 1.215 GB/s, 100/0 NVMe, clat 52 us (-45%)

- RAID1 of Fanxiang S103Pro + SATA HDD:
  4k randread QD1 jobs=1: 0.077 GB/s, 100/0 Fanxiang -> 0.073 GB/s,
    100/0 Fanxiang
  4k randread QD8 jobs=1: 0.278 GB/s, 99.5/0.5 Fanxiang/HDD,
    clat 114 us -> 0.389 GB/s, 100/0 Fanxiang, clat 78 us (-31%)
  4k randread QD16 jobs=1: 0.395 GB/s, 99.6/0.4 Fanxiang/HDD,
    clat 162 us -> 0.398 GB/s, 100/0 Fanxiang, clat 159 us
    (already at the Fanxiang limit)

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid1.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 36520e48826f..523b55d42779 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -832,10 +832,11 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 	for (disk = 0 ; disk < conf->raid_disks * 2 ; disk++) {
 		struct md_rdev *rdev;
 		sector_t dist;
 		unsigned int pending;
 		bool nonrot;
+		bool can_pick;
 
 		if (r1_bio->bios[disk] == IO_BLOCKED)
 			continue;
 
 		rdev = conf->mirrors[disk].rdev;
@@ -848,15 +849,16 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 
 		pending = atomic_read(&rdev->nr_pending);
 		dist = abs(r1_bio->sector -
 			   READ_ONCE(conf->mirrors[disk].head_position));
 		nonrot = test_bit(Nonrot, &rdev->flags);
+		can_pick = nonrot || !has_nonrot;
 
 		/* Don't change to another disk for sequential reads */
 		if (is_sequential(conf, disk, r1_bio)) {
 			if (!should_choose_next(conf, disk) && !pending &&
-			    (nonrot || !has_nonrot))
+			    can_pick)
 				return disk;
 
 			/*
 			 * Add 'pending' to avoid choosing this disk if
 			 * there is other idle disk.
@@ -871,11 +873,12 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 				ctl.sequential_disk = disk;
 				ctl.sequential_nonrot = nonrot;
 			}
 		}
 
-		if (is_better_disk(pending, disk, nonrot, &ctl,
+		if (can_pick &&
+		    is_better_disk(pending, disk, nonrot, &ctl,
 				   rr_start, conf->raid_disks)) {
 			ctl.min_pending = pending;
 			ctl.min_pending_disk = disk;
 			ctl.min_pending_nonrot = nonrot;
 		}
-- 
2.55.0

[RFC PATCH 4/5] md/raid1: use rot policy when no nonrot disk is readable

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

has_nonrot selects mixed policy or rot-only policy.

Current:
1. has_nonrot is true if conf->nonrot_disks > 0.
2. nonrot_disks counts every nonrot disk.
3. A Faulty disk, a rebuild disk, and a WriteMostly disk still
   count.

Problem:
1. The array is NVMe + HDD. The NVMe fails. Only the HDD can
   take reads.
2. nonrot_disks is still 1. The code thinks this is a mixed
   array.
3. Sequential reads on the HDD do not stay on the HDD. Mixed
   policy will not keep a rot disk.
4. Every read still advances the nonrot round-robin, though no
   nonrot disk can take the read.

Improve:
1. Look at disks that can take this read.
2. If none of them is nonrot, use the rot-only policy.

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid1.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 523b55d42779..f476d4dea4be 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -793,10 +793,28 @@ struct read_balance_ctl {
 static int raid1_rr_pos(int disk, int start, int n)
 {
 	return ((disk % n) - start + n) % n;
 }
 
+static bool raid1_has_readable_nonrot(struct r1conf *conf,
+				      struct r1bio *r1_bio)
+{
+	int disk;
+
+	for (disk = 0; disk < conf->raid_disks * 2; disk++) {
+		struct md_rdev *rdev;
+
+		if (r1_bio->bios[disk] == IO_BLOCKED)
+			continue;
+		rdev = conf->mirrors[disk].rdev;
+		if (rdev_readable(rdev, r1_bio) &&
+		    test_bit(Nonrot, &rdev->flags))
+			return true;
+	}
+	return false;
+}
+
 static bool is_better_disk(unsigned int pending, int disk, bool nonrot,
 			   const struct read_balance_ctl *ctl,
 			   int rr_start, int n)
 {
 	if (ctl->min_pending_disk < 0)
@@ -814,11 +832,11 @@ static bool is_better_disk(unsigned int pending, int disk, bool nonrot,
 
 static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 {
 	int disk;
 	int rr_start = 0;
-	bool has_nonrot = READ_ONCE(conf->nonrot_disks);
+	bool has_nonrot = raid1_has_readable_nonrot(conf, r1_bio);
 	struct read_balance_ctl ctl = {
 		.closest_dist_disk      = -1,
 		.closest_dist           = MaxSector,
 		.min_pending_disk       = -1,
 		.min_pending            = UINT_MAX,
-- 
2.55.0

[RFC PATCH 5/5] md/raid1: clarify choose_best_rdev comments

[Chen Cheng]

From: Chen Cheng <chencheng@fnnas.com>

Write the rot/nonrot read policy in short comments next to the code.

No functional change.

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid1.c | 48 +++++++++++++++++++++++++++++++++-------------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index f476d4dea4be..897eef3a022d 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -788,15 +788,17 @@ struct read_balance_ctl {
 	int readable_disks;
 	bool min_pending_nonrot;
 	bool sequential_nonrot;
 };
 
+/* Offset from rr start. Replacement uses the same slot as primary. */
 static int raid1_rr_pos(int disk, int start, int n)
 {
 	return ((disk % n) - start + n) % n;
 }
 
+/* True if some readable member is nonrot. */
 static bool raid1_has_readable_nonrot(struct r1conf *conf,
 				      struct r1bio *r1_bio)
 {
 	int disk;
 
@@ -811,10 +813,11 @@ static bool raid1_has_readable_nonrot(struct r1conf *conf,
 			return true;
 	}
 	return false;
 }
 
+/* Lower pending wins. Same pending: prefer nonrot, then rr order. */
 static bool is_better_disk(unsigned int pending, int disk, bool nonrot,
 			   const struct read_balance_ctl *ctl,
 			   int rr_start, int n)
 {
 	if (ctl->min_pending_disk < 0)
@@ -828,10 +831,24 @@ static bool is_better_disk(unsigned int pending, int disk, bool nonrot,
 	return nonrot && ctl->min_pending_nonrot &&
 	       raid1_rr_pos(disk, rr_start, n) <
 	       raid1_rr_pos(ctl->min_pending_disk, rr_start, n);
 }
 
+/*
+ * Choose a readable disk for this read.
+ *
+ * Prefer nonrot. Use rot only if no nonrot disk is readable.
+ *
+ * Sequential idle: keep this disk. Mixed array: do not keep a
+ * rot disk (after a write every disk looks sequential).
+ * Sequential busy: try an idle disk of the same class. If none
+ * is idle, keep the sequential disk.
+ *
+ * Else fewest pending I/Os among disks we may pick. Same
+ * pending: nonrot, then round-robin. Rot-only with no idle
+ * disk: closest head.
+ */
 static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 {
 	int disk;
 	int rr_start = 0;
 	bool has_nonrot = raid1_has_readable_nonrot(conf, r1_bio);
@@ -867,26 +884,32 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 
 		pending = atomic_read(&rdev->nr_pending);
 		dist = abs(r1_bio->sector -
 			   READ_ONCE(conf->mirrors[disk].head_position));
 		nonrot = test_bit(Nonrot, &rdev->flags);
+		/*
+		 * If a nonrot disk is readable, pick only nonrot.
+		 * Else we must use rot.
+		 */
 		can_pick = nonrot || !has_nonrot;
 
-		/* Don't change to another disk for sequential reads */
+		/*
+		 * Idle sequential disk: return it now, unless
+		 * should_choose_next() wants another disk.
+		 * Mixed array: do not return a rot disk.
+		 */
 		if (is_sequential(conf, disk, r1_bio)) {
 			if (!should_choose_next(conf, disk) && !pending &&
 			    can_pick)
 				return disk;
 
-			/*
-			 * Add 'pending' to avoid choosing this disk if
-			 * there is other idle disk.
-			 */
+			/* Make an idle disk win over this busy one. */
 			pending++;
 			/*
-			 * If there is no other idle disk, this disk
-			 * will be chosen.
+			 * Remember the first sequential disk.
+			 * A nonrot disk may replace a rot disk.
+			 * A later nonrot disk may not replace an earlier one.
 			 */
 			if (ctl.sequential_disk < 0 ||
 			    (nonrot && !ctl.sequential_nonrot)) {
 				ctl.sequential_disk = disk;
 				ctl.sequential_nonrot = nonrot;
@@ -906,22 +929,21 @@ static int choose_best_rdev(struct r1conf *conf, struct r1bio *r1_bio)
 			ctl.closest_dist_disk = disk;
 		}
 	}
 
 	/*
-	 * sequential IO size exceeds optimal iosize, however, there is no other
-	 * idle disk, so choose the sequential disk.
+	 * Keep the sequential disk if no idle peer should take it.
+	 * If a nonrot disk is readable: keep only a nonrot sequential disk.
+	 * If not: an idle rot disk may take it.
 	 */
 	if (ctl.sequential_disk != -1 && ctl.min_pending != 0 &&
 	    (ctl.sequential_nonrot || !has_nonrot))
 		return ctl.sequential_disk;
 
 	/*
-	 * If all disks are rotational, choose the closest disk. If any disk is
-	 * non-rotational, choose the disk with less pending request even the
-	 * disk is rotational, which might/might not be optimal for raids with
-	 * mixed ratation/non-rotational disks depending on workload.
+	 * No readable nonrot disk: closest disk, unless some disk is idle.
+	 * Some readable nonrot disk: that nonrot disk with fewest pending I/Os.
 	 */
 	if (ctl.min_pending_disk != -1 &&
 	    (has_nonrot || ctl.min_pending == 0))
 		return ctl.min_pending_disk;
 	else
-- 
2.55.0

Re: [RFC PATCH 0/5] md/raid1: improve choose_best_rdev read balance

[yu kuai]

Hi,

在 2026/8/18 15:06, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> Prefer a nonrot disk when one can take the read. When pending is
> the same, use a real round-robin among the disks we may pick.

Please note that adding new atomic operations per IO is not acceptable, future
solutions should also try to kill all existed atomic operations as well.

And your solution seems break an important case that if small sequence IO are
issued to the same disk, they can be merged by blk plug. I don't see test result
for this case.

In fact, I'll prefer a simple solution, keep the sequence IO to same disk until
they can't be merged(by io_opt or hw_sectors), and choose next simply by rr. IO
work flow can be complicated, and I'll prefer the simplest solution with minimal
overhead.

>
> fio libaio direct=1, NVMe none, SATA mq-deadline (stock -> this):
>
> - 2x GM9000: 1M QD16 j1 7.031 -> 13.886 GB/s, 50/50; 4k QD1 50/50
> - 2x/3x HDWG740: 0.294 -> 0.514 / 0.630 GB/s
> - GM9000+HDD seq: 2.468 or 0.280 -> 7.112 GB/s, 100/0 NVMe
> - Fanxiang+HDD seq, HDD first: 0.278 -> 0.562 GB/s
> - GM9000+HDD 4k QD8/16: clat -47% / -45%
>
>
> Chen Cheng (5):
>    md/raid1: balance reads across non-rotational disks
>    md/raid1: do not move nonrot reads onto a rot disk
>    md/raid1: do not send random reads to a rot disk
>    md/raid1: use rot policy when no nonrot disk is readable
>    md/raid1: clarify choose_best_rdev comments
>
>   drivers/md/raid1.c | 122 ++++++++++++++++++++++++++++++++++++++-------
>   drivers/md/raid1.h |   1 +
>   2 files changed, 104 insertions(+), 19 deletions(-)
>
-- 
Thanks,
Kuai