* [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 2019-06-24 6:49 ` [Qemu-devel] [PATCH V2 2/3] block/accounting: introduce block size histogram zhenwei pi 0 siblings, 1 reply; 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
* [Qemu-devel] [PATCH V2 2/3] block/accounting: introduce block size histogram 2019-06-24 6:49 [Qemu-devel] [PATCH V2 0/3] Add block size histogram qapi interface zhenwei pi @ 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 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
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 2019-06-24 6:49 ` [Qemu-devel] [PATCH V2 2/3] block/accounting: introduce block size histogram 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).