All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V5 0/3] block: add two statistic tables
@ 2021-04-08 13:58 Md Haris Iqbal
  2021-04-08 13:58 ` [PATCH V5 1/3] block: add io_extra_stats node Md Haris Iqbal
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Md Haris Iqbal @ 2021-04-08 13:58 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, jinpu.wang, danil.kipnis, Md Haris Iqbal

Hi Jens,

This version is rebased against the latest for-next.

Thanks,
Haris

PATCH V4: https://lore.kernel.org/linux-block/20210203151019.27036-1-guoqing.jiang@cloud.ionos.com/
* Adds Reviewed-by tag from Johannes.

PATCH V3: https://lore.kernel.org/linux-block/7f78132a-affc-eb03-735a-4da43e143b6e@cloud.ionos.com/T/#t
* reorgnize the patchset per Johannes's suggestion.

PATCH V2: https://lore.kernel.org/linux-block/20210201012727.28305-1-guoqing.jiang@cloud.ionos.com/T/#t
*. remove BLK_ADDITIONAL_DISKSTAT option per Christoph's comment.
*. move blk_queue_io_extra_stat into blk_additional_{latency,sector}
   per Christoph's comment.
*. simplify blk_additional_latency by pass duration time directly.

PATCH V1: https://marc.info/?l=linux-block&m=161176000024443&w=2
* add Jack's reviewed-by.

RFC V4: https://marc.info/?l=linux-block&m=161027198729158&w=2
* rebase with latest code.

RFC V3: https://marc.info/?l=linux-block&m=159730633416534&w=2
* Move the #ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT into the function body
  per Johannes's comment.
* Tweak the output of two tables to make they are more intuitive

RFC V2: https://marc.info/?l=linux-block&m=159467483514062&w=2
* don't call ktime_get_ns and drop unnecessary patches.
* add io_extra_stats to avoid potential overhead.

RFC V1: https://marc.info/?l=linux-block&m=159419516730386&w=2

Guoqing Jiang (3):
  block: add io_extra_stats node
  block: add a statistic table for io latency
  block: add a statistic table for io sector

 Documentation/ABI/testing/sysfs-block | 26 ++++++++++
 Documentation/block/queue-sysfs.rst   |  5 ++
 block/blk-core.c                      | 43 ++++++++++++++++
 block/blk-sysfs.c                     |  3 ++
 block/genhd.c                         | 74 +++++++++++++++++++++++++++
 include/linux/blkdev.h                |  2 +
 include/linux/part_stat.h             |  6 +++
 7 files changed, 159 insertions(+)

-- 
2.25.1


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

* [PATCH V5 1/3] block: add io_extra_stats node
  2021-04-08 13:58 [PATCH V5 0/3] block: add two statistic tables Md Haris Iqbal
@ 2021-04-08 13:58 ` Md Haris Iqbal
  2021-04-08 13:58 ` [PATCH V5 2/3] block: add a statistic table for io latency Md Haris Iqbal
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Md Haris Iqbal @ 2021-04-08 13:58 UTC (permalink / raw)
  To: axboe
  Cc: linux-block, jinpu.wang, danil.kipnis, Guoqing Jiang,
	Johannes Thumshirn, Guoqing Jiang, Md Haris Iqbal

From: Guoqing Jiang <guoqing.jiang@ionos.com>

We will track the size and latency of each io, which could make people
suffer from the additional overhead if they don't need the statistics.
So introduce a specific sysfs node to enable/disable the tracking.

Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Cc: Guoqing Jiang <jgq516@gmail.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
 Documentation/ABI/testing/sysfs-block | 9 +++++++++
 Documentation/block/queue-sysfs.rst   | 5 +++++
 block/blk-sysfs.c                     | 3 +++
 include/linux/blkdev.h                | 2 ++
 4 files changed, 19 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index e34cdeeeb9d4..aee7464b35da 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -316,3 +316,12 @@ Description:
 		does not complete in this time then the block driver timeout
 		handler is invoked. That timeout handler can decide to retry
 		the request, to fail it or to start a device recovery strategy.
+
+What:		/sys/block/<disk>/queue/io_extra_stats
+Date:		March 2021
+Contact:	Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
+Description:
+		Indicates if people want to know the extra statistics (I/O
+		size and I/O latency) from /sys/block/<disk>/io_latency
+		and /sys/block/<disk>/io_size. The value is 0 by default,
+		set if the extra statistics are needed.
diff --git a/Documentation/block/queue-sysfs.rst b/Documentation/block/queue-sysfs.rst
index 4dc7f0d499a8..5b24c552e3f6 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -99,6 +99,11 @@ iostats (RW)
 This file is used to control (on/off) the iostats accounting of the
 disk.
 
+io_extra_stats (RW)
+-------------------
+This file is used to control (on/off) the additional accounting of the
+io size and io latency of disk.
+
 logical_block_size (RO)
 -----------------------
 This is the logical block size of the device, in bytes.
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index e03bedf180ab..848ed6449eca 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -298,6 +298,7 @@ queue_##name##_store(struct request_queue *q, const char *page, size_t count) \
 QUEUE_SYSFS_BIT_FNS(nonrot, NONROT, 1);
 QUEUE_SYSFS_BIT_FNS(random, ADD_RANDOM, 0);
 QUEUE_SYSFS_BIT_FNS(iostats, IO_STAT, 0);
+QUEUE_SYSFS_BIT_FNS(io_extra_stats, IO_EXTRA_STAT, 0);
 QUEUE_SYSFS_BIT_FNS(stable_writes, STABLE_WRITES, 0);
 #undef QUEUE_SYSFS_BIT_FNS
 
@@ -629,6 +630,7 @@ static struct queue_sysfs_entry queue_hw_sector_size_entry = {
 
 QUEUE_RW_ENTRY(queue_nonrot, "rotational");
 QUEUE_RW_ENTRY(queue_iostats, "iostats");
+QUEUE_RW_ENTRY(queue_io_extra_stats, "io_extra_stats");
 QUEUE_RW_ENTRY(queue_random, "add_random");
 QUEUE_RW_ENTRY(queue_stable_writes, "stable_writes");
 
@@ -664,6 +666,7 @@ static struct attribute *queue_attrs[] = {
 	&queue_nomerges_entry.attr,
 	&queue_rq_affinity_entry.attr,
 	&queue_iostats_entry.attr,
+	&queue_io_extra_stats_entry.attr,
 	&queue_stable_writes_entry.attr,
 	&queue_random_entry.attr,
 	&queue_poll_entry.attr,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 43c4a2d04ea2..b95279494cfa 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -621,6 +621,7 @@ struct request_queue {
 #define QUEUE_FLAG_RQ_ALLOC_TIME 27	/* record rq->alloc_time_ns */
 #define QUEUE_FLAG_HCTX_ACTIVE	28	/* at least one blk-mq hctx is active */
 #define QUEUE_FLAG_NOWAIT       29	/* device supports NOWAIT */
+#define QUEUE_FLAG_IO_EXTRA_STAT 30	/* extra IO accounting for size and latency */
 
 #define QUEUE_FLAG_MQ_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_SAME_COMP) |		\
@@ -641,6 +642,7 @@ bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q);
 #define blk_queue_stable_writes(q) \
 	test_bit(QUEUE_FLAG_STABLE_WRITES, &(q)->queue_flags)
 #define blk_queue_io_stat(q)	test_bit(QUEUE_FLAG_IO_STAT, &(q)->queue_flags)
+#define blk_queue_io_extra_stat(q) test_bit(QUEUE_FLAG_IO_EXTRA_STAT, &(q)->queue_flags)
 #define blk_queue_add_random(q)	test_bit(QUEUE_FLAG_ADD_RANDOM, &(q)->queue_flags)
 #define blk_queue_discard(q)	test_bit(QUEUE_FLAG_DISCARD, &(q)->queue_flags)
 #define blk_queue_zone_resetall(q)	\
-- 
2.25.1


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

* [PATCH V5 2/3] block: add a statistic table for io latency
  2021-04-08 13:58 [PATCH V5 0/3] block: add two statistic tables Md Haris Iqbal
  2021-04-08 13:58 ` [PATCH V5 1/3] block: add io_extra_stats node Md Haris Iqbal
@ 2021-04-08 13:58 ` Md Haris Iqbal
  2021-04-08 16:52   ` Christoph Hellwig
  2021-04-08 13:58 ` [PATCH V5 3/3] block: add a statistic table for io sector Md Haris Iqbal
  2021-04-08 22:22 ` [PATCH V5 0/3] block: add two statistic tables Khazhy Kumykov
  3 siblings, 1 reply; 7+ messages in thread
From: Md Haris Iqbal @ 2021-04-08 13:58 UTC (permalink / raw)
  To: axboe
  Cc: linux-block, jinpu.wang, danil.kipnis, Guoqing Jiang,
	Johannes Thumshirn, Guoqing Jiang, Md Haris Iqbal

From: Guoqing Jiang <guoqing.jiang@ionos.com>

Usually, we get the status of block device by cat stat file, but we can
only know the total time with that file. And we would like to know more
accurate statistic, such as each latency range, which helps people to
diagnose if there is issue about the hardware.

This change is based on our internal patch from Florian-Ewald Mueller
(florian-ewald.mueller@ionos.com).

Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Cc: Guoqing Jiang <jgq516@gmail.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
 Documentation/ABI/testing/sysfs-block |  8 ++++++
 block/blk-core.c                      | 24 +++++++++++++++++
 block/genhd.c                         | 37 +++++++++++++++++++++++++++
 include/linux/part_stat.h             |  5 ++++
 4 files changed, 74 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index aee7464b35da..a027e5afc35a 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -27,6 +27,14 @@ Description:
 
 		For more details refer Documentation/admin-guide/iostats.rst
 
+What:		/sys/block/<disk>/io_latency
+Date:		March 2021
+Contact:	Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
+Description:
+		The /sys/block/<disk>/io_latency files displays the I/O
+		latency of disk <disk>. With it, it is convenient to know
+		the statistics of I/O latency for each type (read, write,
+		discard and flush) which have happened to the disk.
 
 What:		/sys/block/<disk>/<part>/stat
 Date:		February 2008
diff --git a/block/blk-core.c b/block/blk-core.c
index 9bcdae93f6d4..0895d5eddc1f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1263,6 +1263,26 @@ static void update_io_ticks(struct block_device *part, unsigned long now,
 	}
 }
 
+static void blk_additional_latency(struct block_device *part, const int sgrp,
+				   struct request_queue *q,
+				   unsigned long duration)
+{
+	unsigned int idx;
+
+	if (!blk_queue_io_extra_stat(q))
+		return;
+
+	duration /= NSEC_PER_MSEC;
+	duration /= HZ_TO_MSEC_NUM;
+	if (likely(duration > 0)) {
+		idx = ilog2(duration);
+		if (idx > ADD_STAT_NUM - 1)
+			idx = ADD_STAT_NUM - 1;
+	} else
+		idx = 0;
+	part_stat_inc(part, latency_table[idx][sgrp]);
+}
+
 static void blk_account_io_completion(struct request *req, unsigned int bytes)
 {
 	if (req->part && blk_do_io_stat(req)) {
@@ -1287,6 +1307,8 @@ void blk_account_io_done(struct request *req, u64 now)
 
 		part_stat_lock();
 		update_io_ticks(req->part, jiffies, true);
+		blk_additional_latency(req->part, sgrp, req->q,
+				       now - req->start_time_ns);
 		part_stat_inc(req->part, ios[sgrp]);
 		part_stat_add(req->part, nsecs[sgrp], now - req->start_time_ns);
 		part_stat_unlock();
@@ -1353,6 +1375,8 @@ static void __part_end_io_acct(struct block_device *part, unsigned int op,
 
 	part_stat_lock();
 	update_io_ticks(part, now, true);
+	blk_additional_latency(part, sgrp, part->bd_disk->queue,
+			       jiffies_to_nsecs(duration));
 	part_stat_add(part, nsecs[sgrp], jiffies_to_nsecs(duration));
 	part_stat_local_dec(part, in_flight[op_is_write(op)]);
 	part_stat_unlock();
diff --git a/block/genhd.c b/block/genhd.c
index 8c8f543572e6..e054dc5ac9c4 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1144,6 +1144,42 @@ static struct device_attribute dev_attr_fail_timeout =
 	__ATTR(io-timeout-fail, 0644, part_timeout_show, part_timeout_store);
 #endif
 
+static ssize_t io_latency_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct block_device *bdev = dev_to_bdev(dev);
+	size_t count = 0;
+	int i, sgrp;
+
+	for (i = 0; i < ADD_STAT_NUM; i++) {
+		unsigned int from, to;
+
+		if (i == ADD_STAT_NUM - 1) {
+			count += scnprintf(buf + count, PAGE_SIZE - count, "      >= %5d  ms: ",
+					   (2 << (i - 2)) * HZ_TO_MSEC_NUM);
+		} else {
+			if (i < 2) {
+				from = i;
+				to = i + 1;
+			} else {
+				from = 2 << (i - 2);
+				to = 2 << (i - 1);
+			}
+			count += scnprintf(buf + count, PAGE_SIZE - count, "[%5d - %-5d) ms: ",
+					   from * HZ_TO_MSEC_NUM, to * HZ_TO_MSEC_NUM);
+		}
+
+		for (sgrp = 0; sgrp < NR_STAT_GROUPS; sgrp++)
+			count += scnprintf(buf + count, PAGE_SIZE - count, "%lu ",
+					   part_stat_read(bdev, latency_table[i][sgrp]));
+		count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
+	}
+
+	return count;
+}
+
+static struct device_attribute dev_attr_io_latency =
+	__ATTR(io_latency, 0444, io_latency_show, NULL);
+
 static struct attribute *disk_attrs[] = {
 	&dev_attr_range.attr,
 	&dev_attr_ext_range.attr,
@@ -1163,6 +1199,7 @@ static struct attribute *disk_attrs[] = {
 #ifdef CONFIG_FAIL_IO_TIMEOUT
 	&dev_attr_fail_timeout.attr,
 #endif
+	&dev_attr_io_latency.attr,
 	NULL
 };
 
diff --git a/include/linux/part_stat.h b/include/linux/part_stat.h
index d2558121d48c..e2bde5160de4 100644
--- a/include/linux/part_stat.h
+++ b/include/linux/part_stat.h
@@ -9,6 +9,11 @@ struct disk_stats {
 	unsigned long sectors[NR_STAT_GROUPS];
 	unsigned long ios[NR_STAT_GROUPS];
 	unsigned long merges[NR_STAT_GROUPS];
+	/*
+	 * We measure latency (ms) for 1, 2, ..., 1024 and >=1024.
+	 */
+#define ADD_STAT_NUM	12
+	unsigned long latency_table[ADD_STAT_NUM][NR_STAT_GROUPS];
 	unsigned long io_ticks;
 	local_t in_flight[2];
 };
-- 
2.25.1


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

* [PATCH V5 3/3] block: add a statistic table for io sector
  2021-04-08 13:58 [PATCH V5 0/3] block: add two statistic tables Md Haris Iqbal
  2021-04-08 13:58 ` [PATCH V5 1/3] block: add io_extra_stats node Md Haris Iqbal
  2021-04-08 13:58 ` [PATCH V5 2/3] block: add a statistic table for io latency Md Haris Iqbal
@ 2021-04-08 13:58 ` Md Haris Iqbal
  2021-04-08 22:22 ` [PATCH V5 0/3] block: add two statistic tables Khazhy Kumykov
  3 siblings, 0 replies; 7+ messages in thread
From: Md Haris Iqbal @ 2021-04-08 13:58 UTC (permalink / raw)
  To: axboe
  Cc: linux-block, jinpu.wang, danil.kipnis, Guoqing Jiang,
	Johannes Thumshirn, Guoqing Jiang, Md Haris Iqbal

From: Guoqing Jiang <guoqing.jiang@ionos.com>

With the sector table, so we can know the distribution of different IO
size from upper layer, which means we could have the opportunity to tune
the performance based on the mostly issued IOs.

This change is based on our internal patch from Florian-Ewald Mueller
(florian-ewald.mueller@ionos.com).

Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Guoqing Jiang <guoqing.jiang@ionos.com>
Cc: Guoqing Jiang <jgq516@gmail.com>
Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com>
---
 Documentation/ABI/testing/sysfs-block |  9 +++++++
 block/blk-core.c                      | 19 ++++++++++++++
 block/genhd.c                         | 37 +++++++++++++++++++++++++++
 include/linux/part_stat.h             |  3 ++-
 4 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index a027e5afc35a..78f84cd44bf0 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -36,6 +36,15 @@ Description:
 		the statistics of I/O latency for each type (read, write,
 		discard and flush) which have happened to the disk.
 
+What:		/sys/block/<disk>/io_size
+Date:		March 2021
+Contact:	Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
+Description:
+		The /sys/block/<disk>/io_size files displays the I/O
+		size of disk <disk>. With it, it is convenient to know
+		the statistics of I/O size for each type (read, write,
+		discard and flush) which have happened to the disk.
+
 What:		/sys/block/<disk>/<part>/stat
 Date:		February 2008
 Contact:	Jerome Marchand <jmarchan@redhat.com>
diff --git a/block/blk-core.c b/block/blk-core.c
index 0895d5eddc1f..899b0b08f92d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1283,12 +1283,30 @@ static void blk_additional_latency(struct block_device *part, const int sgrp,
 	part_stat_inc(part, latency_table[idx][sgrp]);
 }
 
+static void blk_additional_sector(struct block_device *part, const int sgrp,
+				  struct request_queue *q, unsigned int sectors)
+{
+	unsigned int idx;
+
+	if (!blk_queue_io_extra_stat(q))
+		return;
+
+	if (sectors == 1)
+		idx = 0;
+	else
+		idx = ilog2(sectors);
+
+	idx = (idx > (ADD_STAT_NUM - 1)) ? (ADD_STAT_NUM - 1) : idx;
+	part_stat_inc(part, size_table[idx][sgrp]);
+}
+
 static void blk_account_io_completion(struct request *req, unsigned int bytes)
 {
 	if (req->part && blk_do_io_stat(req)) {
 		const int sgrp = op_stat_group(req_op(req));
 
 		part_stat_lock();
+		blk_additional_sector(req->part, sgrp, req->q, bytes >> SECTOR_SHIFT);
 		part_stat_add(req->part, sectors[sgrp], bytes >> 9);
 		part_stat_unlock();
 	}
@@ -1341,6 +1359,7 @@ static unsigned long __part_start_io_acct(struct block_device *part,
 	update_io_ticks(part, now, false);
 	part_stat_inc(part, ios[sgrp]);
 	part_stat_add(part, sectors[sgrp], sectors);
+	blk_additional_sector(part, sgrp, part->bd_disk->queue, sectors);
 	part_stat_local_inc(part, in_flight[op_is_write(op)]);
 	part_stat_unlock();
 
diff --git a/block/genhd.c b/block/genhd.c
index e054dc5ac9c4..94a692785fed 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1180,6 +1180,42 @@ static ssize_t io_latency_show(struct device *dev, struct device_attribute *attr
 static struct device_attribute dev_attr_io_latency =
 	__ATTR(io_latency, 0444, io_latency_show, NULL);
 
+static ssize_t io_size_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct block_device *bdev = dev_to_bdev(dev);
+	size_t count = 0;
+	int i, sgrp;
+
+	for (i = 0; i < ADD_STAT_NUM; i++) {
+		unsigned int from, to;
+
+		if (i == ADD_STAT_NUM - 1) {
+			from = 2 << (i - 2);
+			count += scnprintf(buf + count, PAGE_SIZE - count,
+					   "      >=%5d   KB: ", from);
+		} else {
+			if (i < 2) {
+				from = i;
+				to = i + 1;
+			} else {
+				from = 2 << (i - 2);
+				to = 2 << (i - 1);
+			}
+			count += scnprintf(buf + count, PAGE_SIZE - count,
+					   "[%5d - %-5d) KB: ", from, to);
+		}
+		for (sgrp = 0; sgrp < NR_STAT_GROUPS; sgrp++)
+			count += scnprintf(buf + count, PAGE_SIZE - count, "%lu ",
+					   part_stat_read(bdev, size_table[i][sgrp]));
+		count += scnprintf(buf + count, PAGE_SIZE - count, "\n");
+	}
+
+	return count;
+}
+
+static struct device_attribute dev_attr_io_size =
+	__ATTR(io_size, 0444, io_size_show, NULL);
+
 static struct attribute *disk_attrs[] = {
 	&dev_attr_range.attr,
 	&dev_attr_ext_range.attr,
@@ -1200,6 +1236,7 @@ static struct attribute *disk_attrs[] = {
 	&dev_attr_fail_timeout.attr,
 #endif
 	&dev_attr_io_latency.attr,
+	&dev_attr_io_size.attr,
 	NULL
 };
 
diff --git a/include/linux/part_stat.h b/include/linux/part_stat.h
index e2bde5160de4..221fb3a884b2 100644
--- a/include/linux/part_stat.h
+++ b/include/linux/part_stat.h
@@ -10,10 +10,11 @@ struct disk_stats {
 	unsigned long ios[NR_STAT_GROUPS];
 	unsigned long merges[NR_STAT_GROUPS];
 	/*
-	 * We measure latency (ms) for 1, 2, ..., 1024 and >=1024.
+	 * We measure latency (ms) and size (KB) for 1, 2, ..., 1024 and >=1024.
 	 */
 #define ADD_STAT_NUM	12
 	unsigned long latency_table[ADD_STAT_NUM][NR_STAT_GROUPS];
+	unsigned long size_table[ADD_STAT_NUM][NR_STAT_GROUPS];
 	unsigned long io_ticks;
 	local_t in_flight[2];
 };
-- 
2.25.1


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

* Re: [PATCH V5 2/3] block: add a statistic table for io latency
  2021-04-08 13:58 ` [PATCH V5 2/3] block: add a statistic table for io latency Md Haris Iqbal
@ 2021-04-08 16:52   ` Christoph Hellwig
  2021-04-08 17:06     ` Haris Iqbal
  0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2021-04-08 16:52 UTC (permalink / raw)
  To: Md Haris Iqbal
  Cc: axboe, linux-block, jinpu.wang, danil.kipnis, Guoqing Jiang,
	Johannes Thumshirn, Guoqing Jiang

> +static ssize_t io_latency_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct block_device *bdev = dev_to_bdev(dev);
> +	size_t count = 0;
> +	int i, sgrp;
> +
> +	for (i = 0; i < ADD_STAT_NUM; i++) {
> +		unsigned int from, to;
> +
> +		if (i == ADD_STAT_NUM - 1) {
> +			count += scnprintf(buf + count, PAGE_SIZE - count, "      >= %5d  ms: ",

Please fix your overly long lines all over this code.


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

* Re: [PATCH V5 2/3] block: add a statistic table for io latency
  2021-04-08 16:52   ` Christoph Hellwig
@ 2021-04-08 17:06     ` Haris Iqbal
  0 siblings, 0 replies; 7+ messages in thread
From: Haris Iqbal @ 2021-04-08 17:06 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, linux-block, Jinpu Wang, Danil Kipnis, Guoqing Jiang,
	Johannes Thumshirn, Guoqing Jiang

On Thu, Apr 8, 2021 at 6:52 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> > +static ssize_t io_latency_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +     struct block_device *bdev = dev_to_bdev(dev);
> > +     size_t count = 0;
> > +     int i, sgrp;
> > +
> > +     for (i = 0; i < ADD_STAT_NUM; i++) {
> > +             unsigned int from, to;
> > +
> > +             if (i == ADD_STAT_NUM - 1) {
> > +                     count += scnprintf(buf + count, PAGE_SIZE - count, "      >= %5d  ms: ",
>
> Please fix your overly long lines all over this code.

Sure, will do.

>

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

* Re: [PATCH V5 0/3] block: add two statistic tables
  2021-04-08 13:58 [PATCH V5 0/3] block: add two statistic tables Md Haris Iqbal
                   ` (2 preceding siblings ...)
  2021-04-08 13:58 ` [PATCH V5 3/3] block: add a statistic table for io sector Md Haris Iqbal
@ 2021-04-08 22:22 ` Khazhy Kumykov
  3 siblings, 0 replies; 7+ messages in thread
From: Khazhy Kumykov @ 2021-04-08 22:22 UTC (permalink / raw)
  To: Md Haris Iqbal; +Cc: Jens Axboe, linux-block, jinpu.wang, danil.kipnis

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

On Thu, Apr 8, 2021 at 6:59 AM Md Haris Iqbal <haris.iqbal@ionos.com> wrote:
>
> Hi Jens,
>
> This version is rebased against the latest for-next.
>
> Thanks,
> Haris
>
> PATCH V4: https://lore.kernel.org/linux-block/20210203151019.27036-1-guoqing.jiang@cloud.ionos.com/
> * Adds Reviewed-by tag from Johannes.
>
> PATCH V3: https://lore.kernel.org/linux-block/7f78132a-affc-eb03-735a-4da43e143b6e@cloud.ionos.com/T/#t
> * reorgnize the patchset per Johannes's suggestion.
>
> PATCH V2: https://lore.kernel.org/linux-block/20210201012727.28305-1-guoqing.jiang@cloud.ionos.com/T/#t
> *. remove BLK_ADDITIONAL_DISKSTAT option per Christoph's comment.
> *. move blk_queue_io_extra_stat into blk_additional_{latency,sector}
>    per Christoph's comment.
> *. simplify blk_additional_latency by pass duration time directly.
>
> PATCH V1: https://marc.info/?l=linux-block&m=161176000024443&w=2
> * add Jack's reviewed-by.
>
> RFC V4: https://marc.info/?l=linux-block&m=161027198729158&w=2
> * rebase with latest code.
>
> RFC V3: https://marc.info/?l=linux-block&m=159730633416534&w=2
> * Move the #ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT into the function body
>   per Johannes's comment.
> * Tweak the output of two tables to make they are more intuitive
>
> RFC V2: https://marc.info/?l=linux-block&m=159467483514062&w=2
> * don't call ktime_get_ns and drop unnecessary patches.
> * add io_extra_stats to avoid potential overhead.
>
> RFC V1: https://marc.info/?l=linux-block&m=159419516730386&w=2
>
> Guoqing Jiang (3):
>   block: add io_extra_stats node
>   block: add a statistic table for io latency
>   block: add a statistic table for io sector

We've also found it useful to have the kernel export latency
statistics. In particular we've been using a 2d-histogram exporting
latency per iosize (there was a proposal a loong time ago that seemed
to fizzle out https://lwn.net/Articles/383515/)

Do you think having a format like that would be useful?

# cat /sys/block/sda/read_request_histo
rows = bytes columns = ms
        10      20      50      100     200     500     1000    2000
 5000    10000   20000
   2048 33      0       0       0       0       0       0       0
 0       0       0
   4096 51798   1114    1271    11      0       0       0       0
 0       0       0
   8192 8803    81      109     1       0       0       0       0
 0       0       0
  16384 10321   116     150     5       0       0       0       0
 0       0       0
  32768 8341    166     215     6       0       0       0       0
 0       0       0
  65536 7550    248     389     2       0       0       0       0
 0       0       0
 131072 40440   923     1317    10      0       0       0       0
 0       0       0
 262144 357     1       0       0       0       0       0       0
 0       0       0
 524288 13      1       0       0       0       0       0       0
 0       0       0
1048576 7       2       0       0       0       0       0       0
 0       0       0

Khazhy

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3996 bytes --]

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

end of thread, other threads:[~2021-04-08 22:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-08 13:58 [PATCH V5 0/3] block: add two statistic tables Md Haris Iqbal
2021-04-08 13:58 ` [PATCH V5 1/3] block: add io_extra_stats node Md Haris Iqbal
2021-04-08 13:58 ` [PATCH V5 2/3] block: add a statistic table for io latency Md Haris Iqbal
2021-04-08 16:52   ` Christoph Hellwig
2021-04-08 17:06     ` Haris Iqbal
2021-04-08 13:58 ` [PATCH V5 3/3] block: add a statistic table for io sector Md Haris Iqbal
2021-04-08 22:22 ` [PATCH V5 0/3] block: add two statistic tables Khazhy Kumykov

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.