* [Qemu-devel] [PATCH v2 0/3] Add block size histogram qapi interface
@ 2019-07-08 9:32 zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 1/3] block/accounting: rename struct BlockLatencyHistogram zhenwei pi
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: zhenwei pi @ 2019-07-08 9:32 UTC (permalink / raw)
To: kwolf, vsementsov, eblake; +Cc: qemu-devel, qemu-block, pizhenwei, mreitz
Modify command 'block-latency-histogram-set' to make block histogram
interface common to use. And support block size histogram.
Thanks to Eric Blake&Vladimir Sementsov-Ogievskiy for the suggestions.
This command has been tested for half year on QEMU-2.12, and we found
that 3K+ virtual machines write 25GB/s totally, the block size
histogram like following:
0 ~ 8k: 58% ~ 62%
8k ~ 32k: 10% ~ 12%
32k ~ 128k: 2% ~ 3%
128K ~ 512K: 24% ~ 26%
512K ~ : ...
And the histogram data help us to optimise backend distributed
storage.
Changelog:
v2:
- make 'block-latency-histogram-set' common.
- remove duplicated functions.
- fix uncommon indentation(reviewed by Vladimir Sementsov-Ogievskiy).
zhenwei pi (3):
block/accounting: rename struct BlockLatencyHistogram
block/accounting: introduce block size histogram
qapi: make block histogram interface common
block/accounting.c | 62 ++++++++++++++++++++--------
block/qapi.c | 32 ++++++++------
blockdev.c | 33 +++++++++++----
include/block/accounting.h | 13 +++---
qapi/block-core.json | 101 +++++++++++++++++++++++++++------------------
5 files changed, 158 insertions(+), 83 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 1/3] block/accounting: rename struct BlockLatencyHistogram
2019-07-08 9:32 [Qemu-devel] [PATCH v2 0/3] Add block size histogram qapi interface zhenwei pi
@ 2019-07-08 9:32 ` zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 2/3] block/accounting: introduce block size histogram zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 3/3] qapi: make block histogram interface common zhenwei pi
2 siblings, 0 replies; 6+ messages in thread
From: zhenwei pi @ 2019-07-08 9:32 UTC (permalink / raw)
To: kwolf, vsementsov, eblake; +Cc: qemu-devel, qemu-block, pizhenwei, mreitz
Rename struct BlockLatencyHistogram to BlockHistogram, and rename
related functions.
Make this struct and functions be common, they can be used widely.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/accounting.c | 44 +++++++++++++++++++++++++++-----------------
block/qapi.c | 23 +++++++++++------------
include/block/accounting.h | 8 ++++----
3 files changed, 42 insertions(+), 33 deletions(-)
diff --git a/block/accounting.c b/block/accounting.c
index 70a3d9a426..d210a73fe9 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -94,13 +94,14 @@ void block_acct_start(BlockAcctStats *stats, BlockAcctCookie *cookie,
cookie->type = type;
}
-/* block_latency_histogram_compare_func:
+/**
+ * block_histogram_compare_func:
* Compare @key with interval [@it[0], @it[1]).
* Return: -1 if @key < @it[0]
* 0 if @key in [@it[0], @it[1])
* +1 if @key >= @it[1]
*/
-static int block_latency_histogram_compare_func(const void *key, const void *it)
+static int block_histogram_compare_func(const void *key, const void *it)
{
uint64_t k = *(uint64_t *)key;
uint64_t a = ((uint64_t *)it)[0];
@@ -109,8 +110,7 @@ static int block_latency_histogram_compare_func(const void *key, const void *it)
return k < a ? -1 : (k < b ? 0 : 1);
}
-static void block_latency_histogram_account(BlockLatencyHistogram *hist,
- int64_t latency_ns)
+static void block_histogram_account(BlockHistogram *hist, int64_t val)
{
uint64_t *pos;
@@ -120,28 +120,26 @@ static void block_latency_histogram_account(BlockLatencyHistogram *hist,
}
- if (latency_ns < hist->boundaries[0]) {
+ if (val < hist->boundaries[0]) {
hist->bins[0]++;
return;
}
- if (latency_ns >= hist->boundaries[hist->nbins - 2]) {
+ if (val >= hist->boundaries[hist->nbins - 2]) {
hist->bins[hist->nbins - 1]++;
return;
}
- pos = bsearch(&latency_ns, hist->boundaries, hist->nbins - 2,
+ pos = bsearch(&val, hist->boundaries, hist->nbins - 2,
sizeof(hist->boundaries[0]),
- block_latency_histogram_compare_func);
+ block_histogram_compare_func);
assert(pos != NULL);
hist->bins[pos - hist->boundaries + 1]++;
}
-int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
- uint64List *boundaries)
+static int block_histogram_set(BlockHistogram *hist, uint64List *boundaries)
{
- BlockLatencyHistogram *hist = &stats->latency_histogram[type];
uint64List *entry;
uint64_t *ptr;
uint64_t prev = 0;
@@ -170,15 +168,27 @@ int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
return 0;
}
+static void block_histogram_clear(BlockHistogram *hist)
+{
+ g_free(hist->bins);
+ g_free(hist->boundaries);
+ memset(hist, 0, sizeof(*hist));
+}
+
+int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
+ uint64List *boundaries)
+{
+ BlockHistogram *hist = &stats->latency_histogram[type];
+
+ return block_histogram_set(hist, boundaries);
+}
+
void block_latency_histograms_clear(BlockAcctStats *stats)
{
int i;
for (i = 0; i < BLOCK_MAX_IOTYPE; i++) {
- BlockLatencyHistogram *hist = &stats->latency_histogram[i];
- g_free(hist->bins);
- g_free(hist->boundaries);
- memset(hist, 0, sizeof(*hist));
+ block_histogram_clear(&stats->latency_histogram[i]);
}
}
@@ -204,8 +214,8 @@ static void block_account_one_io(BlockAcctStats *stats, BlockAcctCookie *cookie,
stats->nr_ops[cookie->type]++;
}
- block_latency_histogram_account(&stats->latency_histogram[cookie->type],
- latency_ns);
+ block_histogram_account(&stats->latency_histogram[cookie->type],
+ latency_ns);
if (!failed || stats->account_failed) {
stats->total_time_ns[cookie->type] += latency_ns;
diff --git a/block/qapi.c b/block/qapi.c
index 917435f022..9d9b2f1927 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -415,9 +415,8 @@ static uint64List *uint64_list(uint64_t *list, int size)
return out_list;
}
-static void bdrv_latency_histogram_stats(BlockLatencyHistogram *hist,
- bool *not_null,
- BlockLatencyHistogramInfo **info)
+static void bdrv_histogram_stats(BlockHistogram *hist, bool *not_null,
+ BlockLatencyHistogramInfo **info)
{
*not_null = hist->bins != NULL;
if (*not_null) {
@@ -494,15 +493,15 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
block_acct_queue_depth(ts, BLOCK_ACCT_WRITE);
}
- bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_READ],
- &ds->has_rd_latency_histogram,
- &ds->rd_latency_histogram);
- bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_WRITE],
- &ds->has_wr_latency_histogram,
- &ds->wr_latency_histogram);
- bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_FLUSH],
- &ds->has_flush_latency_histogram,
- &ds->flush_latency_histogram);
+ bdrv_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_READ],
+ &ds->has_rd_latency_histogram,
+ &ds->rd_latency_histogram);
+ bdrv_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_WRITE],
+ &ds->has_wr_latency_histogram,
+ &ds->wr_latency_histogram);
+ bdrv_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_FLUSH],
+ &ds->has_flush_latency_histogram,
+ &ds->flush_latency_histogram);
}
static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs,
diff --git a/include/block/accounting.h b/include/block/accounting.h
index d1f67b10dd..270fddb69c 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -46,7 +46,7 @@ struct BlockAcctTimedStats {
QSLIST_ENTRY(BlockAcctTimedStats) entries;
};
-typedef struct BlockLatencyHistogram {
+typedef struct BlockHistogram {
/* The following histogram is represented like this:
*
* 5| *
@@ -57,7 +57,7 @@ typedef struct BlockLatencyHistogram {
* +------------------
* 10 50 100
*
- * BlockLatencyHistogram histogram = {
+ * BlockHistogram histogram = {
* .nbins = 4,
* .boundaries = {10, 50, 100},
* .bins = {3, 1, 5, 2},
@@ -74,7 +74,7 @@ typedef struct BlockLatencyHistogram {
uint64_t *boundaries; /* @nbins-1 numbers here
(all boundaries, except 0 and +inf) */
uint64_t *bins;
-} BlockLatencyHistogram;
+} BlockHistogram;
struct BlockAcctStats {
QemuMutex lock;
@@ -88,7 +88,7 @@ struct BlockAcctStats {
QSLIST_HEAD(, BlockAcctTimedStats) intervals;
bool account_invalid;
bool account_failed;
- BlockLatencyHistogram latency_histogram[BLOCK_MAX_IOTYPE];
+ BlockHistogram latency_histogram[BLOCK_MAX_IOTYPE];
};
typedef struct BlockAcctCookie {
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] block/accounting: introduce block size histogram
2019-07-08 9:32 [Qemu-devel] [PATCH v2 0/3] Add block size histogram qapi interface zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 1/3] block/accounting: rename struct BlockLatencyHistogram zhenwei pi
@ 2019-07-08 9:32 ` zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 3/3] qapi: make block histogram interface common zhenwei pi
2 siblings, 0 replies; 6+ messages in thread
From: zhenwei pi @ 2019-07-08 9:32 UTC (permalink / raw)
To: kwolf, vsementsov, eblake; +Cc: qemu-devel, qemu-block, pizhenwei, mreitz
Introduce block size histogram statics for block devices.
For read/write/flush operation type, the block size region
[0, +inf) is divided into subregions by several points.
It works like block latency histogram.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/accounting.c | 18 ++++++++++++++++++
include/block/accounting.h | 5 ++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/block/accounting.c b/block/accounting.c
index d210a73fe9..3a1e41a971 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -192,6 +192,22 @@ void block_latency_histograms_clear(BlockAcctStats *stats)
}
}
+int block_size_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
+ uint64List *boundaries)
+{
+ BlockHistogram *hist = &stats->size_histogram[type];
+
+ return block_histogram_set(hist, boundaries);
+}
+
+void block_size_histograms_clear(BlockAcctStats *stats)
+{
+ int i;
+
+ for (i = 0; i < BLOCK_MAX_IOTYPE; i++) {
+ block_histogram_clear(&stats->size_histogram[i]);
+ }
+}
static void block_account_one_io(BlockAcctStats *stats, BlockAcctCookie *cookie,
bool failed)
{
@@ -216,6 +232,8 @@ static void block_account_one_io(BlockAcctStats *stats, BlockAcctCookie *cookie,
block_histogram_account(&stats->latency_histogram[cookie->type],
latency_ns);
+ block_histogram_account(&stats->size_histogram[cookie->type],
+ cookie->bytes);
if (!failed || stats->account_failed) {
stats->total_time_ns[cookie->type] += latency_ns;
diff --git a/include/block/accounting.h b/include/block/accounting.h
index 270fddb69c..0fbba64408 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -89,6 +89,7 @@ struct BlockAcctStats {
bool account_invalid;
bool account_failed;
BlockHistogram latency_histogram[BLOCK_MAX_IOTYPE];
+ BlockHistogram size_histogram[BLOCK_MAX_IOTYPE];
};
typedef struct BlockAcctCookie {
@@ -117,5 +118,7 @@ double block_acct_queue_depth(BlockAcctTimedStats *stats,
int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
uint64List *boundaries);
void block_latency_histograms_clear(BlockAcctStats *stats);
-
+int block_size_histogram_set(BlockAcctStats *stats, enum BlockAcctType type,
+ uint64List *boundaries);
+void block_size_histograms_clear(BlockAcctStats *stats);
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] qapi: make block histogram interface common
2019-07-08 9:32 [Qemu-devel] [PATCH v2 0/3] Add block size histogram qapi interface zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 1/3] block/accounting: rename struct BlockLatencyHistogram zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 2/3] block/accounting: introduce block size histogram zhenwei pi
@ 2019-07-08 9:32 ` zhenwei pi
2019-07-25 11:53 ` Vladimir Sementsov-Ogievskiy
2 siblings, 1 reply; 6+ messages in thread
From: zhenwei pi @ 2019-07-08 9:32 UTC (permalink / raw)
To: kwolf, vsementsov, eblake; +Cc: qemu-devel, qemu-block, pizhenwei, mreitz
Modify command 'block-latency-histogram-set' to 'block-histogram-set'
and modify struct 'BlockLatencyHistogramInfo' to struct
'BlockHistogramInfo', this makes block histogram interface common to
use.
Currently 'BlockHistogramType' supports 'latency', it works as same
as the old command 'block-latency-histogram-set'. New enum 'size'
allows QEMU to fill histogram by block I/O size in byte.
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
block/qapi.c | 11 +++++-
blockdev.c | 33 +++++++++++++----
qapi/block-core.json | 101 +++++++++++++++++++++++++++++++--------------------
3 files changed, 95 insertions(+), 50 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index 9d9b2f1927..c778b2c376 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -416,11 +416,11 @@ static uint64List *uint64_list(uint64_t *list, int size)
}
static void bdrv_histogram_stats(BlockHistogram *hist, bool *not_null,
- BlockLatencyHistogramInfo **info)
+ BlockHistogramInfo **info)
{
*not_null = hist->bins != NULL;
if (*not_null) {
- *info = g_new0(BlockLatencyHistogramInfo, 1);
+ *info = g_new0(BlockHistogramInfo, 1);
(*info)->boundaries = uint64_list(hist->boundaries, hist->nbins - 1);
(*info)->bins = uint64_list(hist->bins, hist->nbins);
@@ -502,6 +502,13 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
bdrv_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_FLUSH],
&ds->has_flush_latency_histogram,
&ds->flush_latency_histogram);
+ bdrv_histogram_stats(&stats->size_histogram[BLOCK_ACCT_READ],
+ &ds->has_rd_size_histogram, &ds->rd_size_histogram);
+ bdrv_histogram_stats(&stats->size_histogram[BLOCK_ACCT_WRITE],
+ &ds->has_wr_size_histogram, &ds->wr_size_histogram);
+ bdrv_histogram_stats(&stats->size_histogram[BLOCK_ACCT_FLUSH],
+ &ds->has_flush_size_histogram,
+ &ds->flush_size_histogram);
}
static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs,
diff --git a/blockdev.c b/blockdev.c
index 5d6a13dea9..82240ae8c5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4507,8 +4507,9 @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
aio_context_release(old_context);
}
-void qmp_block_latency_histogram_set(
+void qmp_block_histogram_set(
const char *id,
+ BlockHistogramType type,
bool has_boundaries, uint64List *boundaries,
bool has_boundaries_read, uint64List *boundaries_read,
bool has_boundaries_write, uint64List *boundaries_write,
@@ -4517,24 +4518,42 @@ void qmp_block_latency_histogram_set(
{
BlockBackend *blk = qmp_get_blk(NULL, id, errp);
BlockAcctStats *stats;
+ void (*clear_func)(BlockAcctStats *stats);
+ int (*set_func)(BlockAcctStats *stats, enum BlockAcctType type,
+ uint64List *boundaries);
int ret;
if (!blk) {
return;
}
+ switch (type) {
+ case BLOCK_HISTOGRAM_TYPE_LATENCY: {
+ clear_func = block_latency_histograms_clear;
+ set_func = block_latency_histogram_set;
+ break;
+ }
+ case BLOCK_HISTOGRAM_TYPE_SIZE: {
+ clear_func = block_size_histograms_clear;
+ set_func = block_size_histogram_set;
+ break;
+ }
+ default:
+ error_setg(errp, "Unsupported block histogram type");
+ return;
+ }
+
stats = blk_get_stats(blk);
if (!has_boundaries && !has_boundaries_read && !has_boundaries_write &&
!has_boundaries_flush)
{
- block_latency_histograms_clear(stats);
+ clear_func(stats);
return;
}
if (has_boundaries || has_boundaries_read) {
- ret = block_latency_histogram_set(
- stats, BLOCK_ACCT_READ,
+ ret = set_func(stats, BLOCK_ACCT_READ,
has_boundaries_read ? boundaries_read : boundaries);
if (ret) {
error_setg(errp, "Device '%s' set read boundaries fail", id);
@@ -4543,8 +4562,7 @@ void qmp_block_latency_histogram_set(
}
if (has_boundaries || has_boundaries_write) {
- ret = block_latency_histogram_set(
- stats, BLOCK_ACCT_WRITE,
+ ret = set_func(stats, BLOCK_ACCT_WRITE,
has_boundaries_write ? boundaries_write : boundaries);
if (ret) {
error_setg(errp, "Device '%s' set write boundaries fail", id);
@@ -4553,8 +4571,7 @@ void qmp_block_latency_histogram_set(
}
if (has_boundaries || has_boundaries_flush) {
- ret = block_latency_histogram_set(
- stats, BLOCK_ACCT_FLUSH,
+ ret = set_func(stats, BLOCK_ACCT_FLUSH,
has_boundaries_flush ? boundaries_flush : boundaries);
if (ret) {
error_setg(errp, "Device '%s' set flush boundaries fail", id);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d43d4f37c..b3b8d714d3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -533,12 +533,12 @@
'flags': ['Qcow2BitmapInfoFlags'] } }
##
-# @BlockLatencyHistogramInfo:
+# @BlockHistogramInfo:
#
-# Block latency histogram.
+# Block histogram.
#
-# @boundaries: list of interval boundary values in nanoseconds, all greater
-# than zero and in ascending order.
+# @boundaries: list of interval boundary values, all greater than zero and in
+# ascending order.
# For example, the list [10, 50, 100] produces the following
# histogram intervals: [0, 10), [10, 50), [50, 100), [100, +inf).
#
@@ -555,57 +555,72 @@
# +------------------
# 10 50 100
#
-# Since: 4.0
+# Since: 4.1
##
-{ 'struct': 'BlockLatencyHistogramInfo',
+{ 'struct': 'BlockHistogramInfo',
'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }
##
-# @block-latency-histogram-set:
+# @BlockHistogramType:
#
-# Manage read, write and flush latency histograms for the device.
+# An enumeration of block histogram type.
#
-# If only @id parameter is specified, remove all present latency histograms
-# for the device. Otherwise, add/reset some of (or all) latency histograms.
+# @latency: The histogram is filled by block I/O latency in nanosecond.
+#
+# @size: The histogram is filled by block I/O size in byte.
+#
+# Since: 4.1
+##
+{ 'enum': 'BlockHistogramType', 'data': [ 'latency', 'size' ] }
+
+##
+# @block-histogram-set:
+#
+# Manage read, write and flush @BlockHistogramType histograms for the device.
+#
+# If only @id and @type parameter are specified, remove all present @type
+# histograms for the device. Otherwise, add/reset some of (or all) @type
+# histograms.
#
# @id: The name or QOM path of the guest device.
#
+# @type: The type @BlockHistogramType of histogram.
+#
# @boundaries: list of interval boundary values (see description in
-# BlockLatencyHistogramInfo definition). If specified, all
-# latency histograms are removed, and empty ones created for all
-# io types with intervals corresponding to @boundaries (except for
-# io types, for which specific boundaries are set through the
-# following parameters).
+# BlockHistogramInfo definition). If specified, all histograms
+# are removed, and empty ones created for all io types with
+# intervals corresponding to @boundaries (except for io types
+# for which specific boundaries are set through the following
+# parameters).
#
-# @boundaries-read: list of interval boundary values for read latency
-# histogram. If specified, old read latency histogram is
-# removed, and empty one created with intervals
-# corresponding to @boundaries-read. The parameter has higher
-# priority then @boundaries.
+# @boundaries-read: list of interval boundary values for read histogram. If
+# specified, old read histogram is removed, and empty one
+# created with intervals corresponding to @boundaries-read.
+# The parameter has higher priority then @boundaries.
#
-# @boundaries-write: list of interval boundary values for write latency
-# histogram.
+# @boundaries-write: list of interval boundary values for write histogram.
#
-# @boundaries-flush: list of interval boundary values for flush latency
-# histogram.
+# @boundaries-flush: list of interval boundary values for flush histogram.
#
# Returns: error if device is not found or any boundary arrays are invalid.
#
-# Since: 4.0
+# Since: 4.1
#
-# Example: set new histograms for all io types with intervals
+# Example: set new latency histograms for all io types with intervals
# [0, 10), [10, 50), [50, 100), [100, +inf):
#
-# -> { "execute": "block-latency-histogram-set",
+# -> { "execute": "block-histogram-set",
# "arguments": { "id": "drive0",
+# "type":"latency",
# "boundaries": [10, 50, 100] } }
# <- { "return": {} }
#
-# Example: set new histogram only for write, other histograms will remain
-# not changed (or not created):
+# Example: set new latency histogram only for write, other histograms will
+# remain not changed (or not created):
#
-# -> { "execute": "block-latency-histogram-set",
+# -> { "execute": "block-histogram-set",
# "arguments": { "id": "drive0",
+# "type":"latency",
# "boundaries-write": [10, 50, 100] } }
# <- { "return": {} }
#
@@ -613,20 +628,23 @@
# read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
# write: [0, 1000), [1000, 5000), [5000, +inf)
#
-# -> { "execute": "block-latency-histogram-set",
+# -> { "execute": "block-histogram-set",
# "arguments": { "id": "drive0",
+# "type":"latency",
# "boundaries": [10, 50, 100],
# "boundaries-write": [1000, 5000] } }
# <- { "return": {} }
#
# Example: remove all latency histograms:
#
-# -> { "execute": "block-latency-histogram-set",
-# "arguments": { "id": "drive0" } }
+# -> { "execute": "block-histogram-set",
+# "arguments": { "id": "drive0"
+# "type":"latency"} }
# <- { "return": {} }
##
-{ 'command': 'block-latency-histogram-set',
+{ 'command': 'block-histogram-set',
'data': {'id': 'str',
+ 'type': 'BlockHistogramType',
'*boundaries': ['uint64'],
'*boundaries-read': ['uint64'],
'*boundaries-write': ['uint64'],
@@ -912,11 +930,11 @@
# @timed_stats: Statistics specific to the set of previously defined
# intervals of time (Since 2.5)
#
-# @rd_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
+# @rd_latency_histogram: @BlockHistogramInfo. (Since 4.1)
#
-# @wr_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
+# @wr_latency_histogram: @BlockHistogramInfo. (Since 4.1)
#
-# @flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
+# @flush_latency_histogram: @BlockHistogramInfo. (Since 4.1)
#
# Since: 0.14.0
##
@@ -931,9 +949,12 @@
'invalid_wr_operations': 'int', 'invalid_flush_operations': 'int',
'account_invalid': 'bool', 'account_failed': 'bool',
'timed_stats': ['BlockDeviceTimedStats'],
- '*rd_latency_histogram': 'BlockLatencyHistogramInfo',
- '*wr_latency_histogram': 'BlockLatencyHistogramInfo',
- '*flush_latency_histogram': 'BlockLatencyHistogramInfo' } }
+ '*rd_latency_histogram': 'BlockHistogramInfo',
+ '*wr_latency_histogram': 'BlockHistogramInfo',
+ '*flush_latency_histogram': 'BlockHistogramInfo',
+ '*rd_size_histogram': 'BlockHistogramInfo',
+ '*wr_size_histogram': 'BlockHistogramInfo',
+ '*flush_size_histogram': 'BlockHistogramInfo' } }
##
# @BlockStats:
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] qapi: make block histogram interface common
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 3/3] qapi: make block histogram interface common zhenwei pi
@ 2019-07-25 11:53 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-07-25 11:53 UTC (permalink / raw)
To: zhenwei pi, kwolf, eblake; +Cc: qemu-devel, qemu-block, mreitz
08.07.2019 12:32, zhenwei pi wrote:
> Modify command 'block-latency-histogram-set' to 'block-histogram-set'
> and modify struct 'BlockLatencyHistogramInfo' to struct
> 'BlockHistogramInfo', this makes block histogram interface common to
> use.
>
> Currently 'BlockHistogramType' supports 'latency', it works as same
> as the old command 'block-latency-histogram-set'. New enum 'size'
> allows QEMU to fill histogram by block I/O size in byte.
>
> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Hi Zhenwei!
You should add "Reviewed-by" marks only after someone give them,
writing it in reply to your patch. If someones reply don't have
such mark, you should not add it.
> ---
> block/qapi.c | 11 +++++-
> blockdev.c | 33 +++++++++++++----
> qapi/block-core.json | 101 +++++++++++++++++++++++++++++++--------------------
> 3 files changed, 95 insertions(+), 50 deletions(-)
>
> diff --git a/block/qapi.c b/block/qapi.c
> index 9d9b2f1927..c778b2c376 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -416,11 +416,11 @@ static uint64List *uint64_list(uint64_t *list, int size)
> }
>
> static void bdrv_histogram_stats(BlockHistogram *hist, bool *not_null,
> - BlockLatencyHistogramInfo **info)
> + BlockHistogramInfo **info)
> {
> *not_null = hist->bins != NULL;
> if (*not_null) {
> - *info = g_new0(BlockLatencyHistogramInfo, 1);
> + *info = g_new0(BlockHistogramInfo, 1);
>
> (*info)->boundaries = uint64_list(hist->boundaries, hist->nbins - 1);
> (*info)->bins = uint64_list(hist->bins, hist->nbins);
> @@ -502,6 +502,13 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
> bdrv_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_FLUSH],
> &ds->has_flush_latency_histogram,
> &ds->flush_latency_histogram);
> + bdrv_histogram_stats(&stats->size_histogram[BLOCK_ACCT_READ],
> + &ds->has_rd_size_histogram, &ds->rd_size_histogram);
> + bdrv_histogram_stats(&stats->size_histogram[BLOCK_ACCT_WRITE],
> + &ds->has_wr_size_histogram, &ds->wr_size_histogram);
> + bdrv_histogram_stats(&stats->size_histogram[BLOCK_ACCT_FLUSH],
> + &ds->has_flush_size_histogram,
> + &ds->flush_size_histogram);
> }
>
> static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs,
> diff --git a/blockdev.c b/blockdev.c
> index 5d6a13dea9..82240ae8c5 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -4507,8 +4507,9 @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,
> aio_context_release(old_context);
> }
>
> -void qmp_block_latency_histogram_set(
> +void qmp_block_histogram_set(
> const char *id,
> + BlockHistogramType type,
> bool has_boundaries, uint64List *boundaries,
> bool has_boundaries_read, uint64List *boundaries_read,
> bool has_boundaries_write, uint64List *boundaries_write,
> @@ -4517,24 +4518,42 @@ void qmp_block_latency_histogram_set(
> {
> BlockBackend *blk = qmp_get_blk(NULL, id, errp);
> BlockAcctStats *stats;
> + void (*clear_func)(BlockAcctStats *stats);
> + int (*set_func)(BlockAcctStats *stats, enum BlockAcctType type,
> + uint64List *boundaries);
> int ret;
>
> if (!blk) {
> return;
> }
>
> + switch (type) {
> + case BLOCK_HISTOGRAM_TYPE_LATENCY: {
> + clear_func = block_latency_histograms_clear;
> + set_func = block_latency_histogram_set;
Hmm, seems too tricky. This is because we have mapping (int type) -> (stats field name).
And these two functions are mostly duplication, the difference is that they works with different
fields.
So, I think better to combine both size_histogram and latency_histogram to one array field, where
index is histogram type. This will reduce duplication and we can drop this switch(){}.
> + break;
> + }
> + case BLOCK_HISTOGRAM_TYPE_SIZE: {
> + clear_func = block_size_histograms_clear;
> + set_func = block_size_histogram_set;
> + break;
> + }
> + default:
> + error_setg(errp, "Unsupported block histogram type");
> + return;
> + }
> +
> stats = blk_get_stats(blk);
>
> if (!has_boundaries && !has_boundaries_read && !has_boundaries_write &&
> !has_boundaries_flush)
> {
> - block_latency_histograms_clear(stats);
> + clear_func(stats);
> return;
> }
>
> if (has_boundaries || has_boundaries_read) {
> - ret = block_latency_histogram_set(
> - stats, BLOCK_ACCT_READ,
> + ret = set_func(stats, BLOCK_ACCT_READ,
> has_boundaries_read ? boundaries_read : boundaries);
> if (ret) {
> error_setg(errp, "Device '%s' set read boundaries fail", id);
> @@ -4543,8 +4562,7 @@ void qmp_block_latency_histogram_set(
> }
>
> if (has_boundaries || has_boundaries_write) {
> - ret = block_latency_histogram_set(
> - stats, BLOCK_ACCT_WRITE,
> + ret = set_func(stats, BLOCK_ACCT_WRITE,
> has_boundaries_write ? boundaries_write : boundaries);
> if (ret) {
> error_setg(errp, "Device '%s' set write boundaries fail", id);
> @@ -4553,8 +4571,7 @@ void qmp_block_latency_histogram_set(
> }
>
> if (has_boundaries || has_boundaries_flush) {
> - ret = block_latency_histogram_set(
> - stats, BLOCK_ACCT_FLUSH,
> + ret = set_func(stats, BLOCK_ACCT_FLUSH,
> has_boundaries_flush ? boundaries_flush : boundaries);
> if (ret) {
> error_setg(errp, "Device '%s' set flush boundaries fail", id);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 0d43d4f37c..b3b8d714d3 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
And about qapi part:
1. Eric, as I understand we should not drop old command but deprecate it and add a new one?
2. 4.1 -> 4.2
> @@ -533,12 +533,12 @@
> 'flags': ['Qcow2BitmapInfoFlags'] } }
>
> ##
> -# @BlockLatencyHistogramInfo:
> +# @BlockHistogramInfo:
> #
> -# Block latency histogram.
> +# Block histogram.
> #
> -# @boundaries: list of interval boundary values in nanoseconds, all greater
> -# than zero and in ascending order.
> +# @boundaries: list of interval boundary values, all greater than zero and in
> +# ascending order.
> # For example, the list [10, 50, 100] produces the following
> # histogram intervals: [0, 10), [10, 50), [50, 100), [100, +inf).
> #
> @@ -555,57 +555,72 @@
> # +------------------
> # 10 50 100
> #
> -# Since: 4.0
> +# Since: 4.1
> ##
> -{ 'struct': 'BlockLatencyHistogramInfo',
> +{ 'struct': 'BlockHistogramInfo',
> 'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }
>
> ##
> -# @block-latency-histogram-set:
> +# @BlockHistogramType:
> #
> -# Manage read, write and flush latency histograms for the device.
> +# An enumeration of block histogram type.
> #
> -# If only @id parameter is specified, remove all present latency histograms
> -# for the device. Otherwise, add/reset some of (or all) latency histograms.
> +# @latency: The histogram is filled by block I/O latency in nanosecond.
> +#
> +# @size: The histogram is filled by block I/O size in byte.
> +#
> +# Since: 4.1
> +##
> +{ 'enum': 'BlockHistogramType', 'data': [ 'latency', 'size' ] }
> +
> +##
> +# @block-histogram-set:
> +#
> +# Manage read, write and flush @BlockHistogramType histograms for the device.
> +#
> +# If only @id and @type parameter are specified, remove all present @type
> +# histograms for the device. Otherwise, add/reset some of (or all) @type
> +# histograms.
> #
> # @id: The name or QOM path of the guest device.
> #
> +# @type: The type @BlockHistogramType of histogram.
> +#
> # @boundaries: list of interval boundary values (see description in
> -# BlockLatencyHistogramInfo definition). If specified, all
> -# latency histograms are removed, and empty ones created for all
> -# io types with intervals corresponding to @boundaries (except for
> -# io types, for which specific boundaries are set through the
> -# following parameters).
> +# BlockHistogramInfo definition). If specified, all histograms
> +# are removed, and empty ones created for all io types with
> +# intervals corresponding to @boundaries (except for io types
> +# for which specific boundaries are set through the following
> +# parameters).
> #
> -# @boundaries-read: list of interval boundary values for read latency
> -# histogram. If specified, old read latency histogram is
> -# removed, and empty one created with intervals
> -# corresponding to @boundaries-read. The parameter has higher
> -# priority then @boundaries.
> +# @boundaries-read: list of interval boundary values for read histogram. If
> +# specified, old read histogram is removed, and empty one
> +# created with intervals corresponding to @boundaries-read.
> +# The parameter has higher priority then @boundaries.
> #
> -# @boundaries-write: list of interval boundary values for write latency
> -# histogram.
> +# @boundaries-write: list of interval boundary values for write histogram.
> #
> -# @boundaries-flush: list of interval boundary values for flush latency
> -# histogram.
> +# @boundaries-flush: list of interval boundary values for flush histogram.
> #
> # Returns: error if device is not found or any boundary arrays are invalid.
> #
> -# Since: 4.0
> +# Since: 4.1
> #
> -# Example: set new histograms for all io types with intervals
> +# Example: set new latency histograms for all io types with intervals
> # [0, 10), [10, 50), [50, 100), [100, +inf):
> #
> -# -> { "execute": "block-latency-histogram-set",
> +# -> { "execute": "block-histogram-set",
> # "arguments": { "id": "drive0",
> +# "type":"latency",
> # "boundaries": [10, 50, 100] } }
> # <- { "return": {} }
> #
> -# Example: set new histogram only for write, other histograms will remain
> -# not changed (or not created):
> +# Example: set new latency histogram only for write, other histograms will
> +# remain not changed (or not created):
> #
> -# -> { "execute": "block-latency-histogram-set",
> +# -> { "execute": "block-histogram-set",
> # "arguments": { "id": "drive0",
> +# "type":"latency",
> # "boundaries-write": [10, 50, 100] } }
> # <- { "return": {} }
> #
> @@ -613,20 +628,23 @@
> # read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
> # write: [0, 1000), [1000, 5000), [5000, +inf)
> #
> -# -> { "execute": "block-latency-histogram-set",
> +# -> { "execute": "block-histogram-set",
> # "arguments": { "id": "drive0",
> +# "type":"latency",
> # "boundaries": [10, 50, 100],
> # "boundaries-write": [1000, 5000] } }
> # <- { "return": {} }
> #
> # Example: remove all latency histograms:
> #
> -# -> { "execute": "block-latency-histogram-set",
> -# "arguments": { "id": "drive0" } }
> +# -> { "execute": "block-histogram-set",
> +# "arguments": { "id": "drive0"
> +# "type":"latency"} }
> # <- { "return": {} }
> ##
> -{ 'command': 'block-latency-histogram-set',
> +{ 'command': 'block-histogram-set',
> 'data': {'id': 'str',
> + 'type': 'BlockHistogramType',
> '*boundaries': ['uint64'],
> '*boundaries-read': ['uint64'],
> '*boundaries-write': ['uint64'],
> @@ -912,11 +930,11 @@
> # @timed_stats: Statistics specific to the set of previously defined
> # intervals of time (Since 2.5)
> #
> -# @rd_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
> +# @rd_latency_histogram: @BlockHistogramInfo. (Since 4.1)
> #
> -# @wr_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
> +# @wr_latency_histogram: @BlockHistogramInfo. (Since 4.1)
> #
> -# @flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)
> +# @flush_latency_histogram: @BlockHistogramInfo. (Since 4.1)
> #
> # Since: 0.14.0
> ##
> @@ -931,9 +949,12 @@
> 'invalid_wr_operations': 'int', 'invalid_flush_operations': 'int',
> 'account_invalid': 'bool', 'account_failed': 'bool',
> 'timed_stats': ['BlockDeviceTimedStats'],
> - '*rd_latency_histogram': 'BlockLatencyHistogramInfo',
> - '*wr_latency_histogram': 'BlockLatencyHistogramInfo',
> - '*flush_latency_histogram': 'BlockLatencyHistogramInfo' } }
> + '*rd_latency_histogram': 'BlockHistogramInfo',
> + '*wr_latency_histogram': 'BlockHistogramInfo',
> + '*flush_latency_histogram': 'BlockHistogramInfo',
> + '*rd_size_histogram': 'BlockHistogramInfo',
> + '*wr_size_histogram': 'BlockHistogramInfo',
> + '*flush_size_histogram': 'BlockHistogramInfo' } }
>
> ##
> # @BlockStats:
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH V2 0/3] Add block size histogram qapi interface
@ 2019-06-24 6:49 zhenwei pi
0 siblings, 0 replies; 6+ messages in thread
From: zhenwei pi @ 2019-06-24 6:49 UTC (permalink / raw)
To: kwolf, mreitz, eblake; +Cc: fam, qemu-block, vsementsov, qemu-devel, pizhenwei
Modify command 'block-latency-histogram-set' to make block histogram
interface common to use. And support block size histogram.
Thanks to Eric Blake&Vladimir Sementsov-Ogievskiy for the suggestions.
This command has been tested for half year on QEMU-2.12, and we found
that 3K+ virtual machines write 25GB/s totally, the block size
histogram like following:
0 ~ 8k: 58% ~ 62%
8k ~ 32k: 10% ~ 12%
32k ~ 128k: 2% ~ 3%
128K ~ 512K: 24% ~ 26%
512K ~ : ...
And the histogram data help us to optimise backend distributed
storage.
zhenwei pi (3):
block/accounting: rename struct BlockLatencyHistogram
block/accounting: introduce block size histogram
qapi: make block histogram interface common
block/accounting.c | 62 ++++++++++++++++++++--------
block/qapi.c | 32 ++++++++------
blockdev.c | 33 +++++++++++----
include/block/accounting.h | 13 +++---
qapi/block-core.json | 101 +++++++++++++++++++++++++++------------------
5 files changed, 158 insertions(+), 83 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-25 11:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-08 9:32 [Qemu-devel] [PATCH v2 0/3] Add block size histogram qapi interface zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 1/3] block/accounting: rename struct BlockLatencyHistogram zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 2/3] block/accounting: introduce block size histogram zhenwei pi
2019-07-08 9:32 ` [Qemu-devel] [PATCH v2 3/3] qapi: make block histogram interface common zhenwei pi
2019-07-25 11:53 ` Vladimir Sementsov-Ogievskiy
-- strict thread matches above, loose matches on Subject: below --
2019-06-24 6:49 [Qemu-devel] [PATCH V2 0/3] Add block size histogram qapi interface zhenwei pi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).