* [PATCH V6 1/3] block: add io_extra_stats node
2021-04-09 16:03 [PATCH V6 0/3] block: add two statistic tables Md Haris Iqbal
@ 2021-04-09 16:03 ` Md Haris Iqbal
2021-04-09 16:03 ` [PATCH V6 2/3] block: add a statistic table for io latency Md Haris Iqbal
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Md Haris Iqbal @ 2021-04-09 16:03 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..3cf5a2cfaeb9 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@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] 14+ messages in thread* [PATCH V6 2/3] block: add a statistic table for io latency
2021-04-09 16:03 [PATCH V6 0/3] block: add two statistic tables Md Haris Iqbal
2021-04-09 16:03 ` [PATCH V6 1/3] block: add io_extra_stats node Md Haris Iqbal
@ 2021-04-09 16:03 ` Md Haris Iqbal
2021-04-09 16:03 ` [PATCH V6 3/3] block: add a statistic table for io sector Md Haris Iqbal
2021-04-09 21:03 ` [PATCH V6 0/3] block: add two statistic tables Jens Axboe
3 siblings, 0 replies; 14+ messages in thread
From: Md Haris Iqbal @ 2021-04-09 16:03 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 | 42 +++++++++++++++++++++++++++
include/linux/part_stat.h | 5 ++++
4 files changed, 79 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 3cf5a2cfaeb9..90970a8dc70f 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@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 39ca97b0edc6..66c6342968a3 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1067,6 +1067,47 @@ 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,
@@ -1086,6 +1127,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] 14+ messages in thread* [PATCH V6 3/3] block: add a statistic table for io sector
2021-04-09 16:03 [PATCH V6 0/3] block: add two statistic tables Md Haris Iqbal
2021-04-09 16:03 ` [PATCH V6 1/3] block: add io_extra_stats node Md Haris Iqbal
2021-04-09 16:03 ` [PATCH V6 2/3] block: add a statistic table for io latency Md Haris Iqbal
@ 2021-04-09 16:03 ` Md Haris Iqbal
2021-04-09 21:03 ` [PATCH V6 0/3] block: add two statistic tables Jens Axboe
3 siblings, 0 replies; 14+ messages in thread
From: Md Haris Iqbal @ 2021-04-09 16:03 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 | 20 ++++++++++++++
block/genhd.c | 39 +++++++++++++++++++++++++++
include/linux/part_stat.h | 3 ++-
4 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block
index 90970a8dc70f..a32b2c399e81 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@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..deaf82f7a478 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1283,12 +1283,31 @@ 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 +1360,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 66c6342968a3..cce3c1234282 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1108,6 +1108,44 @@ static ssize_t io_latency_show(struct device *dev,
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,
@@ -1128,6 +1166,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] 14+ messages in thread* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-09 16:03 [PATCH V6 0/3] block: add two statistic tables Md Haris Iqbal
` (2 preceding siblings ...)
2021-04-09 16:03 ` [PATCH V6 3/3] block: add a statistic table for io sector Md Haris Iqbal
@ 2021-04-09 21:03 ` Jens Axboe
2021-04-12 5:35 ` Jinpu Wang
3 siblings, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2021-04-09 21:03 UTC (permalink / raw)
To: Md Haris Iqbal; +Cc: linux-block, jinpu.wang, danil.kipnis
On 4/9/21 10:03 AM, Md Haris Iqbal wrote:
> Hi Jens,
>
> This version fixes the long lines in the code as per Christoph's comment.
I'd really like to see some solid justification for the addition,
though. I clicked the v1 link and it's got details on what you get out
of it, but not really the 'why' of reasoning for the feature. I mean,
you could feasibly have a blktrace based userspace solution. Just
wondering if that has been tried, I know that's what we do at Facebook
for example.
--
Jens Axboe
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-09 21:03 ` [PATCH V6 0/3] block: add two statistic tables Jens Axboe
@ 2021-04-12 5:35 ` Jinpu Wang
2021-04-19 6:37 ` Jinpu Wang
2021-04-19 17:57 ` Jens Axboe
0 siblings, 2 replies; 14+ messages in thread
From: Jinpu Wang @ 2021-04-12 5:35 UTC (permalink / raw)
To: Jens Axboe; +Cc: Md Haris Iqbal, linux-block, Danil Kipnis
On Fri, Apr 9, 2021 at 11:03 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 4/9/21 10:03 AM, Md Haris Iqbal wrote:
> > Hi Jens,
> >
> > This version fixes the long lines in the code as per Christoph's comment.
>
> I'd really like to see some solid justification for the addition,
> though. I clicked the v1 link and it's got details on what you get out
> of it, but not really the 'why' of reasoning for the feature. I mean,
> you could feasibly have a blktrace based userspace solution. Just
> wondering if that has been tried, I know that's what we do at Facebook
> for example.
>
Hi Jens,
Thanks for the reply.
For the use case of the additional stats, as a cloud provider, we
often need to handle report from the customers regarding
performance problem in a period of time in the past, so it's not
feasible for us to run blktrace, customer workload could change from
time to time, with the additional stats, we gather through all metrics
using Prometheus, we can navigate to the period of time interested,
to check if the performance matches the SLA, it also helps us to find
the user IO pattern, we can more easily reproduce.
We do use blktrace from time to time too if it's not too late (when IO
pattern has not changed.)
Thanks!
Jack
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-12 5:35 ` Jinpu Wang
@ 2021-04-19 6:37 ` Jinpu Wang
2021-04-19 17:57 ` Jens Axboe
1 sibling, 0 replies; 14+ messages in thread
From: Jinpu Wang @ 2021-04-19 6:37 UTC (permalink / raw)
To: Jens Axboe; +Cc: Md Haris Iqbal, linux-block, Danil Kipnis
On Mon, Apr 12, 2021 at 7:35 AM Jinpu Wang <jinpu.wang@ionos.com> wrote:
>
> On Fri, Apr 9, 2021 at 11:03 PM Jens Axboe <axboe@kernel.dk> wrote:
> >
> > On 4/9/21 10:03 AM, Md Haris Iqbal wrote:
> > > Hi Jens,
> > >
> > > This version fixes the long lines in the code as per Christoph's comment.
> >
> > I'd really like to see some solid justification for the addition,
> > though. I clicked the v1 link and it's got details on what you get out
> > of it, but not really the 'why' of reasoning for the feature. I mean,
> > you could feasibly have a blktrace based userspace solution. Just
> > wondering if that has been tried, I know that's what we do at Facebook
> > for example.
> >
> Hi Jens,
>
> Thanks for the reply.
> For the use case of the additional stats, as a cloud provider, we
> often need to handle report from the customers regarding
> performance problem in a period of time in the past, so it's not
> feasible for us to run blktrace, customer workload could change from
> time to time, with the additional stats, we gather through all metrics
> using Prometheus, we can navigate to the period of time interested,
> to check if the performance matches the SLA, it also helps us to find
> the user IO pattern, we can more easily reproduce.
>
> We do use blktrace from time to time too if it's not too late (when IO
> pattern has not changed.)
>
> Thanks!
> Jack
Hi Jens,
A gentle ping!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-12 5:35 ` Jinpu Wang
2021-04-19 6:37 ` Jinpu Wang
@ 2021-04-19 17:57 ` Jens Axboe
2021-04-21 7:49 ` Jinpu Wang
1 sibling, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2021-04-19 17:57 UTC (permalink / raw)
To: Jinpu Wang; +Cc: Md Haris Iqbal, linux-block, Danil Kipnis
On 4/11/21 11:35 PM, Jinpu Wang wrote:
> On Fri, Apr 9, 2021 at 11:03 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 4/9/21 10:03 AM, Md Haris Iqbal wrote:
>>> Hi Jens,
>>>
>>> This version fixes the long lines in the code as per Christoph's comment.
>>
>> I'd really like to see some solid justification for the addition,
>> though. I clicked the v1 link and it's got details on what you get out
>> of it, but not really the 'why' of reasoning for the feature. I mean,
>> you could feasibly have a blktrace based userspace solution. Just
>> wondering if that has been tried, I know that's what we do at Facebook
>> for example.
>>
> Hi Jens,
>
> Thanks for the reply.
> For the use case of the additional stats, as a cloud provider, we
> often need to handle report from the customers regarding
> performance problem in a period of time in the past, so it's not
> feasible for us to run blktrace, customer workload could change from
> time to time, with the additional stats, we gather through all metrics
> using Prometheus, we can navigate to the period of time interested,
> to check if the performance matches the SLA, it also helps us to find
> the user IO pattern, we can more easily reproduce.
My suggestion isn't to run just blktrace all the time, rather collect
the tracing info from there and store them away. Then you can go back
in time and see what is going on. Hence my questioning on adding this
new stat tracking, when it's already readily available to be consumed
by a small daemon that can continually track it in userspace.
--
Jens Axboe
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-19 17:57 ` Jens Axboe
@ 2021-04-21 7:49 ` Jinpu Wang
2021-04-21 10:20 ` Pavel Begunkov
0 siblings, 1 reply; 14+ messages in thread
From: Jinpu Wang @ 2021-04-21 7:49 UTC (permalink / raw)
To: Jens Axboe; +Cc: Md Haris Iqbal, linux-block, Danil Kipnis
On Mon, Apr 19, 2021 at 7:57 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 4/11/21 11:35 PM, Jinpu Wang wrote:
> > On Fri, Apr 9, 2021 at 11:03 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 4/9/21 10:03 AM, Md Haris Iqbal wrote:
> >>> Hi Jens,
> >>>
> >>> This version fixes the long lines in the code as per Christoph's comment.
> >>
> >> I'd really like to see some solid justification for the addition,
> >> though. I clicked the v1 link and it's got details on what you get out
> >> of it, but not really the 'why' of reasoning for the feature. I mean,
> >> you could feasibly have a blktrace based userspace solution. Just
> >> wondering if that has been tried, I know that's what we do at Facebook
> >> for example.
> >>
> > Hi Jens,
> >
> > Thanks for the reply.
> > For the use case of the additional stats, as a cloud provider, we
> > often need to handle report from the customers regarding
> > performance problem in a period of time in the past, so it's not
> > feasible for us to run blktrace, customer workload could change from
> > time to time, with the additional stats, we gather through all metrics
> > using Prometheus, we can navigate to the period of time interested,
> > to check if the performance matches the SLA, it also helps us to find
> > the user IO pattern, we can more easily reproduce.
>
> My suggestion isn't to run just blktrace all the time, rather collect
> the tracing info from there and store them away. Then you can go back
> in time and see what is going on. Hence my questioning on adding this
> new stat tracking, when it's already readily available to be consumed
> by a small daemon that can continually track it in userspace.
>
> --
> Jens Axboe
>
Hi Jens,
The problem with using blktrace at production may cause a performance
drop ~30%. while with the block stats here, we only see ~3% when
enabled.
We did a benchmark with rnbd.
Fio config:
[global]
description=Emulation of Storage Server Access Pattern
bssplit=512/20:1k/16:2k/9:4k/12:8k/19:16k/10:32k/8:64k/4
fadvise_hint=0
rw=randrw:2
direct=1
random_distribution=zipf:1.2
#size=1G
time_based=1
runtime=10
ramp_time=1
ioengine=libaio
iodepth=128
iodepth_batch_submit=128
iodepth_batch_complete=128
numjobs=1
#gtod_reduce=1
group_reporting
[job1]
filename=/dev/rnbd0
blktrace command:
# blktrace -a read -a write -d /dev/rnbd0
read IOPS drops 35%, similar for write IOPS.
RNBD-No-blktrace RNBD-With-blktrace
102056.894311 -35.5%
The tests are done with v5.4.30.
Test hardware is
root@x4-left:~/haris/sds-perf# uname -a
Linux x4-left 5.10.30-pserver
#5.10.30-1+feature+linux+5.10.y+20210414.1233+e3dd267~deb10 SMP
x86_64 GNU/Linux
root@x4-left:~/haris/sds-perf# lscpu
Architecture: x86_64
CPU op-mode(s): 32-bit, 64-bit
Byte Order: Little Endian
Address sizes: 46 bits physical, 48 bits virtual
CPU(s): 40
On-line CPU(s) list: 0-39
Thread(s) per core: 2
Core(s) per socket: 10
Socket(s): 2
NUMA node(s): 2
Vendor ID: GenuineIntel
CPU family: 6
Model: 85
Model name: Intel(R) Xeon(R) Silver 4114 CPU @ 2.20GHz
Stepping: 4
CPU MHz: 800.571
CPU max MHz: 3000.0000
CPU min MHz: 800.0000
BogoMIPS: 4400.00
Virtualization: VT-x
L1d cache: 32K
L1i cache: 32K
L2 cache: 1024K
L3 cache: 14080K
NUMA node0 CPU(s): 0-9,20-29
NUMA node1 CPU(s): 10-19,30-39
Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts
rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq
dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm
pcid dca sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes
xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb cat_l3
cdp_l3 invpcid_single intel_ppin mba ibrs ibpb stibp tpr_shadow vnmi
flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 hle avx2 smep
bmi2 erms invpcid rtm cqm mpx rdt_a avx512f avx512dq rdseed adx smap
clflushopt clwb intel_pt avx512cd avx512bw avx512vl xsaveopt xsavec
xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local
dtherm ida arat pln pts pku ospke
root@x4-left:~/haris/sds-perf# ibstat
CA 'mlx5_0'
CA type: MT4115
Number of ports: 1
Firmware version: 12.26.4012
Hardware version: 0
Node GUID: 0xec0d9a0300c5fffc
System image GUID: 0xec0d9a0300c5fffc
Port 1:
State: Active
Physical state: LinkUp
Rate: 100
Base lid: 3
LMC: 0
SM lid: 3
Capability mask: 0x2651e84a
Port GUID: 0xec0d9a0300c5fffc
Link layer: InfiniBand
CA 'mlx5_1'
CA type: MT4115
Number of ports: 1
Firmware version: 12.26.4012
Hardware version: 0
Node GUID: 0xec0d9a0300c5fffd
System image GUID: 0xec0d9a0300c5fffc
Port 1:
State: Active
Physical state: LinkUp
Rate: 100
Base lid: 1
LMC: 0
SM lid: 1
Capability mask: 0x2651e84a
Port GUID: 0xec0d9a0300c5fffd
Link layer: InfiniBand
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-21 7:49 ` Jinpu Wang
@ 2021-04-21 10:20 ` Pavel Begunkov
2021-04-21 11:50 ` Jinpu Wang
0 siblings, 1 reply; 14+ messages in thread
From: Pavel Begunkov @ 2021-04-21 10:20 UTC (permalink / raw)
To: Jinpu Wang, Jens Axboe; +Cc: Md Haris Iqbal, linux-block, Danil Kipnis
On 4/21/21 8:49 AM, Jinpu Wang wrote:
> On Mon, Apr 19, 2021 at 7:57 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 4/11/21 11:35 PM, Jinpu Wang wrote:
>>> On Fri, Apr 9, 2021 at 11:03 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 4/9/21 10:03 AM, Md Haris Iqbal wrote:
>>>>> Hi Jens,
>>>>>
>>>>> This version fixes the long lines in the code as per Christoph's comment.
>>>>
>>>> I'd really like to see some solid justification for the addition,
>>>> though. I clicked the v1 link and it's got details on what you get out
>>>> of it, but not really the 'why' of reasoning for the feature. I mean,
>>>> you could feasibly have a blktrace based userspace solution. Just
>>>> wondering if that has been tried, I know that's what we do at Facebook
>>>> for example.
>>>>
>>> Hi Jens,
>>>
>>> Thanks for the reply.
>>> For the use case of the additional stats, as a cloud provider, we
>>> often need to handle report from the customers regarding
>>> performance problem in a period of time in the past, so it's not
>>> feasible for us to run blktrace, customer workload could change from
>>> time to time, with the additional stats, we gather through all metrics
>>> using Prometheus, we can navigate to the period of time interested,
>>> to check if the performance matches the SLA, it also helps us to find
>>> the user IO pattern, we can more easily reproduce.
>>
>> My suggestion isn't to run just blktrace all the time, rather collect
>> the tracing info from there and store them away. Then you can go back
>> in time and see what is going on. Hence my questioning on adding this
>> new stat tracking, when it's already readily available to be consumed
>> by a small daemon that can continually track it in userspace.
>>
>> --
>> Jens Axboe
>>
> Hi Jens,
> The problem with using blktrace at production may cause a performance
> drop ~30%. while with the block stats here, we only see ~3% when
> enabled.
It's probably was asked before, but let's refresh as the discussion
erupted again.
I get your problem with blktrace(8), IIRC it definitely can deteriorate
performance if run constantly, but did you try to write a bpf program
that does smarter accumulation in the kernel? Like making bpf to collect
a latency table (right as in your patches do) and flushing it to the
disk periodically?
> We did a benchmark with rnbd.
>
> Fio config:
> [global]
> description=Emulation of Storage Server Access Pattern
> bssplit=512/20:1k/16:2k/9:4k/12:8k/19:16k/10:32k/8:64k/4
> fadvise_hint=0
> rw=randrw:2
> direct=1
> random_distribution=zipf:1.2
> #size=1G
> time_based=1
> runtime=10
> ramp_time=1
> ioengine=libaio
> iodepth=128
> iodepth_batch_submit=128
> iodepth_batch_complete=128
> numjobs=1
> #gtod_reduce=1
> group_reporting
>
> [job1]
> filename=/dev/rnbd0
>
> blktrace command:
> # blktrace -a read -a write -d /dev/rnbd0
>
> read IOPS drops 35%, similar for write IOPS.
>
> RNBD-No-blktrace RNBD-With-blktrace
>
> 102056.894311 -35.5%
>
> The tests are done with v5.4.30.
> Test hardware is
> root@x4-left:~/haris/sds-perf# uname -a
> Linux x4-left 5.10.30-pserver
> #5.10.30-1+feature+linux+5.10.y+20210414.1233+e3dd267~deb10 SMP
> x86_64 GNU/Linux
> root@x4-left:~/haris/sds-perf# lscpu
> Architecture: x86_64
> CPU op-mode(s): 32-bit, 64-bit
> Byte Order: Little Endian
> Address sizes: 46 bits physical, 48 bits virtual
> CPU(s): 40
> On-line CPU(s) list: 0-39
> Thread(s) per core: 2
> Core(s) per socket: 10
> Socket(s): 2
> NUMA node(s): 2
> Vendor ID: GenuineIntel
> CPU family: 6
> Model: 85
> Model name: Intel(R) Xeon(R) Silver 4114 CPU @ 2.20GHz
> Stepping: 4
> CPU MHz: 800.571
> CPU max MHz: 3000.0000
> CPU min MHz: 800.0000
> BogoMIPS: 4400.00
> Virtualization: VT-x
> L1d cache: 32K
> L1i cache: 32K
> L2 cache: 1024K
> L3 cache: 14080K
> NUMA node0 CPU(s): 0-9,20-29
> NUMA node1 CPU(s): 10-19,30-39
> Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
> pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
> syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts
> rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq
> dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm
> pcid dca sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes
> xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb cat_l3
> cdp_l3 invpcid_single intel_ppin mba ibrs ibpb stibp tpr_shadow vnmi
> flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 hle avx2 smep
> bmi2 erms invpcid rtm cqm mpx rdt_a avx512f avx512dq rdseed adx smap
> clflushopt clwb intel_pt avx512cd avx512bw avx512vl xsaveopt xsavec
> xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local
> dtherm ida arat pln pts pku ospke
> root@x4-left:~/haris/sds-perf# ibstat
> CA 'mlx5_0'
> CA type: MT4115
> Number of ports: 1
> Firmware version: 12.26.4012
> Hardware version: 0
> Node GUID: 0xec0d9a0300c5fffc
> System image GUID: 0xec0d9a0300c5fffc
> Port 1:
> State: Active
> Physical state: LinkUp
> Rate: 100
> Base lid: 3
> LMC: 0
> SM lid: 3
> Capability mask: 0x2651e84a
> Port GUID: 0xec0d9a0300c5fffc
> Link layer: InfiniBand
> CA 'mlx5_1'
> CA type: MT4115
> Number of ports: 1
> Firmware version: 12.26.4012
> Hardware version: 0
> Node GUID: 0xec0d9a0300c5fffd
> System image GUID: 0xec0d9a0300c5fffc
> Port 1:
> State: Active
> Physical state: LinkUp
> Rate: 100
> Base lid: 1
> LMC: 0
> SM lid: 1
> Capability mask: 0x2651e84a
> Port GUID: 0xec0d9a0300c5fffd
> Link layer: InfiniBand
>
> Thanks!
>
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-21 10:20 ` Pavel Begunkov
@ 2021-04-21 11:50 ` Jinpu Wang
2021-04-21 11:55 ` Pavel Begunkov
0 siblings, 1 reply; 14+ messages in thread
From: Jinpu Wang @ 2021-04-21 11:50 UTC (permalink / raw)
To: Pavel Begunkov; +Cc: Jens Axboe, Md Haris Iqbal, linux-block, Danil Kipnis
On Wed, Apr 21, 2021 at 12:20 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 4/21/21 8:49 AM, Jinpu Wang wrote:
> > On Mon, Apr 19, 2021 at 7:57 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 4/11/21 11:35 PM, Jinpu Wang wrote:
> >>> On Fri, Apr 9, 2021 at 11:03 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>
> >>>> On 4/9/21 10:03 AM, Md Haris Iqbal wrote:
> >>>>> Hi Jens,
> >>>>>
> >>>>> This version fixes the long lines in the code as per Christoph's comment.
> >>>>
> >>>> I'd really like to see some solid justification for the addition,
> >>>> though. I clicked the v1 link and it's got details on what you get out
> >>>> of it, but not really the 'why' of reasoning for the feature. I mean,
> >>>> you could feasibly have a blktrace based userspace solution. Just
> >>>> wondering if that has been tried, I know that's what we do at Facebook
> >>>> for example.
> >>>>
> >>> Hi Jens,
> >>>
> >>> Thanks for the reply.
> >>> For the use case of the additional stats, as a cloud provider, we
> >>> often need to handle report from the customers regarding
> >>> performance problem in a period of time in the past, so it's not
> >>> feasible for us to run blktrace, customer workload could change from
> >>> time to time, with the additional stats, we gather through all metrics
> >>> using Prometheus, we can navigate to the period of time interested,
> >>> to check if the performance matches the SLA, it also helps us to find
> >>> the user IO pattern, we can more easily reproduce.
> >>
> >> My suggestion isn't to run just blktrace all the time, rather collect
> >> the tracing info from there and store them away. Then you can go back
> >> in time and see what is going on. Hence my questioning on adding this
> >> new stat tracking, when it's already readily available to be consumed
> >> by a small daemon that can continually track it in userspace.
> >>
> >> --
> >> Jens Axboe
> >>
> > Hi Jens,
> > The problem with using blktrace at production may cause a performance
> > drop ~30%. while with the block stats here, we only see ~3% when
> > enabled.
>
> It's probably was asked before, but let's refresh as the discussion
> erupted again.
>
> I get your problem with blktrace(8), IIRC it definitely can deteriorate
> performance if run constantly, but did you try to write a bpf program
> that does smarter accumulation in the kernel? Like making bpf to collect
> a latency table (right as in your patches do) and flushing it to the
> disk periodically?
Hi Pavel,
Thanks for the suggestion.
We did test with ebpf with kprobe in the past (~kernel 4.4/4.14), we
saw 10% performance drop, that's the reason we develop this
stats patches.
But I just did another test with bpftrace on k 5.10.30, I do not see
performance lost.
It must be ebpf is improving very much since then.
So to summarize, we can use bpftrace to do the drop in latest kernel,
there is no need to have it build into the kernel.
Thanks!
the bpftrace I used during testing:
root@x4-left:~# cat /usr/sbin/biolatency.bt
#!/usr/bin/env bpftrace
/*
* biolatency.bt Block I/O latency as a histogram.
* For Linux, uses bpftrace, eBPF.
*
* This is a bpftrace version of the bcc tool of the same name.
*
* Copyright 2018 Netflix, Inc.
* Licensed under the Apache License, Version 2.0 (the "License")
*
* 13-Sep-2018 Brendan Gregg Created this.
*/
BEGIN
{
printf("Tracing block device I/O... Hit Ctrl-C to end.\n");
}
kprobe:blk_account_io_start
{
@start[arg0] = nsecs;
}
kprobe:blk_account_io_done
/@start[arg0]/
{
@usecs = hist((nsecs - @start[arg0]) / 1000);
delete(@start[arg0]);
}
>
>
> > We did a benchmark with rnbd.
> >
> > Fio config:
> > [global]
> > description=Emulation of Storage Server Access Pattern
> > bssplit=512/20:1k/16:2k/9:4k/12:8k/19:16k/10:32k/8:64k/4
> > fadvise_hint=0
> > rw=randrw:2
> > direct=1
> > random_distribution=zipf:1.2
> > #size=1G
> > time_based=1
> > runtime=10
> > ramp_time=1
> > ioengine=libaio
> > iodepth=128
> > iodepth_batch_submit=128
> > iodepth_batch_complete=128
> > numjobs=1
> > #gtod_reduce=1
> > group_reporting
> >
> > [job1]
> > filename=/dev/rnbd0
> >
> > blktrace command:
> > # blktrace -a read -a write -d /dev/rnbd0
> >
> > read IOPS drops 35%, similar for write IOPS.
> >
> > RNBD-No-blktrace RNBD-With-blktrace
> >
> > 102056.894311 -35.5%
> >
> > The tests are done with v5.4.30.
> > Test hardware is
> > root@x4-left:~/haris/sds-perf# uname -a
> > Linux x4-left 5.10.30-pserver
> > #5.10.30-1+feature+linux+5.10.y+20210414.1233+e3dd267~deb10 SMP
> > x86_64 GNU/Linux
> > root@x4-left:~/haris/sds-perf# lscpu
> > Architecture: x86_64
> > CPU op-mode(s): 32-bit, 64-bit
> > Byte Order: Little Endian
> > Address sizes: 46 bits physical, 48 bits virtual
> > CPU(s): 40
> > On-line CPU(s) list: 0-39
> > Thread(s) per core: 2
> > Core(s) per socket: 10
> > Socket(s): 2
> > NUMA node(s): 2
> > Vendor ID: GenuineIntel
> > CPU family: 6
> > Model: 85
> > Model name: Intel(R) Xeon(R) Silver 4114 CPU @ 2.20GHz
> > Stepping: 4
> > CPU MHz: 800.571
> > CPU max MHz: 3000.0000
> > CPU min MHz: 800.0000
> > BogoMIPS: 4400.00
> > Virtualization: VT-x
> > L1d cache: 32K
> > L1i cache: 32K
> > L2 cache: 1024K
> > L3 cache: 14080K
> > NUMA node0 CPU(s): 0-9,20-29
> > NUMA node1 CPU(s): 10-19,30-39
> > Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
> > pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
> > syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts
> > rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq
> > dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm
> > pcid dca sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes
> > xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb cat_l3
> > cdp_l3 invpcid_single intel_ppin mba ibrs ibpb stibp tpr_shadow vnmi
> > flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 hle avx2 smep
> > bmi2 erms invpcid rtm cqm mpx rdt_a avx512f avx512dq rdseed adx smap
> > clflushopt clwb intel_pt avx512cd avx512bw avx512vl xsaveopt xsavec
> > xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local
> > dtherm ida arat pln pts pku ospke
> > root@x4-left:~/haris/sds-perf# ibstat
> > CA 'mlx5_0'
> > CA type: MT4115
> > Number of ports: 1
> > Firmware version: 12.26.4012
> > Hardware version: 0
> > Node GUID: 0xec0d9a0300c5fffc
> > System image GUID: 0xec0d9a0300c5fffc
> > Port 1:
> > State: Active
> > Physical state: LinkUp
> > Rate: 100
> > Base lid: 3
> > LMC: 0
> > SM lid: 3
> > Capability mask: 0x2651e84a
> > Port GUID: 0xec0d9a0300c5fffc
> > Link layer: InfiniBand
> > CA 'mlx5_1'
> > CA type: MT4115
> > Number of ports: 1
> > Firmware version: 12.26.4012
> > Hardware version: 0
> > Node GUID: 0xec0d9a0300c5fffd
> > System image GUID: 0xec0d9a0300c5fffc
> > Port 1:
> > State: Active
> > Physical state: LinkUp
> > Rate: 100
> > Base lid: 1
> > LMC: 0
> > SM lid: 1
> > Capability mask: 0x2651e84a
> > Port GUID: 0xec0d9a0300c5fffd
> > Link layer: InfiniBand
> >
> > Thanks!
> >
>
> --
> Pavel Begunkov
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-21 11:50 ` Jinpu Wang
@ 2021-04-21 11:55 ` Pavel Begunkov
2021-04-21 11:57 ` Jinpu Wang
0 siblings, 1 reply; 14+ messages in thread
From: Pavel Begunkov @ 2021-04-21 11:55 UTC (permalink / raw)
To: Jinpu Wang; +Cc: Jens Axboe, Md Haris Iqbal, linux-block, Danil Kipnis
On 4/21/21 12:50 PM, Jinpu Wang wrote:
[snip]
>>> Hi Jens,
>>> The problem with using blktrace at production may cause a performance
>>> drop ~30%. while with the block stats here, we only see ~3% when
>>> enabled.
>>
>> It's probably was asked before, but let's refresh as the discussion
>> erupted again.
>>
>> I get your problem with blktrace(8), IIRC it definitely can deteriorate
>> performance if run constantly, but did you try to write a bpf program
>> that does smarter accumulation in the kernel? Like making bpf to collect
>> a latency table (right as in your patches do) and flushing it to the
>> disk periodically?
> Hi Pavel,
>
> Thanks for the suggestion.
>
> We did test with ebpf with kprobe in the past (~kernel 4.4/4.14), we
> saw 10% performance drop, that's the reason we develop this
> stats patches.
>
> But I just did another test with bpftrace on k 5.10.30, I do not see
> performance lost.
> It must be ebpf is improving very much since then.
>
> So to summarize, we can use bpftrace to do the drop in latest kernel,
> there is no need to have it build into the kernel.
Perfect, and I'm sure it will be even more convenient for you, for
instance to gather other stats or do it somehow differently
> Thanks!
>
> the bpftrace I used during testing:
> root@x4-left:~# cat /usr/sbin/biolatency.bt
> #!/usr/bin/env bpftrace
> /*
> * biolatency.bt Block I/O latency as a histogram.
> * For Linux, uses bpftrace, eBPF.
> *
> * This is a bpftrace version of the bcc tool of the same name.
> *
> * Copyright 2018 Netflix, Inc.
> * Licensed under the Apache License, Version 2.0 (the "License")
> *
> * 13-Sep-2018 Brendan Gregg Created this.
> */
>
> BEGIN
> {
> printf("Tracing block device I/O... Hit Ctrl-C to end.\n");
> }
>
> kprobe:blk_account_io_start
> {
> @start[arg0] = nsecs;
> }
>
> kprobe:blk_account_io_done
> /@start[arg0]/
>
> {
> @usecs = hist((nsecs - @start[arg0]) / 1000);
> delete(@start[arg0]);
> }
>
>
>
>>
>>
>>> We did a benchmark with rnbd.
>>>
>>> Fio config:
>>> [global]
>>> description=Emulation of Storage Server Access Pattern
>>> bssplit=512/20:1k/16:2k/9:4k/12:8k/19:16k/10:32k/8:64k/4
>>> fadvise_hint=0
>>> rw=randrw:2
>>> direct=1
>>> random_distribution=zipf:1.2
>>> #size=1G
>>> time_based=1
>>> runtime=10
>>> ramp_time=1
>>> ioengine=libaio
>>> iodepth=128
>>> iodepth_batch_submit=128
>>> iodepth_batch_complete=128
>>> numjobs=1
>>> #gtod_reduce=1
>>> group_reporting
>>>
>>> [job1]
>>> filename=/dev/rnbd0
>>>
>>> blktrace command:
>>> # blktrace -a read -a write -d /dev/rnbd0
>>>
>>> read IOPS drops 35%, similar for write IOPS.
>>>
>>> RNBD-No-blktrace RNBD-With-blktrace
>>>
>>> 102056.894311 -35.5%
>>>
>>> The tests are done with v5.4.30.
>>> Test hardware is
>>> root@x4-left:~/haris/sds-perf# uname -a
>>> Linux x4-left 5.10.30-pserver
>>> #5.10.30-1+feature+linux+5.10.y+20210414.1233+e3dd267~deb10 SMP
>>> x86_64 GNU/Linux
>>> root@x4-left:~/haris/sds-perf# lscpu
>>> Architecture: x86_64
>>> CPU op-mode(s): 32-bit, 64-bit
>>> Byte Order: Little Endian
>>> Address sizes: 46 bits physical, 48 bits virtual
>>> CPU(s): 40
>>> On-line CPU(s) list: 0-39
>>> Thread(s) per core: 2
>>> Core(s) per socket: 10
>>> Socket(s): 2
>>> NUMA node(s): 2
>>> Vendor ID: GenuineIntel
>>> CPU family: 6
>>> Model: 85
>>> Model name: Intel(R) Xeon(R) Silver 4114 CPU @ 2.20GHz
>>> Stepping: 4
>>> CPU MHz: 800.571
>>> CPU max MHz: 3000.0000
>>> CPU min MHz: 800.0000
>>> BogoMIPS: 4400.00
>>> Virtualization: VT-x
>>> L1d cache: 32K
>>> L1i cache: 32K
>>> L2 cache: 1024K
>>> L3 cache: 14080K
>>> NUMA node0 CPU(s): 0-9,20-29
>>> NUMA node1 CPU(s): 10-19,30-39
>>> Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
>>> pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
>>> syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts
>>> rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq
>>> dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm
>>> pcid dca sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes
>>> xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb cat_l3
>>> cdp_l3 invpcid_single intel_ppin mba ibrs ibpb stibp tpr_shadow vnmi
>>> flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 hle avx2 smep
>>> bmi2 erms invpcid rtm cqm mpx rdt_a avx512f avx512dq rdseed adx smap
>>> clflushopt clwb intel_pt avx512cd avx512bw avx512vl xsaveopt xsavec
>>> xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local
>>> dtherm ida arat pln pts pku ospke
>>> root@x4-left:~/haris/sds-perf# ibstat
>>> CA 'mlx5_0'
>>> CA type: MT4115
>>> Number of ports: 1
>>> Firmware version: 12.26.4012
>>> Hardware version: 0
>>> Node GUID: 0xec0d9a0300c5fffc
>>> System image GUID: 0xec0d9a0300c5fffc
>>> Port 1:
>>> State: Active
>>> Physical state: LinkUp
>>> Rate: 100
>>> Base lid: 3
>>> LMC: 0
>>> SM lid: 3
>>> Capability mask: 0x2651e84a
>>> Port GUID: 0xec0d9a0300c5fffc
>>> Link layer: InfiniBand
>>> CA 'mlx5_1'
>>> CA type: MT4115
>>> Number of ports: 1
>>> Firmware version: 12.26.4012
>>> Hardware version: 0
>>> Node GUID: 0xec0d9a0300c5fffd
>>> System image GUID: 0xec0d9a0300c5fffc
>>> Port 1:
>>> State: Active
>>> Physical state: LinkUp
>>> Rate: 100
>>> Base lid: 1
>>> LMC: 0
>>> SM lid: 1
>>> Capability mask: 0x2651e84a
>>> Port GUID: 0xec0d9a0300c5fffd
>>> Link layer: InfiniBand
>>>
>>> Thanks!
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-21 11:55 ` Pavel Begunkov
@ 2021-04-21 11:57 ` Jinpu Wang
2021-04-21 17:02 ` Jens Axboe
0 siblings, 1 reply; 14+ messages in thread
From: Jinpu Wang @ 2021-04-21 11:57 UTC (permalink / raw)
To: Pavel Begunkov; +Cc: Jens Axboe, Md Haris Iqbal, linux-block, Danil Kipnis
On Wed, Apr 21, 2021 at 1:55 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 4/21/21 12:50 PM, Jinpu Wang wrote:
> [snip]
> >>> Hi Jens,
> >>> The problem with using blktrace at production may cause a performance
> >>> drop ~30%. while with the block stats here, we only see ~3% when
> >>> enabled.
> >>
> >> It's probably was asked before, but let's refresh as the discussion
> >> erupted again.
> >>
> >> I get your problem with blktrace(8), IIRC it definitely can deteriorate
> >> performance if run constantly, but did you try to write a bpf program
> >> that does smarter accumulation in the kernel? Like making bpf to collect
> >> a latency table (right as in your patches do) and flushing it to the
> >> disk periodically?
> > Hi Pavel,
> >
> > Thanks for the suggestion.
> >
> > We did test with ebpf with kprobe in the past (~kernel 4.4/4.14), we
> > saw 10% performance drop, that's the reason we develop this
> > stats patches.
> >
> > But I just did another test with bpftrace on k 5.10.30, I do not see
> > performance lost.
> > It must be ebpf is improving very much since then.
> >
> > So to summarize, we can use bpftrace to do the drop in latest kernel,
> > there is no need to have it build into the kernel.
>
> Perfect, and I'm sure it will be even more convenient for you, for
> instance to gather other stats or do it somehow differently
Yeah, agree.
Thanks again!
>
> > Thanks!
> >
> > the bpftrace I used during testing:
> > root@x4-left:~# cat /usr/sbin/biolatency.bt
> > #!/usr/bin/env bpftrace
> > /*
> > * biolatency.bt Block I/O latency as a histogram.
> > * For Linux, uses bpftrace, eBPF.
> > *
> > * This is a bpftrace version of the bcc tool of the same name.
> > *
> > * Copyright 2018 Netflix, Inc.
> > * Licensed under the Apache License, Version 2.0 (the "License")
> > *
> > * 13-Sep-2018 Brendan Gregg Created this.
> > */
> >
> > BEGIN
> > {
> > printf("Tracing block device I/O... Hit Ctrl-C to end.\n");
> > }
> >
> > kprobe:blk_account_io_start
> > {
> > @start[arg0] = nsecs;
> > }
> >
> > kprobe:blk_account_io_done
> > /@start[arg0]/
> >
> > {
> > @usecs = hist((nsecs - @start[arg0]) / 1000);
> > delete(@start[arg0]);
> > }
> >
> >
> >
> >>
> >>
> >>> We did a benchmark with rnbd.
> >>>
> >>> Fio config:
> >>> [global]
> >>> description=Emulation of Storage Server Access Pattern
> >>> bssplit=512/20:1k/16:2k/9:4k/12:8k/19:16k/10:32k/8:64k/4
> >>> fadvise_hint=0
> >>> rw=randrw:2
> >>> direct=1
> >>> random_distribution=zipf:1.2
> >>> #size=1G
> >>> time_based=1
> >>> runtime=10
> >>> ramp_time=1
> >>> ioengine=libaio
> >>> iodepth=128
> >>> iodepth_batch_submit=128
> >>> iodepth_batch_complete=128
> >>> numjobs=1
> >>> #gtod_reduce=1
> >>> group_reporting
> >>>
> >>> [job1]
> >>> filename=/dev/rnbd0
> >>>
> >>> blktrace command:
> >>> # blktrace -a read -a write -d /dev/rnbd0
> >>>
> >>> read IOPS drops 35%, similar for write IOPS.
> >>>
> >>> RNBD-No-blktrace RNBD-With-blktrace
> >>>
> >>> 102056.894311 -35.5%
> >>>
> >>> The tests are done with v5.4.30.
> >>> Test hardware is
> >>> root@x4-left:~/haris/sds-perf# uname -a
> >>> Linux x4-left 5.10.30-pserver
> >>> #5.10.30-1+feature+linux+5.10.y+20210414.1233+e3dd267~deb10 SMP
> >>> x86_64 GNU/Linux
> >>> root@x4-left:~/haris/sds-perf# lscpu
> >>> Architecture: x86_64
> >>> CPU op-mode(s): 32-bit, 64-bit
> >>> Byte Order: Little Endian
> >>> Address sizes: 46 bits physical, 48 bits virtual
> >>> CPU(s): 40
> >>> On-line CPU(s) list: 0-39
> >>> Thread(s) per core: 2
> >>> Core(s) per socket: 10
> >>> Socket(s): 2
> >>> NUMA node(s): 2
> >>> Vendor ID: GenuineIntel
> >>> CPU family: 6
> >>> Model: 85
> >>> Model name: Intel(R) Xeon(R) Silver 4114 CPU @ 2.20GHz
> >>> Stepping: 4
> >>> CPU MHz: 800.571
> >>> CPU max MHz: 3000.0000
> >>> CPU min MHz: 800.0000
> >>> BogoMIPS: 4400.00
> >>> Virtualization: VT-x
> >>> L1d cache: 32K
> >>> L1i cache: 32K
> >>> L2 cache: 1024K
> >>> L3 cache: 14080K
> >>> NUMA node0 CPU(s): 0-9,20-29
> >>> NUMA node1 CPU(s): 10-19,30-39
> >>> Flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr
> >>> pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
> >>> syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts
> >>> rep_good nopl xtopology nonstop_tsc cpuid aperfmperf pni pclmulqdq
> >>> dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm
> >>> pcid dca sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes
> >>> xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb cat_l3
> >>> cdp_l3 invpcid_single intel_ppin mba ibrs ibpb stibp tpr_shadow vnmi
> >>> flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 hle avx2 smep
> >>> bmi2 erms invpcid rtm cqm mpx rdt_a avx512f avx512dq rdseed adx smap
> >>> clflushopt clwb intel_pt avx512cd avx512bw avx512vl xsaveopt xsavec
> >>> xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local
> >>> dtherm ida arat pln pts pku ospke
> >>> root@x4-left:~/haris/sds-perf# ibstat
> >>> CA 'mlx5_0'
> >>> CA type: MT4115
> >>> Number of ports: 1
> >>> Firmware version: 12.26.4012
> >>> Hardware version: 0
> >>> Node GUID: 0xec0d9a0300c5fffc
> >>> System image GUID: 0xec0d9a0300c5fffc
> >>> Port 1:
> >>> State: Active
> >>> Physical state: LinkUp
> >>> Rate: 100
> >>> Base lid: 3
> >>> LMC: 0
> >>> SM lid: 3
> >>> Capability mask: 0x2651e84a
> >>> Port GUID: 0xec0d9a0300c5fffc
> >>> Link layer: InfiniBand
> >>> CA 'mlx5_1'
> >>> CA type: MT4115
> >>> Number of ports: 1
> >>> Firmware version: 12.26.4012
> >>> Hardware version: 0
> >>> Node GUID: 0xec0d9a0300c5fffd
> >>> System image GUID: 0xec0d9a0300c5fffc
> >>> Port 1:
> >>> State: Active
> >>> Physical state: LinkUp
> >>> Rate: 100
> >>> Base lid: 1
> >>> LMC: 0
> >>> SM lid: 1
> >>> Capability mask: 0x2651e84a
> >>> Port GUID: 0xec0d9a0300c5fffd
> >>> Link layer: InfiniBand
> >>>
> >>> Thanks!
>
> --
> Pavel Begunkov
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH V6 0/3] block: add two statistic tables
2021-04-21 11:57 ` Jinpu Wang
@ 2021-04-21 17:02 ` Jens Axboe
0 siblings, 0 replies; 14+ messages in thread
From: Jens Axboe @ 2021-04-21 17:02 UTC (permalink / raw)
To: Jinpu Wang, Pavel Begunkov; +Cc: Md Haris Iqbal, linux-block, Danil Kipnis
On 4/21/21 5:57 AM, Jinpu Wang wrote:
> On Wed, Apr 21, 2021 at 1:55 PM Pavel Begunkov <asml.silence@gmail.com> wrote:
>>
>> On 4/21/21 12:50 PM, Jinpu Wang wrote:
>> [snip]
>>>>> Hi Jens,
>>>>> The problem with using blktrace at production may cause a performance
>>>>> drop ~30%. while with the block stats here, we only see ~3% when
>>>>> enabled.
>>>>
>>>> It's probably was asked before, but let's refresh as the discussion
>>>> erupted again.
>>>>
>>>> I get your problem with blktrace(8), IIRC it definitely can deteriorate
>>>> performance if run constantly, but did you try to write a bpf program
>>>> that does smarter accumulation in the kernel? Like making bpf to collect
>>>> a latency table (right as in your patches do) and flushing it to the
>>>> disk periodically?
>>> Hi Pavel,
>>>
>>> Thanks for the suggestion.
>>>
>>> We did test with ebpf with kprobe in the past (~kernel 4.4/4.14), we
>>> saw 10% performance drop, that's the reason we develop this
>>> stats patches.
>>>
>>> But I just did another test with bpftrace on k 5.10.30, I do not see
>>> performance lost.
>>> It must be ebpf is improving very much since then.
>>>
>>> So to summarize, we can use bpftrace to do the drop in latest kernel,
>>> there is no need to have it build into the kernel.
>>
>> Perfect, and I'm sure it will be even more convenient for you, for
>> instance to gather other stats or do it somehow differently
>
> Yeah, agree.
> Thanks again!
Perfect, thanks! I'll drop the series.
--
Jens Axboe
^ permalink raw reply [flat|nested] 14+ messages in thread