All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC V4 0/4] block: add two statistic tables
@ 2021-01-10  9:44 Guoqing Jiang
  2021-01-10  9:44 ` [PATCH RFC V4 1/4] block: add a statistic table for io latency Guoqing Jiang
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Guoqing Jiang @ 2021-01-10  9:44 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, danil.kipnis, jinpu.wang, Guoqing Jiang

Hi,

No more comments since the last version, so this version is just rebase
with latest tree.

To make the tables work, it is necessary to enable BLK_ADDITIONAL_DISKSTAT
option, and also enable the sysfs node.
# echo 1 > /sys/block/md0/queue/io_extra_stats

After that, the size and latency of io are recorded in each table.

Thanks,
Guoqing

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 (4):
  block: add a statistic table for io latency
  block: add a statistic table for io sector
  block: add io_extra_stats node
  block: call blk_additional_{latency,sector} only when io_extra_stats
    is true

 Documentation/ABI/testing/sysfs-block | 26 +++++++++
 Documentation/block/queue-sysfs.rst   |  6 +++
 block/Kconfig                         |  9 ++++
 block/blk-core.c                      | 51 ++++++++++++++++++
 block/blk-sysfs.c                     | 10 ++++
 block/genhd.c                         | 78 +++++++++++++++++++++++++++
 include/linux/blkdev.h                |  6 +++
 include/linux/part_stat.h             |  8 +++
 8 files changed, 194 insertions(+)

-- 
2.17.1


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

* [PATCH RFC V4 1/4] block: add a statistic table for io latency
  2021-01-10  9:44 [PATCH RFC V4 0/4] block: add two statistic tables Guoqing Jiang
@ 2021-01-10  9:44 ` Guoqing Jiang
  2021-01-10  9:44 ` [PATCH RFC V4 2/4] block: add a statistic table for io sector Guoqing Jiang
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guoqing Jiang @ 2021-01-10  9:44 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, danil.kipnis, jinpu.wang, Guoqing Jiang

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.

Also a new config option is introduced to control if people want to know
the additional statistics or not, and we use the option for io sector in
next patch.

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

Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 Documentation/ABI/testing/sysfs-block |  8 ++++++
 block/Kconfig                         |  8 ++++++
 block/blk-core.c                      | 29 +++++++++++++++++++
 block/genhd.c                         | 41 +++++++++++++++++++++++++++
 include/linux/part_stat.h             |  7 +++++
 5 files changed, 93 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index e34cdeeeb9d4..4371a0f2cb5e 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:		January 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/Kconfig b/block/Kconfig
index a2297edfdde8..ca3854a7e3db 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -174,6 +174,14 @@ config BLK_DEBUG_FS
 	Unless you are building a kernel for a tiny system, you should
 	say Y here.
 
+config BLK_ADDITIONAL_DISKSTAT
+	bool "Block layer additional diskstat"
+	default n
+	help
+	Enabling this option adds io latency statistics for each block device.
+
+	If unsure, say N.
+
 config BLK_DEBUG_FS_ZONED
        bool
        default BLK_DEBUG_FS && BLK_DEV_ZONED
diff --git a/block/blk-core.c b/block/blk-core.c
index 96e5fcd7f071..18cf881d8194 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1277,6 +1277,33 @@ static void update_io_ticks(struct block_device *part, unsigned long now,
 	}
 }
 
+/*
+ * Either account additional stat for request if req is not NULL or account for bio.
+ */
+static void blk_additional_latency(struct block_device *part, const int sgrp,
+				   struct request *req, unsigned long start_jiffies)
+{
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+	unsigned int idx;
+	unsigned long duration, now = READ_ONCE(jiffies);
+
+	if (req)
+		duration = jiffies_to_nsecs(now) - req->start_time_ns;
+	else
+		duration = jiffies_to_nsecs(now - start_jiffies);
+
+	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]);
+#endif
+}
+
 static void blk_account_io_completion(struct request *req, unsigned int bytes)
 {
 	if (req->part && blk_do_io_stat(req)) {
@@ -1301,6 +1328,7 @@ 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, 0);
 		part_stat_inc(req->part, ios[sgrp]);
 		part_stat_add(req->part, nsecs[sgrp], now - req->start_time_ns);
 		part_stat_unlock();
@@ -1360,6 +1388,7 @@ 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, NULL, start_time);
 	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 419548e92d82..643b679883e1 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1276,6 +1276,44 @@ static struct device_attribute dev_attr_fail_timeout =
 	__ATTR(io-timeout-fail, 0644, part_timeout_show, part_timeout_store);
 #endif
 
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+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);
+#endif
+
 static struct attribute *disk_attrs[] = {
 	&dev_attr_range.attr,
 	&dev_attr_ext_range.attr,
@@ -1294,6 +1332,9 @@ static struct attribute *disk_attrs[] = {
 #endif
 #ifdef CONFIG_FAIL_IO_TIMEOUT
 	&dev_attr_fail_timeout.attr,
+#endif
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+	&dev_attr_io_latency.attr,
 #endif
 	NULL
 };
diff --git a/include/linux/part_stat.h b/include/linux/part_stat.h
index d2558121d48c..763953238227 100644
--- a/include/linux/part_stat.h
+++ b/include/linux/part_stat.h
@@ -9,6 +9,13 @@ struct disk_stats {
 	unsigned long sectors[NR_STAT_GROUPS];
 	unsigned long ios[NR_STAT_GROUPS];
 	unsigned long merges[NR_STAT_GROUPS];
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+	/*
+	 * 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];
+#endif
 	unsigned long io_ticks;
 	local_t in_flight[2];
 };
-- 
2.17.1


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

* [PATCH RFC V4 2/4] block: add a statistic table for io sector
  2021-01-10  9:44 [PATCH RFC V4 0/4] block: add two statistic tables Guoqing Jiang
  2021-01-10  9:44 ` [PATCH RFC V4 1/4] block: add a statistic table for io latency Guoqing Jiang
@ 2021-01-10  9:44 ` Guoqing Jiang
  2021-01-10  9:44 ` [PATCH RFC V4 3/4] block: add io_extra_stats node Guoqing Jiang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Guoqing Jiang @ 2021-01-10  9:44 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, danil.kipnis, jinpu.wang, Guoqing Jiang

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@cloud.ionos.com).

Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 Documentation/ABI/testing/sysfs-block |  9 +++++++
 block/Kconfig                         |  3 ++-
 block/blk-core.c                      | 18 +++++++++++++
 block/genhd.c                         | 37 +++++++++++++++++++++++++++
 include/linux/part_stat.h             |  3 ++-
 5 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 4371a0f2cb5e..0ffb63469772 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:		January 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/Kconfig b/block/Kconfig
index ca3854a7e3db..ad04ec253b09 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -178,7 +178,8 @@ config BLK_ADDITIONAL_DISKSTAT
 	bool "Block layer additional diskstat"
 	default n
 	help
-	Enabling this option adds io latency statistics for each block device.
+	Enabling this option adds io latency and io size statistics for each
+	block device.
 
 	If unsure, say N.
 
diff --git a/block/blk-core.c b/block/blk-core.c
index 18cf881d8194..719595578bc8 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1304,12 +1304,29 @@ static void blk_additional_latency(struct block_device *part, const int sgrp,
 #endif
 }
 
+static void blk_additional_sector(struct block_device *part, const int sgrp,
+				  unsigned int sectors)
+{
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+	unsigned int idx;
+
+	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]);
+#endif
+}
+
 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, bytes >> SECTOR_SHIFT);
 		part_stat_add(req->part, sectors[sgrp], bytes >> 9);
 		part_stat_unlock();
 	}
@@ -1357,6 +1374,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, 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 643b679883e1..059c82088cc4 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1312,6 +1312,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);
 #endif
 
 static struct attribute *disk_attrs[] = {
@@ -1335,6 +1371,7 @@ static struct attribute *disk_attrs[] = {
 #endif
 #ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
 	&dev_attr_io_latency.attr,
+	&dev_attr_io_size.attr,
 #endif
 	NULL
 };
diff --git a/include/linux/part_stat.h b/include/linux/part_stat.h
index 763953238227..8a6440ec9134 100644
--- a/include/linux/part_stat.h
+++ b/include/linux/part_stat.h
@@ -11,10 +11,11 @@ struct disk_stats {
 	unsigned long merges[NR_STAT_GROUPS];
 #ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
 	/*
-	 * 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];
 #endif
 	unsigned long io_ticks;
 	local_t in_flight[2];
-- 
2.17.1


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

* [PATCH RFC V4 3/4] block: add io_extra_stats node
  2021-01-10  9:44 [PATCH RFC V4 0/4] block: add two statistic tables Guoqing Jiang
  2021-01-10  9:44 ` [PATCH RFC V4 1/4] block: add a statistic table for io latency Guoqing Jiang
  2021-01-10  9:44 ` [PATCH RFC V4 2/4] block: add a statistic table for io sector Guoqing Jiang
@ 2021-01-10  9:44 ` Guoqing Jiang
  2021-01-10  9:44 ` [PATCH RFC V4 4/4] block: call blk_additional_{latency,sector} only when io_extra_stats is true Guoqing Jiang
  2021-01-20 12:52 ` [PATCH RFC V4 0/4] block: add two statistic tables Jinpu Wang
  4 siblings, 0 replies; 6+ messages in thread
From: Guoqing Jiang @ 2021-01-10  9:44 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, danil.kipnis, jinpu.wang, Guoqing Jiang

Even we have introduced a Kconfig option (default N) to control the
accounting of additional data, but the option still could be enabled
occasionally while user doesn't care about the size and latency of io,
and they could suffer from the additional overhead. So introduce a
specific sysfs node to avoid such mistake.

Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 Documentation/ABI/testing/sysfs-block |  9 +++++++++
 Documentation/block/queue-sysfs.rst   |  6 ++++++
 block/blk-sysfs.c                     | 10 ++++++++++
 include/linux/blkdev.h                |  6 ++++++
 4 files changed, 31 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 0ffb63469772..e1611c62a3e1 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -333,3 +333,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:		January 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 2638d3446b79..14c4a4c7b9a9 100644
--- a/Documentation/block/queue-sysfs.rst
+++ b/Documentation/block/queue-sysfs.rst
@@ -99,6 +99,12 @@ 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, and BLK_ADDITIONAL_DISKSTAT should be
+enabled if you want the additional accounting.
+
 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 b513f1683af0..87f174f32e9a 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -288,6 +288,9 @@ 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(stable_writes, STABLE_WRITES, 0);
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+QUEUE_SYSFS_BIT_FNS(io_extra_stats, IO_EXTRA_STAT, 0);
+#endif
 #undef QUEUE_SYSFS_BIT_FNS
 
 static ssize_t queue_zoned_show(struct request_queue *q, char *page)
@@ -616,6 +619,10 @@ QUEUE_RW_ENTRY(queue_iostats, "iostats");
 QUEUE_RW_ENTRY(queue_random, "add_random");
 QUEUE_RW_ENTRY(queue_stable_writes, "stable_writes");
 
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+QUEUE_RW_ENTRY(queue_io_extra_stats, "io_extra_stats");
+#endif
+
 static struct attribute *queue_attrs[] = {
 	&queue_requests_entry.attr,
 	&queue_ra_entry.attr,
@@ -658,6 +665,9 @@ static struct attribute *queue_attrs[] = {
 	&queue_io_timeout_entry.attr,
 #ifdef CONFIG_BLK_DEV_THROTTLING_LOW
 	&blk_throtl_sample_time_entry.attr,
+#endif
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+	&queue_io_extra_stats_entry.attr,
 #endif
 	NULL,
 };
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 070de09425ad..a86ecd062340 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -625,6 +625,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) |		\
@@ -662,6 +663,11 @@ bool blk_queue_flag_test_and_set(unsigned int flag, struct request_queue *q);
 #else
 #define blk_queue_rq_alloc_time(q)	false
 #endif
+#ifdef CONFIG_BLK_ADDITIONAL_DISKSTAT
+#define blk_queue_io_extra_stat(q) test_bit(QUEUE_FLAG_IO_EXTRA_STAT, &(q)->queue_flags)
+#else
+#define blk_queue_io_extra_stat(q) false
+#endif
 
 #define blk_noretry_request(rq) \
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
-- 
2.17.1


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

* [PATCH RFC V4 4/4] block: call blk_additional_{latency,sector} only when io_extra_stats is true
  2021-01-10  9:44 [PATCH RFC V4 0/4] block: add two statistic tables Guoqing Jiang
                   ` (2 preceding siblings ...)
  2021-01-10  9:44 ` [PATCH RFC V4 3/4] block: add io_extra_stats node Guoqing Jiang
@ 2021-01-10  9:44 ` Guoqing Jiang
  2021-01-20 12:52 ` [PATCH RFC V4 0/4] block: add two statistic tables Jinpu Wang
  4 siblings, 0 replies; 6+ messages in thread
From: Guoqing Jiang @ 2021-01-10  9:44 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, danil.kipnis, jinpu.wang, Guoqing Jiang

If ADDITIONAL_DISKSTAT is enabled carelessly, then it is bad to people
who don't want the additional overhead.

Now add check before call blk_additional_{latency,sector}, which guarntee
only those who really know about the attribute can account the additional
data.

Signed-off-by: Guoqing Jiang <guoqing.jiang@cloud.ionos.com>
---
 block/blk-core.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 719595578bc8..c38287d34825 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1326,7 +1326,8 @@ static void blk_account_io_completion(struct request *req, unsigned int bytes)
 		const int sgrp = op_stat_group(req_op(req));
 
 		part_stat_lock();
-		blk_additional_sector(req->part, sgrp, bytes >> SECTOR_SHIFT);
+		if (blk_queue_io_extra_stat(req->q))
+			blk_additional_sector(req->part, sgrp, bytes >> SECTOR_SHIFT);
 		part_stat_add(req->part, sectors[sgrp], bytes >> 9);
 		part_stat_unlock();
 	}
@@ -1345,7 +1346,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, 0);
+		if (blk_queue_io_extra_stat(req->q))
+			blk_additional_latency(req->part, sgrp, req, 0);
 		part_stat_inc(req->part, ios[sgrp]);
 		part_stat_add(req->part, nsecs[sgrp], now - req->start_time_ns);
 		part_stat_unlock();
@@ -1374,7 +1376,8 @@ 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, sectors);
+	if (blk_queue_io_extra_stat(part->bd_disk->queue))
+		blk_additional_sector(part, sgrp, sectors);
 	part_stat_local_inc(part, in_flight[op_is_write(op)]);
 	part_stat_unlock();
 
@@ -1406,7 +1409,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, NULL, start_time);
+	if (blk_queue_io_extra_stat(part->bd_disk->queue))
+		blk_additional_latency(part, sgrp, NULL, start_time);
 	part_stat_add(part, nsecs[sgrp], jiffies_to_nsecs(duration));
 	part_stat_local_dec(part, in_flight[op_is_write(op)]);
 	part_stat_unlock();
-- 
2.17.1


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

* Re: [PATCH RFC V4 0/4] block: add two statistic tables
  2021-01-10  9:44 [PATCH RFC V4 0/4] block: add two statistic tables Guoqing Jiang
                   ` (3 preceding siblings ...)
  2021-01-10  9:44 ` [PATCH RFC V4 4/4] block: call blk_additional_{latency,sector} only when io_extra_stats is true Guoqing Jiang
@ 2021-01-20 12:52 ` Jinpu Wang
  4 siblings, 0 replies; 6+ messages in thread
From: Jinpu Wang @ 2021-01-20 12:52 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: Jens Axboe, linux-block, Danil Kipnis

On Sun, Jan 10, 2021 at 10:45 AM Guoqing Jiang
<guoqing.jiang@cloud.ionos.com> wrote:
>
> Hi,
>
> No more comments since the last version, so this version is just rebase
> with latest tree.
>
> To make the tables work, it is necessary to enable BLK_ADDITIONAL_DISKSTAT
> option, and also enable the sysfs node.
> # echo 1 > /sys/block/md0/queue/io_extra_stats
>
> After that, the size and latency of io are recorded in each table.
>
> Thanks,
> Guoqing
>
> 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 (4):
>   block: add a statistic table for io latency
>   block: add a statistic table for io sector
>   block: add io_extra_stats node
>   block: call blk_additional_{latency,sector} only when io_extra_stats
>     is true
>
>  Documentation/ABI/testing/sysfs-block | 26 +++++++++
>  Documentation/block/queue-sysfs.rst   |  6 +++
>  block/Kconfig                         |  9 ++++
>  block/blk-core.c                      | 51 ++++++++++++++++++
>  block/blk-sysfs.c                     | 10 ++++
>  block/genhd.c                         | 78 +++++++++++++++++++++++++++
>  include/linux/blkdev.h                |  6 +++
>  include/linux/part_stat.h             |  8 +++
>  8 files changed, 194 insertions(+)
>
> --
> 2.17.1
>
For the whole series,  look good to me, thx.
Reviewed-by: Jack Wang <jinpu.wang@cloud.ionos.com>

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

end of thread, other threads:[~2021-01-20 14:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-10  9:44 [PATCH RFC V4 0/4] block: add two statistic tables Guoqing Jiang
2021-01-10  9:44 ` [PATCH RFC V4 1/4] block: add a statistic table for io latency Guoqing Jiang
2021-01-10  9:44 ` [PATCH RFC V4 2/4] block: add a statistic table for io sector Guoqing Jiang
2021-01-10  9:44 ` [PATCH RFC V4 3/4] block: add io_extra_stats node Guoqing Jiang
2021-01-10  9:44 ` [PATCH RFC V4 4/4] block: call blk_additional_{latency,sector} only when io_extra_stats is true Guoqing Jiang
2021-01-20 12:52 ` [PATCH RFC V4 0/4] block: add two statistic tables Jinpu Wang

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.