[PATCH RFC 1/7] block: export part_stat_read_all

[PATCH RFC 1/7] block: export part_stat_read_all

[Anand Jain]

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

[PATCH RFC 2/7] block: export part_stat_read_inflight

[Anand Jain]

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

Re: [PATCH RFC 1/7] block: export part_stat_read_all

[Josef Bacik]

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

Re: [PATCH RFC 2/7] block: export part_stat_read_inflight

[Josef Bacik]

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

Re: [PATCH RFC 1/7] block: export part_stat_read_all

[Anand Jain]

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

Re: [PATCH RFC 2/7] block: export part_stat_read_inflight

[Anand Jain]

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