All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/7] btrfs: read_policy types latency, device and
@ 2020-10-26 23:55 Anand Jain
  2020-10-26 23:55 ` [PATCH RFC 1/7] block: export part_stat_read_all Anand Jain
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs

Based on mainline 5.10-rc1

Depends on the following 3 patches in the mailing list.
  btrfs: add btrfs_strmatch helper
  btrfs: create read policy framework
  btrfs: create read policy sysfs attribute, pid

Patch 1-3 adds new read policy: latency.
Patch 4 adds tracing function.
Patch 5-6 were sent before they add read policy: device.
Patch 7 adds new read policy: round-robin.

I am sending this patchset to receive feedback, comments, and more test
results. Few cleanups, optimization, and broader performance test are
may be possible. This patchset does not alter the original default
read_policy that is PID.

This has been tested on VMs only, as my hardware based test host is
currently down.

This patchset adds read policies type latency, device, and round-robin.

These policies are for the mirrored raid profiles such as raid1, raid1c3,
raid1c4, and raid10 as they provide a choice to read from any one of the
given devices or any one pair of devices for raid10 for a given block.

Latency:

Latency read policy routes the read IO based on the historical average
wait time experienced by the read IOs on the individual device factored
by 1/10 of inflight commands in the queue. I need to add a factor of 1/10
because for most of the block devices the queue depth is more than 1, and
there can be commands in the queue even before the previous commands have
completed. However, the factor 1/10 for the inflight commands is yet to be
fine-tuned. Suggestions are welcome.

Device:

With the device read policy along with the read_preferred flag, you can
set the device to be used for reading manually. Useful to test mirrors in
a deterministic way and helps advance system administrations.

Round-robin:

When set picks the next device to read in the round-robin loop. To
achieve this first we put the stripes in an array, sort it by devid and
pick the next device.

The test results from my VM with 2 devices of type sata and 2 devices of 
type virtio, are as below.

Here below I have included a few scripts which were useful for testing.

-------------------8<--------------------------------
$ cat readpolicyset
#!/bin/bash

: ${1?"arg1 <mnt> missing"}
: ${2?"arg2 <pid|latency|device|roundrobin> missing"}

mnt=$1
policy=$2
[ $policy == "device" ] && { : ${3?"arg3 <devid> missing"}; }
devid=$3

uuid=$(btrfs fi show -m /btrfs | grep uuid | awk '{print $4}')
p=/sys/fs/btrfs/$uuid/read_policy
q=/sys/fs/btrfs/$uuid/devinfo

[ $policy == "device" ] && { echo 1 > ${q}/$devid/read_preferred || exit $?; }

echo $policy > $p
exit $?
-------------------8<--------------------------------

$ cat readpolicy
#!/bin/bash

: ${1?"arg1 <mnt> missing"}
mnt=$1

uuid=$(btrfs fi show -m /btrfs | grep uuid | awk '{print $4}')
p=/sys/fs/btrfs/$uuid/read_policy
q=/sys/fs/btrfs/$uuid/devinfo

policy=$(cat $p)
echo -n "$policy ( "

for i in $(find $q -type f -name read_preferred | xargs cat)
do
	echo -n "$i"
done
echo ")"
-------------------8<--------------------------------

$ cat readstat 
#!/bin/bash

: ${1?"arg1 <mnt> is missing"}
: ${2?"arg2 <cmd-to-run> is missing"}

mnt=$1; shift
mountpoint -q $mnt || { echo "ERROR: $mnt is not mounted"; exit 1; }

declare -A devread

for dev in $(btrfs filesystem show -m $mnt | grep devid |awk '{print $8}')
do
	prefix=$(echo $dev | rev | cut -d"/" -f1 | rev)
	sysfs_path=$(find /sys | grep $prefix/stat$)

	devread[$sysfs_path]=$(cat $sysfs_path | awk '{print $1}')
done

"$@" | grep "READ: bw"

echo
echo
for sysfs_path in ${!devread[@]}
do
	dev=$(echo $sysfs_path | rev | cut -d"/" -f2 | rev)
	new=$(cat $sysfs_path | awk '{print $1}')
	old=${devread[$sysfs_path]}
	echo "$dev $((new - old))"
done
-------------------8<--------------------------------

$ cat fioread 
#!/bin/bash

: ${1?"arg1 </mnt/file> is missing"}
: ${2?"arg2 <1Gi|50Gi> is missing"}

tf=$1
sz=$2
mnt=$(stat -c '%m' $tf)

fio \
--filename=$tf \
--directory=$mnt \
--filesize=$sz \
--size=$sz \
--rw=randread \
--bs=64k \
--ioengine=libaio \
--direct=1 \
--numjobs=32 \
--group_reporting \
--thread \
--name iops-test-job
-------------------8<--------------------------------


Here below are the performance results for raid1c4, raid10, and raid1.

The workload is fio read 32 threads, 500m random reads.

Fio is passed to the script readstat, which returns the number of read IOs
per device sent during the fio.

Few inferences:

1.
In some cases as mine, PID is also associated with the stripe 0 being
circulating among the devices at the chunk level before moving to the next
device. So PID is not a bad option for such configurations. However, it is
not a good choice if the devices are heterogeneous in terms of type, speed,
and size.

2.
Latency provides performance equal to PID on my test setup. But needs iostat
be enabled. And there is a cost of calculating the avg wait time. (If you
factor in the similar cost to PID using the test code [1] then latency's
performance is better than PID. So that proves that read IO distribution
as per latency is really working)

3.
Round robin is worst (unless there is a bug in my patch). As the total
number of new IOs issued was almost double compared to the PID and the
latency read_policy, that's because I think there was a fewer number of
merges due to constant switching of devices.

4.
Device read_policy is quite useful in testing and advance sysadmin
maintenance. When known how to use, this policy could help to avert
too many csum errors in midest of peak production.

Supporting fio logs are below. And readstat shows the number of read IOs
issued to devices (excluding the merges).

raid1c4
-------

pid:

$ readpolicyset /btrfs pid && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
[pid] latency device roundrobin ( 0000)
 READ: bw=87.0MiB/s (91.2MB/s), 87.0MiB/s-87.0MiB/s (91.2MB/s-91.2MB/s), io=15.6GiB (16.8GB), run=183884-183884msec

vdb 64060
vdc 64053
sdb 64072
sda 64054


latency:  (All devices are non-rotational, but sda and sdb are of type
sata and vdb and vdc are of type virtio).

$ readpolicyset /btrfs latency && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m

pid [latency] device roundrobin ( 0000)
 READ: bw=87.1MiB/s (91.3MB/s), 87.1MiB/s-87.1MiB/s (91.3MB/s-91.3MB/s), io=15.6GiB (16.8GB), run=183774-183774msec

vdb 255844
vdc 559
sdb 0
sda 93

roundrobin:

$ readpolicyset /btrfs roundrobin && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m

pid latency device [roundrobin] ( 0000)
 READ: bw=51.0MiB/s (54.5MB/s), 51.0MiB/s-51.0MiB/s (54.5MB/s-54.5MB/s), io=15.6GiB (16.8GB), run=307755-307755msec

vdb 866859
vdc 866651
sdb 864139
sda 865533


raid10
------

pid:

$ readpolicyset /btrfs pid && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
[pid] latency device roundrobin ( 0000)
 READ: bw=85.2MiB/s (89.3MB/s), 85.2MiB/s-85.2MiB/s (89.3MB/s-89.3MB/s), io=15.6GiB (16.8GB), run=187864-187864msec


sdf 64053
sde 64036
sdd 64043
sdc 64038


latency:

$ readpolicyset /btrfs latency && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
pid [latency] device roundrobin ( 0000)
 READ: bw=85.4MiB/s (89.5MB/s), 85.4MiB/s-85.4MiB/s (89.5MB/s-89.5MB/s), io=15.6GiB (16.8GB), run=187370-187370msec


sdf 117494
sde 10748
sdd 125247
sdc 2921

roundrobin:

$ readpolicyset /btrfs roundrobin && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
pid latency device [roundrobin] ( 0000)
 READ: bw=55.4MiB/s (58.1MB/s), 55.4MiB/s-55.4MiB/s (58.1MB/s-58.1MB/s), io=15.6GiB (16.8GB), run=288701-288701msec

sdf 617593
sde 617381
sdd 618486
sdc 618633


raid1:

$ readpolicyset /btrfs pid && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
[pid] latency device roundrobin ( 00)
 READ: bw=78.8MiB/s (82.6MB/s), 78.8MiB/s-78.8MiB/s (82.6MB/s-82.6MB/s), io=15.6GiB (16.8GB), run=203158-203158msec


sdb 128087
sda 128090

$ readpolicyset /btrfs latency && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
pid [latency] device roundrobin ( 00)
 READ: bw=86.5MiB/s (90.7MB/s), 86.5MiB/s-86.5MiB/s (90.7MB/s-90.7MB/s), io=15.6GiB (16.8GB), run=185023-185023msec


sdb 567
sda 255942


From the latency test results we know sda is providing low latency read
IO. So set sda as read proffered device.

$ readpolicyset /btrfs device 1 && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
pid latency [device] roundrobin ( 10)
 READ: bw=88.2MiB/s (92.5MB/s), 88.2MiB/s-88.2MiB/s (92.5MB/s-92.5MB/s), io=15.6GiB (16.8GB), run=181374-181374msec


sdb 0
sda 256191


$ readpolicyset /btrfs roundrobin && readpolicy /btrfs && dropcache && readstat /btrfs fioread /btrfs/largefile 500m
pid latency device [roundrobin] ( 00)
 READ: bw=54.1MiB/s (56.7MB/s), 54.1MiB/s-54.1MiB/s (56.7MB/s-56.7MB/s), io=15.6GiB (16.8GB), run=295693-295693msec


sdb 1252584
sda 1254258


Thanks, Anand


------------------
[1]
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d3023879bdf6..72ec633e9063 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5665,6 +5665,12 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 fs_info->fs_devices->read_policy = BTRFS_READ_POLICY_PID;
 fallthrough;
 case BTRFS_READ_POLICY_PID:
+ /*
+ * Just to factor in the cost of calculating the avg wait using
+ * iostat for testing.
+ */
+ btrfs_find_best_stripe(fs_info, map, first, num_stripes, log,
+ logsz);
 preferred_mirror = first + current->pid % num_stripes;
 scnprintf(log, logsz,
 "first %d num_stripe %d %s (%d) preferred %d",

Anand Jain (7):
  block: export part_stat_read_all
  block: export part_stat_read_inflight
  btrfs: add read_policy latency
  btrfs: trace, add event btrfs_read_policy
  btrfs: introduce new device-state read_preferred
  btrfs: introduce new read_policy device
  btrfs: introduce new read_policy round-robin

 block/genhd.c                |  22 ++--
 fs/btrfs/sysfs.c             |  59 ++++++++++-
 fs/btrfs/volumes.c           | 188 ++++++++++++++++++++++++++++++++++-
 fs/btrfs/volumes.h           |   5 +
 include/linux/genhd.h        |   3 +
 include/trace/events/btrfs.h |  20 ++++
 6 files changed, 286 insertions(+), 11 deletions(-)

-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 1/7] block: export part_stat_read_all
  2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
@ 2020-10-26 23:55 ` Anand Jain
  2020-10-27 18:09   ` Josef Bacik
  2020-10-26 23:55 ` [PATCH RFC 2/7] block: export part_stat_read_inflight Anand Jain
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs; +Cc: linux-block

For mirrored raid profiles such as raid1, raid1c3, raid1c4, and raid10,
currently, btrfs use the PID method to determine which one of the
mirrored devices to use to read the block. However, the PID method is
not the best choice if the devices are heterogeneous in terms of type,
speed, and size, as we may end up reading from the slower device.

Export the function part_stat_read_all() so that the btrfs can determine
the device with the least average wait time to use.

Cc: linux-block@vger.kernel.org
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 block/genhd.c         | 3 ++-
 include/linux/genhd.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/genhd.c b/block/genhd.c
index 0a273211fec2..81b10b90de71 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -89,7 +89,7 @@ const char *bdevname(struct block_device *bdev, char *buf)
 }
 EXPORT_SYMBOL(bdevname);
 
-static void part_stat_read_all(struct hd_struct *part, struct disk_stats *stat)
+void part_stat_read_all(struct hd_struct *part, struct disk_stats *stat)
 {
 	int cpu;
 
@@ -108,6 +108,7 @@ static void part_stat_read_all(struct hd_struct *part, struct disk_stats *stat)
 		stat->io_ticks += ptr->io_ticks;
 	}
 }
+EXPORT_SYMBOL_GPL(part_stat_read_all);
 
 static unsigned int part_in_flight(struct hd_struct *part)
 {
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 38f23d757013..eb77e0ac8a82 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -281,6 +281,7 @@ struct disk_part_iter {
 	unsigned int		flags;
 };
 
+extern void part_stat_read_all(struct hd_struct *part, struct disk_stats *stat);
 extern void disk_part_iter_init(struct disk_part_iter *piter,
 				 struct gendisk *disk, unsigned int flags);
 extern struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 2/7] block: export part_stat_read_inflight
  2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
  2020-10-26 23:55 ` [PATCH RFC 1/7] block: export part_stat_read_all Anand Jain
@ 2020-10-26 23:55 ` Anand Jain
  2020-10-27 18:10   ` Josef Bacik
  2020-10-26 23:55 ` [PATCH RFC 3/7] btrfs: add read_policy latency Anand Jain
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs; +Cc: linux-block

The exported function part_in_flight() returns commands in-flight in the
given block device.

Cc: linux-block@vger.kernel.org
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 block/genhd.c         | 19 +++++++++++--------
 include/linux/genhd.h |  2 ++
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 81b10b90de71..21876d4cf2fb 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -125,6 +125,15 @@ static unsigned int part_in_flight(struct hd_struct *part)
 	return inflight;
 }
 
+unsigned int part_stat_read_inflight(struct request_queue *q, struct hd_struct *p)
+{
+	if (queue_is_mq(q))
+		return blk_mq_in_flight(q, p);
+	else
+		return part_in_flight(p);
+}
+EXPORT_SYMBOL_GPL(part_stat_read_inflight);
+
 static void part_in_flight_rw(struct hd_struct *part, unsigned int inflight[2])
 {
 	int cpu;
@@ -1291,10 +1300,7 @@ ssize_t part_stat_show(struct device *dev,
 	unsigned int inflight;
 
 	part_stat_read_all(p, &stat);
-	if (queue_is_mq(q))
-		inflight = blk_mq_in_flight(q, p);
-	else
-		inflight = part_in_flight(p);
+	inflight = part_stat_read_inflight(q, p);
 
 	return sprintf(buf,
 		"%8lu %8lu %8llu %8u "
@@ -1613,10 +1619,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
 	disk_part_iter_init(&piter, gp, DISK_PITER_INCL_EMPTY_PART0);
 	while ((hd = disk_part_iter_next(&piter))) {
 		part_stat_read_all(hd, &stat);
-		if (queue_is_mq(gp->queue))
-			inflight = blk_mq_in_flight(gp->queue, hd);
-		else
-			inflight = part_in_flight(hd);
+		inflight = part_stat_read_inflight(gp->queue, hd);
 
 		seq_printf(seqf, "%4d %7d %s "
 			   "%lu %lu %lu %u "
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index eb77e0ac8a82..93dd7a96d444 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -282,6 +282,8 @@ struct disk_part_iter {
 };
 
 extern void part_stat_read_all(struct hd_struct *part, struct disk_stats *stat);
+extern unsigned int part_stat_read_inflight(struct request_queue *q,
+					    struct hd_struct *part);
 extern void disk_part_iter_init(struct disk_part_iter *piter,
 				 struct gendisk *disk, unsigned int flags);
 extern struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 3/7] btrfs: add read_policy latency
  2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
  2020-10-26 23:55 ` [PATCH RFC 1/7] block: export part_stat_read_all Anand Jain
  2020-10-26 23:55 ` [PATCH RFC 2/7] block: export part_stat_read_inflight Anand Jain
@ 2020-10-26 23:55 ` Anand Jain
  2020-10-27 18:20   ` Josef Bacik
  2020-10-26 23:55 ` [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy Anand Jain
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs

The read policy type latency routes the read IO based on the historical
average wait time experienced by the read IOs through the individual
device factored by 1/10 of inflight commands in the queue. The factor
1/10 is because generally the block device queue depth is more than 1,
so there can be commands in the queue even before the previous commands
have been completed. This patch obtains the historical read IO stats from
the kernel block layer.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c   |  3 +-
 fs/btrfs/volumes.c | 74 +++++++++++++++++++++++++++++++++++++++++++++-
 fs/btrfs/volumes.h |  1 +
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index d159f7c70bcd..6690abeeb889 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -874,7 +874,8 @@ static int btrfs_strmatch(const char *given, const char *golden)
 	return -EINVAL;
 }
 
-static const char * const btrfs_read_policy_name[] = { "pid" };
+/* Must follow the order as in enum btrfs_read_policy */
+static const char * const btrfs_read_policy_name[] = { "pid", "latency"};
 
 static ssize_t btrfs_read_policy_show(struct kobject *kobj,
 				      struct kobj_attribute *a, char *buf)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index da31b11ceb61..9bab6080cebf 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -14,6 +14,7 @@
 #include <linux/semaphore.h>
 #include <linux/uuid.h>
 #include <linux/list_sort.h>
+#include <linux/part_stat.h>
 #include "misc.h"
 #include "ctree.h"
 #include "extent_map.h"
@@ -5465,6 +5466,66 @@ int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
 	return ret;
 }
 
+static u64 btrfs_estimate_read(struct btrfs_device *device,
+			       unsigned long *inflight)
+{
+	u64 read_wait;
+	u64 avg_wait = 0;
+	unsigned long read_ios;
+	struct disk_stats stat;
+
+	/* Commands in flight on this partition/device */
+	*inflight = part_stat_read_inflight(bdev_get_queue(device->bdev),
+					    device->bdev->bd_part);
+	part_stat_read_all(device->bdev->bd_part, &stat);
+
+	read_wait = stat.nsecs[STAT_READ];
+	read_ios = stat.ios[STAT_READ];
+
+	if (read_wait && read_ios && read_wait >= read_ios)
+		avg_wait = div_u64(read_wait, read_ios);
+	else
+		btrfs_info_rl(device->fs_devices->fs_info,
+			"devid: %llu avg_wait ZERO read_wait %llu read_ios %lu",
+			      device->devid, read_wait, read_ios);
+
+	return avg_wait;
+}
+
+static int btrfs_find_best_stripe(struct btrfs_fs_info *fs_info,
+				  struct map_lookup *map, int first,
+				  int num_stripe)
+{
+	int index;
+	int best_stripe = 0;
+	int est_wait = -EINVAL;
+	int last = first + num_stripe;
+	unsigned long inflight;
+
+	for (index = first; index < last; index++) {
+		struct btrfs_device *device = map->stripes[index].dev;
+
+		if (!blk_queue_io_stat(bdev_get_queue(device->bdev)))
+			return -ENOENT;
+	}
+
+	for (index = first; index < last; index++) {
+		struct btrfs_device *device = map->stripes[index].dev;
+		u64 avg_wait;
+		u64 final_wait;
+
+		avg_wait = btrfs_estimate_read(device, &inflight);
+		final_wait = avg_wait + (avg_wait * (inflight / 10));
+
+		if (est_wait == 0 || est_wait > final_wait) {
+			est_wait = final_wait;
+			best_stripe = index;
+		}
+	}
+
+	return best_stripe;
+}
+
 static int find_live_mirror(struct btrfs_fs_info *fs_info,
 			    struct map_lookup *map, int first,
 			    int dev_replace_is_ongoing)
@@ -5491,6 +5552,18 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 		btrfs_warn_rl(fs_info,
 			      "unknown read_policy type %u, fallback to pid",
 			      fs_info->fs_devices->read_policy);
+		fs_info->fs_devices->read_policy = BTRFS_READ_POLICY_PID;
+		preferred_mirror = first + current->pid % num_stripes;
+		break;
+	case BTRFS_READ_POLICY_LATENCY:
+		preferred_mirror = btrfs_find_best_stripe(fs_info, map, first,
+							  num_stripes);
+		if (preferred_mirror >= 0)
+			break;
+
+		btrfs_warn(fs_info,
+   "iostat is disabled, cannot set latency read_policy, fallback to pid");
+		fs_info->fs_devices->read_policy = BTRFS_READ_POLICY_PID;
 		fallthrough;
 	case BTRFS_READ_POLICY_PID:
 		preferred_mirror = first + current->pid % num_stripes;
@@ -6111,7 +6184,6 @@ static int __btrfs_map_block(struct btrfs_fs_info *fs_info,
 
 	} else if (map->type & BTRFS_BLOCK_GROUP_RAID10) {
 		u32 factor = map->num_stripes / map->sub_stripes;
-
 		stripe_nr = div_u64_rem(stripe_nr, factor, &stripe_index);
 		stripe_index *= map->sub_stripes;
 
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index e3c36951742d..8705d755d148 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -217,6 +217,7 @@ enum btrfs_chunk_allocation_policy {
  */
 enum btrfs_read_policy {
 	BTRFS_READ_POLICY_PID,
+	BTRFS_READ_POLICY_LATENCY,
 	BTRFS_NR_READ_POLICY,
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy
  2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
                   ` (2 preceding siblings ...)
  2020-10-26 23:55 ` [PATCH RFC 3/7] btrfs: add read_policy latency Anand Jain
@ 2020-10-26 23:55 ` Anand Jain
  2020-10-27 18:22   ` Josef Bacik
  2020-10-26 23:55 ` [PATCH 5/7] btrfs: introduce new device-state read_preferred Anand Jain
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs

This patch adds trace event btrfs_read_policy, which is common to all the
read policies.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c           | 28 ++++++++++++++++++++++++++--
 include/trace/events/btrfs.h | 20 ++++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 9bab6080cebf..b51e690b3b18 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5494,9 +5494,10 @@ static u64 btrfs_estimate_read(struct btrfs_device *device,
 
 static int btrfs_find_best_stripe(struct btrfs_fs_info *fs_info,
 				  struct map_lookup *map, int first,
-				  int num_stripe)
+				  int num_stripe, char *log, int logsz)
 {
 	int index;
+	int lognr;
 	int best_stripe = 0;
 	int est_wait = -EINVAL;
 	int last = first + num_stripe;
@@ -5509,6 +5510,8 @@ static int btrfs_find_best_stripe(struct btrfs_fs_info *fs_info,
 			return -ENOENT;
 	}
 
+	lognr = scnprintf(log, logsz, ": first %d num_stripe %d [",
+			  first, num_stripe);
 	for (index = first; index < last; index++) {
 		struct btrfs_device *device = map->stripes[index].dev;
 		u64 avg_wait;
@@ -5517,11 +5520,19 @@ static int btrfs_find_best_stripe(struct btrfs_fs_info *fs_info,
 		avg_wait = btrfs_estimate_read(device, &inflight);
 		final_wait = avg_wait + (avg_wait * (inflight / 10));
 
+		lognr = lognr + scnprintf(log + lognr, logsz - lognr,
+		 " stripe %d devid %llu wait %llu inflight %lu final %llu,",
+					  index, device->devid, avg_wait,
+					  inflight, final_wait);
+
 		if (est_wait == 0 || est_wait > final_wait) {
 			est_wait = final_wait;
 			best_stripe = index;
 		}
 	}
+	lognr = lognr + scnprintf(log + lognr, logsz - lognr,
+				  "] best_stripe %d devid %llu", best_stripe,
+				  map->stripes[best_stripe].dev->devid);
 
 	return best_stripe;
 }
@@ -5535,6 +5546,8 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 	int preferred_mirror;
 	int tolerance;
 	struct btrfs_device *srcdev;
+	int logsz = 400;
+	char log[400] = {0};
 
 	ASSERT((map->type &
 		 (BTRFS_BLOCK_GROUP_RAID1_MASK | BTRFS_BLOCK_GROUP_RAID10)));
@@ -5557,7 +5570,8 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 		break;
 	case BTRFS_READ_POLICY_LATENCY:
 		preferred_mirror = btrfs_find_best_stripe(fs_info, map, first,
-							  num_stripes);
+							  num_stripes, log,
+							  logsz);
 		if (preferred_mirror >= 0)
 			break;
 
@@ -5567,9 +5581,19 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 		fallthrough;
 	case BTRFS_READ_POLICY_PID:
 		preferred_mirror = first + current->pid % num_stripes;
+		scnprintf(log, logsz,
+			  "first %d num_stripe %d %s (%d) preferred %d",
+			  first, num_stripes, current->comm, task_pid_nr(current),
+			  preferred_mirror);
 		break;
 	}
 
+	/*
+	 * we could enhance tracing to trace other policies as well, so please
+	 * keep this in the parent function.
+	 */
+	trace_btrfs_read_policy(fs_info, log);
+
 	if (dev_replace_is_ongoing &&
 	    fs_info->dev_replace.cont_reading_from_srcdev_mode ==
 	     BTRFS_DEV_REPLACE_ITEM_CONT_READING_FROM_SRCDEV_MODE_AVOID)
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index ecd24c719de4..8df7e7d0b336 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -2148,6 +2148,26 @@ DEFINE_EVENT(btrfs__space_info_update, update_bytes_pinned,
 	TP_ARGS(fs_info, sinfo, old, diff)
 );
 
+TRACE_EVENT(btrfs_read_policy,
+
+	TP_PROTO(const struct btrfs_fs_info *fs_info, const char *log),
+
+	TP_ARGS(fs_info, log),
+
+	TP_STRUCT__entry_btrfs(
+		__string(	log,	log		)
+		__field(	int,	read_policy	)
+	),
+
+	TP_fast_assign_btrfs(fs_info,
+		__assign_str(log, log);
+		__entry->read_policy	= fs_info->fs_devices->read_policy;
+	),
+
+	TP_printk_btrfs("read_policy %d %s",
+			__entry->read_policy, __get_str(log))
+);
+
 #endif /* _TRACE_BTRFS_H */
 
 /* This part must be outside protection */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/7] btrfs: introduce new device-state read_preferred
  2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
                   ` (3 preceding siblings ...)
  2020-10-26 23:55 ` [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy Anand Jain
@ 2020-10-26 23:55 ` Anand Jain
  2020-10-26 23:55 ` [PATCH 6/7] btrfs: introduce new read_policy device Anand Jain
  2020-10-26 23:55 ` [PATCH RFC 7/7] btrfs: introduce new read_policy round-robin Anand Jain
  6 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs

This is a preparatory patch and introduces a new device flag
'read_preferred', RW-able using sysfs interface.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c   | 55 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h |  1 +
 2 files changed, 56 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 6690abeeb889..2339c4a1ea2d 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1380,11 +1380,66 @@ static ssize_t btrfs_devinfo_writeable_show(struct kobject *kobj,
 }
 BTRFS_ATTR(devid, writeable, btrfs_devinfo_writeable_show);
 
+static ssize_t btrfs_devinfo_read_pref_show(struct kobject *kobj,
+					    struct kobj_attribute *a, char *buf)
+{
+	int val;
+	struct btrfs_device *device = container_of(kobj, struct btrfs_device,
+						   devid_kobj);
+
+	val = !!test_bit(BTRFS_DEV_STATE_READ_PREFERRED, &device->dev_state);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", val);
+}
+
+static ssize_t btrfs_devinfo_read_pref_store(struct kobject *kobj,
+					     struct kobj_attribute *a,
+					     const char *buf, size_t len)
+{
+	int ret;
+	unsigned long val;
+	struct btrfs_device *device;
+
+	ret = kstrtoul(skip_spaces(buf), 0, &val);
+	if (ret)
+		return ret;
+
+	if (val != 0 && val != 1)
+		return -EINVAL;
+
+	/*
+	 * lock is not required, the btrfs_device struct can't be freed while
+	 * its kobject btrfs_device::devid_kobj is still open.
+	 */
+	device = container_of(kobj, struct btrfs_device, devid_kobj);
+
+	if (val &&
+	    ! test_bit(BTRFS_DEV_STATE_READ_PREFERRED, &device->dev_state)) {
+
+		set_bit(BTRFS_DEV_STATE_READ_PREFERRED, &device->dev_state);
+		btrfs_info(device->fs_devices->fs_info,
+			   "set read preferred on devid %llu (%d)",
+			   device->devid, task_pid_nr(current));
+	} else if (!val &&
+		   test_bit(BTRFS_DEV_STATE_READ_PREFERRED, &device->dev_state)) {
+
+		clear_bit(BTRFS_DEV_STATE_READ_PREFERRED, &device->dev_state);
+		btrfs_info(device->fs_devices->fs_info,
+			   "reset read preferred on devid %llu (%d)",
+			   device->devid, task_pid_nr(current));
+	}
+
+	return len;
+}
+BTRFS_ATTR_RW(devid, read_preferred, btrfs_devinfo_read_pref_show,
+	      btrfs_devinfo_read_pref_store);
+
 static struct attribute *devid_attrs[] = {
 	BTRFS_ATTR_PTR(devid, in_fs_metadata),
 	BTRFS_ATTR_PTR(devid, missing),
 	BTRFS_ATTR_PTR(devid, replace_target),
 	BTRFS_ATTR_PTR(devid, writeable),
+	BTRFS_ATTR_PTR(devid, read_preferred),
 	NULL
 };
 ATTRIBUTE_GROUPS(devid);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 8705d755d148..6e5518bf9fe5 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -50,6 +50,7 @@ struct btrfs_io_geometry {
 #define BTRFS_DEV_STATE_MISSING		(2)
 #define BTRFS_DEV_STATE_REPLACE_TGT	(3)
 #define BTRFS_DEV_STATE_FLUSH_SENT	(4)
+#define BTRFS_DEV_STATE_READ_PREFERRED	(6)
 
 struct btrfs_device {
 	struct list_head dev_list; /* device_list_mutex */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/7] btrfs: introduce new read_policy device
  2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
                   ` (4 preceding siblings ...)
  2020-10-26 23:55 ` [PATCH 5/7] btrfs: introduce new device-state read_preferred Anand Jain
@ 2020-10-26 23:55 ` Anand Jain
  2020-10-27  7:11   ` Wang Yugui
  2020-10-26 23:55 ` [PATCH RFC 7/7] btrfs: introduce new read_policy round-robin Anand Jain
  6 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs

Read-policy type 'device' and device flag 'read-preferred':

The read-policy type device picks the device(s) flagged as
read-preferred for reading chunks of type raid1, raid10,
raid1c3 and raid1c4.

A system might contain SSD, nvme, iscsi or san lun, and which are all
a non-rotational device, so it is not a good idea to set the read-preferred
automatically. Instead device read-policy along with the read-preferred
flag provides an ability to do it manually. This advance tuning is
useful in more than one situation, for example,
 - In heterogeneous-disk volume, it provides an ability to manually choose
    the low latency disks for reading.
 - Useful for more accurate testing.
 - Avoid known problematic device from reading the chunk until it is
   replaced (by marking the other good devices as read-preferred).

Note:

If the read-policy type is set to 'device', but there isn't any device
which is flagged as read-preferred, then stripe 0 is used for reading.

The device replace won't migrate the read-preferred flag to the new
replace the target device.

As of now, this is an in-memory only feature.

It's pointless to set the read-preferred flag on the missing device,
as IOs aren't submitted to the missing device.

If there is more than one read-preferred device in a chunk, the read IO
shall go to the stripe 0 (as of now, when depth patches are integrated
we will use the least busy device among the read-preferred devices).

Usage example:

Consider a typical two disks raid1.

Configure devid1 for reading.

$ echo 1 > devinfo/1/read_preferred
$ cat devinfo/1/read_preferred; cat devinfo/2/read_preferred
1
0

$ pwd
/sys/fs/btrfs/12345678-1234-1234-1234-123456789abc

$ cat read_policy; echo device > ./read_policy; cat read_policy
[pid] device
pid [device]

Now read IOs are sent to devid 1 (sdb).

$ echo 3 > /proc/sys/vm/drop_caches; md5sum /btrfs/YkZI

$ iostat -zy 1 | egrep 'sdb|sdc' (from another terminal)
sdb              50.00     40048.00         0.00      40048          0

Change the read-preferred device from devid 1 to devid 2 (sdc).

$ echo 0 > ./devinfo/1/read_preferred; echo 1 > ./devinfo/2/read_preferred;

[ 3343.918658] BTRFS info (device sdb): reset read preferred on devid 1 (1334)
[ 3343.919876] BTRFS info (device sdb): set read preferred on devid 2 (1334)

Further read ios are sent to devid 2 (sdc).

$ echo 3 > /proc/sys/vm/drop_caches; md5sum /btrfs/YkZI

$ iostat -zy 1 | egrep 'sdb|sdc' (from another terminal)
sdc              49.00     40048.00         0.00      40048          0

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c   |  3 ++-
 fs/btrfs/volumes.c | 22 ++++++++++++++++++++++
 fs/btrfs/volumes.h |  1 +
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 2339c4a1ea2d..677070bab1e0 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -875,7 +875,8 @@ static int btrfs_strmatch(const char *given, const char *golden)
 }
 
 /* Must follow the order as in enum btrfs_read_policy */
-static const char * const btrfs_read_policy_name[] = { "pid", "latency"};
+static const char * const btrfs_read_policy_name[] = { "pid", "latency",
+						       "device" };
 
 static ssize_t btrfs_read_policy_show(struct kobject *kobj,
 				      struct kobj_attribute *a, char *buf)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b51e690b3b18..e71af8ab4ad8 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5537,6 +5537,25 @@ static int btrfs_find_best_stripe(struct btrfs_fs_info *fs_info,
 	return best_stripe;
 }
 
+static int btrfs_find_read_preferred(struct map_lookup *map, int first, int num_stripe)
+{
+	int stripe_index;
+	int last = first + num_stripe;
+
+	/*
+	 * If there are more than one read preferred devices, then just pick the
+	 * first found read preferred device as of now.
+	 */
+	for (stripe_index = first; stripe_index < last; stripe_index++) {
+		if (test_bit(BTRFS_DEV_STATE_READ_PREFERRED,
+			     &map->stripes[stripe_index].dev->dev_state))
+			return stripe_index;
+        }
+
+	/* If there is no read preferred device then just use the first stripe */
+	return first;
+}
+
 static int find_live_mirror(struct btrfs_fs_info *fs_info,
 			    struct map_lookup *map, int first,
 			    int dev_replace_is_ongoing)
@@ -5558,6 +5577,9 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 		num_stripes = map->num_stripes;
 
 	switch (fs_info->fs_devices->read_policy) {
+	case BTRFS_READ_POLICY_DEVICE:
+		preferred_mirror = btrfs_find_read_preferred(map, first, num_stripes);
+		break;
 	default:
 		/*
 		 * Shouldn't happen, just warn and use pid instead of failing.
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 6e5518bf9fe5..40e56287204a 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -219,6 +219,7 @@ enum btrfs_chunk_allocation_policy {
 enum btrfs_read_policy {
 	BTRFS_READ_POLICY_PID,
 	BTRFS_READ_POLICY_LATENCY,
+	BTRFS_READ_POLICY_DEVICE,
 	BTRFS_NR_READ_POLICY,
 };
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH RFC 7/7] btrfs: introduce new read_policy round-robin
  2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
                   ` (5 preceding siblings ...)
  2020-10-26 23:55 ` [PATCH 6/7] btrfs: introduce new read_policy device Anand Jain
@ 2020-10-26 23:55 ` Anand Jain
  6 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2020-10-26 23:55 UTC (permalink / raw)
  To: linux-btrfs

Add round-robin read policy to route the read IO to the next device in the
round-robin order. The chunk allocation and thus the stripe-index follows
the order of free space available on devices. So to make the round-robin
effective it shall follow the devid order instead of the stripe-index
order.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c   |  2 +-
 fs/btrfs/volumes.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h |  2 ++
 3 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 677070bab1e0..45efc9755336 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -876,7 +876,7 @@ static int btrfs_strmatch(const char *given, const char *golden)
 
 /* Must follow the order as in enum btrfs_read_policy */
 static const char * const btrfs_read_policy_name[] = { "pid", "latency",
-						       "device" };
+						       "device", "roundrobin" };
 
 static ssize_t btrfs_read_policy_show(struct kobject *kobj,
 				      struct kobj_attribute *a, char *buf)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index e71af8ab4ad8..d3023879bdf6 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5466,6 +5466,69 @@ int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
 	return ret;
 }
 
+struct stripe_mirror {
+	u64 devid;
+	int map;
+};
+
+static int btrfs_cmp_devid(const void *a, const void *b)
+{
+	struct stripe_mirror *s1 = (struct stripe_mirror *)a;
+	struct stripe_mirror *s2 = (struct stripe_mirror *)b;
+
+	if (s1->devid < s2->devid)
+		return -1;
+	if (s1->devid > s2->devid)
+		return 1;
+	return 0;
+}
+
+static int btrfs_find_read_round_robin(struct map_lookup *map, int first,
+				       int num_stripe, char *log, int logsz)
+{
+	struct stripe_mirror stripes[4] = {0}; //4: for testing, works for now.
+	struct btrfs_fs_devices *fs_devices;
+	u64 devid;
+	int index, j, cnt;
+	int lognr = 0;
+	int next_stripe;
+
+	index = 0;
+	lognr += scnprintf(log + lognr, logsz - lognr, "index=map:devid [");
+	for (j = first; j < first + num_stripe; j++) {
+		devid = map->stripes[j].dev->devid;
+
+		stripes[index].devid = devid;
+		stripes[index].map = j;
+
+		lognr += scnprintf(log + lognr, logsz - lognr, "%d=%d:%llu, ",
+				  index, j, devid);
+
+		index++;
+	}
+
+	sort(stripes, num_stripe, sizeof(struct stripe_mirror),
+	     btrfs_cmp_devid, NULL);
+
+	lognr += scnprintf(log + lognr, logsz - lognr, "] sorted=[");
+	for (index = 0; index < num_stripe; index++) {
+		j = stripes[index].map;
+		devid = stripes[index].devid;
+
+		lognr += scnprintf(log + lognr, logsz - lognr, "%d=%d:%llu, ",
+				  index, j, devid);
+	}
+
+	fs_devices = map->stripes[first].dev->fs_devices;
+	cnt = atomic_inc_return(&fs_devices->total_reads);
+	next_stripe = stripes[cnt % num_stripe].map;
+
+	lognr += scnprintf(log + lognr, logsz - lognr, "] next_stripe %d",
+			  next_stripe);
+
+	return next_stripe;
+}
+
 static u64 btrfs_estimate_read(struct btrfs_device *device,
 			       unsigned long *inflight)
 {
@@ -5608,6 +5671,11 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 			  first, num_stripes, current->comm, task_pid_nr(current),
 			  preferred_mirror);
 		break;
+	case BTRFS_READ_POLICY_ROUND_ROBIN:
+		preferred_mirror = btrfs_find_read_round_robin(map, first,
+							       num_stripes, log,
+							       logsz);
+		break;
 	}
 
 	/*
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 40e56287204a..571b52afcaa1 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -220,6 +220,7 @@ enum btrfs_read_policy {
 	BTRFS_READ_POLICY_PID,
 	BTRFS_READ_POLICY_LATENCY,
 	BTRFS_READ_POLICY_DEVICE,
+	BTRFS_READ_POLICY_ROUND_ROBIN,
 	BTRFS_NR_READ_POLICY,
 };
 
@@ -281,6 +282,7 @@ struct btrfs_fs_devices {
 	 * policy used to read the mirrored stripes
 	 */
 	enum btrfs_read_policy read_policy;
+	atomic_t total_reads;
 };
 
 #define BTRFS_BIO_INLINE_CSUM_SIZE	64
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] btrfs: introduce new read_policy device
  2020-10-26 23:55 ` [PATCH 6/7] btrfs: introduce new read_policy device Anand Jain
@ 2020-10-27  7:11   ` Wang Yugui
  0 siblings, 0 replies; 17+ messages in thread
From: Wang Yugui @ 2020-10-27  7:11 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs, louis

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

Hi, Anand Jain
Cc: Louis Jencka

> Read-policy type 'device' and device flag 'read-preferred':
> 
> The read-policy type device picks the device(s) flagged as
> read-preferred for reading chunks of type raid1, raid10,
> raid1c3 and raid1c4.
> 
> A system might contain SSD, nvme, iscsi or san lun, and which are all
> a non-rotational device, so it is not a good idea to set the read-preferred
> automatically. Instead device read-policy along with the read-preferred
> flag provides an ability to do it manually. This advance tuning is
> useful in more than one situation, for example,
>  - In heterogeneous-disk volume, it provides an ability to manually choose
>     the low latency disks for reading.
>  - Useful for more accurate testing.
>  - Avoid known problematic device from reading the chunk until it is
>    replaced (by marking the other good devices as read-preferred).

It is still OK to auto for the most common case of the mixed of ssd and
hdd?

I am trying 'manually if failed to auto' with a 'u8' var rather than a 'bool'
var.

There are 2 patch I am working but yet not completed.

and someone of them is based on 'btrfs: balance RAID1/RAID10 mirror
selection' from Louis Jencka louis@waffle.tech

Feel free to merge them into your patch as a new one please.

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2020/10/27


[-- Attachment #2: 0001-btrfs-add-tier-score-to-device.patch --]
[-- Type: application/octet-stream, Size: 2530 bytes --]

From 8a8f6405073f835531664aafa333570fba28c31f Mon Sep 17 00:00:00 2001
From: wangyugui <wangyugui@e16-tech.com>
Date: Tue, 27 Oct 2020 08:14:46 +0800
Subject: [PATCH 1/3] btrfs: add tier score to device

We use a single score value to define the tier level of a device.
Different score means different tier, and bigger is faster.
    DAX device(dax=1)
    SSD device(rotational=0)
    HDD device(rotational=1)
TODO/FIXME: Different bus(DIMM/NVMe/SAS/SATA/VirtIO/...) support.
TODO/FIXME: Different media detail(SSD MLC/TLC/..; HDD PMR/SMR/...) support.
TODO/FIXME: User-assigned property to mark as the top tier score.

In most case, only 1 or 2 tiers are used at the same time, so we group them into
top tier and other tier(s).
---
 fs/btrfs/volumes.c | 18 ++++++++++++++++++
 fs/btrfs/volumes.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 1997a7d..d767c99 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -608,6 +608,22 @@ static int btrfs_free_stale_devices(const char *path,
 	return ret;
 }
 
+/*
+ * Get the tier score to the device, higher is faster.
+ * FIXME: detech bus(DIMM/NVMe(40)/SCSI(30)/SATA(20)/..)
+ * FIXME: media detail(SSD SLC/MLC/..,)
+ * FIXME: usre-defined property to set to max score 127
+ */
+static void dev_get_tier_score(struct btrfs_device *device, struct request_queue *q)
+{
+	if (blk_queue_dax(q))
+		device->tier_score = 50;
+	else if (blk_queue_nonrot(q))
+		device->tier_score = 10;
+	else
+		device->tier_score = 0;
+}
+
 /*
  * This is only used on mount, and we are protected from competing things
  * messing with our fs_devices by the uuid_mutex, thus we do not need the
@@ -660,6 +676,7 @@ static int btrfs_open_one_device(struct btrfs_fs_devices *fs_devices,
 	}
 
 	q = bdev_get_queue(bdev);
+	dev_get_tier_score(device,q);
 	if (!blk_queue_nonrot(q))
 		fs_devices->rotating = true;
 
@@ -2598,6 +2615,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 
 	atomic64_add(device->total_bytes, &fs_info->free_chunk_space);
 
+	dev_get_tier_score(device,q);
 	if (!blk_queue_nonrot(q))
 		fs_devices->rotating = true;
 
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 302c923..5ffa429 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -138,6 +138,8 @@ struct btrfs_device {
 	struct completion kobj_unregister;
 	/* For sysfs/FSID/devinfo/devid/ */
 	struct kobject devid_kobj;
+
+	u8 tier_score; /* storage tier_score; higher is faster */
 };
 
 /*
-- 
2.29.1


[-- Attachment #3: 0003-btrfs-tier-aware-mirror-path-select.patch --]
[-- Type: application/octet-stream, Size: 2182 bytes --]

From 4ac2fc0a3be670e0960928012f5f156b50f2c69d Mon Sep 17 00:00:00 2001
From: wangyugui <wangyugui@e16-tech.com>
Date: Tue, 27 Oct 2020 09:33:21 +0800
Subject: [PATCH 3/3] btrfs: tier-aware mirror path select

This extended the patch 'btrfs: balance RAID1/RAID10 mirror selection' from louis@waffle.tech
- add the tier-aware feature
---
 fs/btrfs/volumes.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d380b20..cc4a791 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5562,6 +5562,9 @@ int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
 	return ret;
 }
 
+/* Used for round-robin balancing RAID1/RAID10 reads. */
+static atomic_t rr_counter = ATOMIC_INIT(0);
+
 static int find_live_mirror(struct btrfs_fs_info *fs_info,
 			    struct map_lookup *map, int first,
 			    int dev_replace_is_ongoing)
@@ -5572,6 +5575,11 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 	int tolerance;
 	struct btrfs_device *srcdev;
 
+	/* tier-aware */
+	int top_tier_num_stripes;
+	int top_tier_stripe_idxs[4];
+	u8 top_tier_score = 0;
+
 	ASSERT((map->type &
 		 (BTRFS_BLOCK_GROUP_RAID1_MASK | BTRFS_BLOCK_GROUP_RAID10)));
 
@@ -5580,7 +5588,29 @@ static int find_live_mirror(struct btrfs_fs_info *fs_info,
 	else
 		num_stripes = map->num_stripes;
 
-	preferred_mirror = first + current->pid % num_stripes;
+	for (i = 0; i < num_stripes; ++i)
+	{
+		if (map->stripes[i].dev->tier_score > top_tier_score)
+		{
+			top_tier_score = map->stripes[i].dev->tier_score;
+			top_tier_stripe_idxs[0] = i;
+			top_tier_num_stripes = 1;
+		}
+		else if (map->stripes[i].dev->tier_score == top_tier_score)
+		{
+			top_tier_stripe_idxs[top_tier_num_stripes] = i;
+			top_tier_num_stripes++;
+		}
+	}
+	preferred_mirror = first;
+	if (top_tier_num_stripes > 1)
+	{
+		preferred_mirror += top_tier_stripe_idxs[((unsigned)atomic_inc_return(&rr_counter)) % top_tier_num_stripes];
+	}
+	else
+	{
+		preferred_mirror += top_tier_stripe_idxs[0];
+	}
 
 	if (dev_replace_is_ongoing &&
 	    fs_info->dev_replace.cont_reading_from_srcdev_mode ==
-- 
2.29.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 1/7] block: export part_stat_read_all
  2020-10-26 23:55 ` [PATCH RFC 1/7] block: export part_stat_read_all Anand Jain
@ 2020-10-27 18:09   ` Josef Bacik
  2020-10-28  8:26     ` Anand Jain
  0 siblings, 1 reply; 17+ messages in thread
From: Josef Bacik @ 2020-10-27 18:09 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs; +Cc: linux-block

On 10/26/20 7:55 PM, Anand Jain wrote:
> For mirrored raid profiles such as raid1, raid1c3, raid1c4, and raid10,
> currently, btrfs use the PID method to determine which one of the
> mirrored devices to use to read the block. However, the PID method is
> not the best choice if the devices are heterogeneous in terms of type,
> speed, and size, as we may end up reading from the slower device.
> 
> Export the function part_stat_read_all() so that the btrfs can determine
> the device with the least average wait time to use.
> 
> Cc: linux-block@vger.kernel.org
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

You don't need this, it can be accomplished with part_stat_read, or any variety 
of the helpers in part_stat.h.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/7] block: export part_stat_read_inflight
  2020-10-26 23:55 ` [PATCH RFC 2/7] block: export part_stat_read_inflight Anand Jain
@ 2020-10-27 18:10   ` Josef Bacik
  2020-10-28  8:32     ` Anand Jain
  0 siblings, 1 reply; 17+ messages in thread
From: Josef Bacik @ 2020-10-27 18:10 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs; +Cc: linux-block

On 10/26/20 7:55 PM, Anand Jain wrote:
> The exported function part_in_flight() returns commands in-flight in the
> given block device.
> 
> Cc: linux-block@vger.kernel.org
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

This is much more internal to block and I'd rather not rely on it, I feel like 
getting the average latency is good enough.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 3/7] btrfs: add read_policy latency
  2020-10-26 23:55 ` [PATCH RFC 3/7] btrfs: add read_policy latency Anand Jain
@ 2020-10-27 18:20   ` Josef Bacik
  0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2020-10-27 18:20 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs

On 10/26/20 7:55 PM, Anand Jain wrote:
> The read policy type latency routes the read IO based on the historical
> average wait time experienced by the read IOs through the individual
> device factored by 1/10 of inflight commands in the queue. The factor
> 1/10 is because generally the block device queue depth is more than 1,
> so there can be commands in the queue even before the previous commands
> have been completed. This patch obtains the historical read IO stats from
> the kernel block layer.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   fs/btrfs/sysfs.c   |  3 +-
>   fs/btrfs/volumes.c | 74 +++++++++++++++++++++++++++++++++++++++++++++-
>   fs/btrfs/volumes.h |  1 +
>   3 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index d159f7c70bcd..6690abeeb889 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -874,7 +874,8 @@ static int btrfs_strmatch(const char *given, const char *golden)
>   	return -EINVAL;
>   }
>   
> -static const char * const btrfs_read_policy_name[] = { "pid" };
> +/* Must follow the order as in enum btrfs_read_policy */
> +static const char * const btrfs_read_policy_name[] = { "pid", "latency"};
>   
>   static ssize_t btrfs_read_policy_show(struct kobject *kobj,
>   				      struct kobj_attribute *a, char *buf)
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index da31b11ceb61..9bab6080cebf 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -14,6 +14,7 @@
>   #include <linux/semaphore.h>
>   #include <linux/uuid.h>
>   #include <linux/list_sort.h>
> +#include <linux/part_stat.h>
>   #include "misc.h"
>   #include "ctree.h"
>   #include "extent_map.h"
> @@ -5465,6 +5466,66 @@ int btrfs_is_parity_mirror(struct btrfs_fs_info *fs_info, u64 logical, u64 len)
>   	return ret;
>   }
>   
> +static u64 btrfs_estimate_read(struct btrfs_device *device,
> +			       unsigned long *inflight)
> +{
> +	u64 read_wait;
> +	u64 avg_wait = 0;
> +	unsigned long read_ios;
> +	struct disk_stats stat;
> +
> +	/* Commands in flight on this partition/device */
> +	*inflight = part_stat_read_inflight(bdev_get_queue(device->bdev),
> +					    device->bdev->bd_part);
> +	part_stat_read_all(device->bdev->bd_part, &stat);
> +
> +	read_wait = stat.nsecs[STAT_READ];
> +	read_ios = stat.ios[STAT_READ];
> +
> +	if (read_wait && read_ios && read_wait >= read_ios)
> +		avg_wait = div_u64(read_wait, read_ios);
> +	else
> +		btrfs_info_rl(device->fs_devices->fs_info,
> +			"devid: %llu avg_wait ZERO read_wait %llu read_ios %lu",
> +			      device->devid, read_wait, read_ios);
> +
> +	return avg_wait;
> +}
> +
> +static int btrfs_find_best_stripe(struct btrfs_fs_info *fs_info,
> +				  struct map_lookup *map, int first,
> +				  int num_stripe)
> +{
> +	int index;
> +	int best_stripe = 0;
> +	int est_wait = -EINVAL;
> +	int last = first + num_stripe;
> +	unsigned long inflight;
> +
> +	for (index = first; index < last; index++) {
> +		struct btrfs_device *device = map->stripes[index].dev;
> +
> +		if (!blk_queue_io_stat(bdev_get_queue(device->bdev)))
> +			return -ENOENT;
> +	}
> +
> +	for (index = first; index < last; index++) {
> +		struct btrfs_device *device = map->stripes[index].dev;
> +		u64 avg_wait;
> +		u64 final_wait;
> +
> +		avg_wait = btrfs_estimate_read(device, &inflight);
> +		final_wait = avg_wait + (avg_wait * (inflight / 10));

Inflight is going to lag because it's only going to account for bio's that 
actually have been attached to requests here.  Since we're already on fuzzy 
ground, why not just skip the inflight and go with the average latencies.  If we 
heavily load one side it's latencies will creep up and then we'll favor the 
other side.  If we want to really aim for the lowest latency, we could add our 
own inflight counter to the stripe itself, and this would account for actual 
IO's that we have inflight currently, and would be much less fuzzy than relying 
on the block inflight counters.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy
  2020-10-26 23:55 ` [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy Anand Jain
@ 2020-10-27 18:22   ` Josef Bacik
  2020-10-28  8:59     ` Anand Jain
  0 siblings, 1 reply; 17+ messages in thread
From: Josef Bacik @ 2020-10-27 18:22 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs

On 10/26/20 7:55 PM, Anand Jain wrote:
> This patch adds trace event btrfs_read_policy, which is common to all the
> read policies.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   fs/btrfs/volumes.c           | 28 ++++++++++++++++++++++++++--
>   include/trace/events/btrfs.h | 20 ++++++++++++++++++++
>   2 files changed, 46 insertions(+), 2 deletions(-)

Noooooo this isn't the way we do this.  Simply make a trace class with all the 
variations, and then add individual trace events that inherit from the class 
that print out the appropriate values.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 1/7] block: export part_stat_read_all
  2020-10-27 18:09   ` Josef Bacik
@ 2020-10-28  8:26     ` Anand Jain
  0 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2020-10-28  8:26 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs; +Cc: linux-block

On 28/10/20 2:09 am, Josef Bacik wrote:
> On 10/26/20 7:55 PM, Anand Jain wrote:
>> For mirrored raid profiles such as raid1, raid1c3, raid1c4, and raid10,
>> currently, btrfs use the PID method to determine which one of the
>> mirrored devices to use to read the block. However, the PID method is
>> not the best choice if the devices are heterogeneous in terms of type,
>> speed, and size, as we may end up reading from the slower device.
>>
>> Export the function part_stat_read_all() so that the btrfs can determine
>> the device with the least average wait time to use.
>>
>> Cc: linux-block@vger.kernel.org
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> You don't need this, it can be accomplished with part_stat_read, or any 
> variety of the helpers in part_stat.h.  Thanks,

Oh. I have missed #define part_stat_read. It works for now.
We don't have to export part_stat_read_all() as in this patch.

Sorry for the noise.

Thanks, Anand

> 
> Josef


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 2/7] block: export part_stat_read_inflight
  2020-10-27 18:10   ` Josef Bacik
@ 2020-10-28  8:32     ` Anand Jain
  0 siblings, 0 replies; 17+ messages in thread
From: Anand Jain @ 2020-10-28  8:32 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs; +Cc: linux-block

On 28/10/20 2:10 am, Josef Bacik wrote:
> On 10/26/20 7:55 PM, Anand Jain wrote:
>> The exported function part_in_flight() returns commands in-flight in the
>> given block device.
>>
>> Cc: linux-block@vger.kernel.org
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> This is much more internal to block and I'd rather not rely on it, I 
> feel like getting the average latency is good enough.  Thanks,
> 

And also, as mentioned in the cover letter, it is hard to know the 
relation between the number of inflight commands and its effect on avg 
latency.

So ok, we don't need this.

Thanks, Anand


> Josef


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy
  2020-10-27 18:22   ` Josef Bacik
@ 2020-10-28  8:59     ` Anand Jain
  2020-10-28 12:41       ` Josef Bacik
  0 siblings, 1 reply; 17+ messages in thread
From: Anand Jain @ 2020-10-28  8:59 UTC (permalink / raw)
  To: Josef Bacik, linux-btrfs

On 28/10/20 2:22 am, Josef Bacik wrote:
> On 10/26/20 7:55 PM, Anand Jain wrote:
>> This patch adds trace event btrfs_read_policy, which is common to all the
>> read policies.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/volumes.c           | 28 ++++++++++++++++++++++++++--
>>   include/trace/events/btrfs.h | 20 ++++++++++++++++++++
>>   2 files changed, 46 insertions(+), 2 deletions(-)
> 
> Noooooo this isn't the way we do this.  Simply make a trace class with 
> all the variations, and then add individual trace events that inherit 
> from the class that print out the appropriate values.  Thanks,
> 

I added this mainly to debug the inflight thing. I was kind of not so
successful in finding a relation between latency, queue depth, and
inflight. As it is gone now, I doubt if we still need tracing. I am ok
to drop this. Or we could add when we start digging anything around
this?

Thanks, Anand


> Josef


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy
  2020-10-28  8:59     ` Anand Jain
@ 2020-10-28 12:41       ` Josef Bacik
  0 siblings, 0 replies; 17+ messages in thread
From: Josef Bacik @ 2020-10-28 12:41 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs

On 10/28/20 4:59 AM, Anand Jain wrote:
> On 28/10/20 2:22 am, Josef Bacik wrote:
>> On 10/26/20 7:55 PM, Anand Jain wrote:
>>> This patch adds trace event btrfs_read_policy, which is common to all the
>>> read policies.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   fs/btrfs/volumes.c           | 28 ++++++++++++++++++++++++++--
>>>   include/trace/events/btrfs.h | 20 ++++++++++++++++++++
>>>   2 files changed, 46 insertions(+), 2 deletions(-)
>>
>> Noooooo this isn't the way we do this.  Simply make a trace class with all the 
>> variations, and then add individual trace events that inherit from the class 
>> that print out the appropriate values.  Thanks,
>>
> 
> I added this mainly to debug the inflight thing. I was kind of not so
> successful in finding a relation between latency, queue depth, and
> inflight. As it is gone now, I doubt if we still need tracing. I am ok
> to drop this. Or we could add when we start digging anything around
> this?

If you were just using it for debugging then I say we just drop it and add 
something later if we need it.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-10-28 22:29 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 23:55 [PATCH RFC 0/7] btrfs: read_policy types latency, device and Anand Jain
2020-10-26 23:55 ` [PATCH RFC 1/7] block: export part_stat_read_all Anand Jain
2020-10-27 18:09   ` Josef Bacik
2020-10-28  8:26     ` Anand Jain
2020-10-26 23:55 ` [PATCH RFC 2/7] block: export part_stat_read_inflight Anand Jain
2020-10-27 18:10   ` Josef Bacik
2020-10-28  8:32     ` Anand Jain
2020-10-26 23:55 ` [PATCH RFC 3/7] btrfs: add read_policy latency Anand Jain
2020-10-27 18:20   ` Josef Bacik
2020-10-26 23:55 ` [PATCH RFC 4/7] btrfs: trace, add event btrfs_read_policy Anand Jain
2020-10-27 18:22   ` Josef Bacik
2020-10-28  8:59     ` Anand Jain
2020-10-28 12:41       ` Josef Bacik
2020-10-26 23:55 ` [PATCH 5/7] btrfs: introduce new device-state read_preferred Anand Jain
2020-10-26 23:55 ` [PATCH 6/7] btrfs: introduce new read_policy device Anand Jain
2020-10-27  7:11   ` Wang Yugui
2020-10-26 23:55 ` [PATCH RFC 7/7] btrfs: introduce new read_policy round-robin Anand Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.