* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.