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

[Chen Cheng]

From: Chen Cheng <chencheng@xxxxxxxxx>

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@xxxxxxxxx>
---
 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