subject:"\[Qemu\-block\] \[PATCH 2\/2\] qapi\: add block latency histogram interface"

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

2018-02-07 Thread Vladimir Sementsov-Ogievskiy


06.02.2018 22:02, Eric Blake wrote:

On 02/06/2018 12:06 PM, Vladimir Sementsov-Ogievskiy wrote:

06.02.2018 18:50, Eric Blake wrote:

On 02/06/2018 08:07 AM, Vladimir Sementsov-Ogievskiy wrote:

Set (and clear) histogram through new command
block-latency-histogram-set and show new statistics in
query-blockstats results.



The commit message mentioned that you can set and clear histogram 
tracking; how does this interface let you clear things?  By passing 
a 0-length latency array?  If you violate the constraint (pass 
non-ascending points, for example), does the previous latency map 
get wiped out or is it still preserved unchanged?


On error nothing is changed.

By "clear" I mean zeroing statistics, not removing the whole 
histogram. And to zero statistics you can call set with the same 
latency array.
There is no way to remove histogram at all.. We can add 
block-latency-histogram-unset later if needed.


Maybe "set (or restart) histogram collection points" might read 
better? I also don't think we need a new command; if you make 
'latency' optional, then omitting it can serve to stop collecting 
statistics altogether, without needing a new command (then again, if 
you do that, now the command is used to "set, restart, and clear 
histogram collection").




I like the idea, will do.

--
Best regards,
Vladimir

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

2018-02-06 Thread Eric Blake


On 02/06/2018 12:06 PM, Vladimir Sementsov-Ogievskiy wrote:

06.02.2018 18:50, Eric Blake wrote:

On 02/06/2018 08:07 AM, Vladimir Sementsov-Ogievskiy wrote:

Set (and clear) histogram through new command
block-latency-histogram-set and show new statistics in
query-blockstats results.



The commit message mentioned that you can set and clear histogram 
tracking; how does this interface let you clear things?  By passing a 
0-length latency array?  If you violate the constraint (pass 
non-ascending points, for example), does the previous latency map get 
wiped out or is it still preserved unchanged?


On error nothing is changed.

By "clear" I mean zeroing statistics, not removing the whole histogram. 
And to zero statistics you can call set with the same latency array.
There is no way to remove histogram at all.. We can add 
block-latency-histogram-unset later if needed.


Maybe "set (or restart) histogram collection points" might read better? 
I also don't think we need a new command; if you make 'latency' 
optional, then omitting it can serve to stop collecting statistics 
altogether, without needing a new command (then again, if you do that, 
now the command is used to "set, restart, and clear histogram collection").


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

2018-02-06 Thread Vladimir Sementsov-Ogievskiy


06.02.2018 18:50, Eric Blake wrote:

On 02/06/2018 08:07 AM, Vladimir Sementsov-Ogievskiy wrote:

Set (and clear) histogram through new command
block-latency-histogram-set and show new statistics in
query-blockstats results.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/block-core.json | 62 
+++-

  block/qapi.c | 31 ++
  blockdev.c   | 15 +
  3 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8225308904..4706a934d9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -451,6 +451,63 @@
 'status': 'DirtyBitmapStatus'} }
    ##
+# @BlockLatencyHistogramInfo:
+#
+# Block latency histogram.
+#
+# @latency: list of latency points in microseconds. Equals to 
@latency parameter

+#   of last called block-latency-histogram-set.


Second sentence might read better as:

Matches the @latency parameter from the last call to 
block-latency-histogram-set for the given device.



+#
+# @read: list of read-request counts corresponding to latency region.
+#    len(@read) = len(@latency) + 1
+#    @read[0] corresponds to latency region [0, @latency[0])
+#    for 0 < i < len(@latency): @read[i] corresponds to latency 
region

+#    [@latency[i-1], @latency[i])
+#    @read.last_element corresponds to latency region
+#    [@latency.last_element, +inf)
+#
+# @write: list of write-request counts (see @read semantics)
+#
+# @flush: list of flush-request counts (see @read semantics)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockLatencyHistogramInfo',
+  'data': {'latency': ['uint64'],
+   'read': ['uint64'],
+   'write': ['uint64'],
+   'flush': ['uint64'] } }


Okay, I can see how this interface works.


+
+##
+# @block-latency-histogram-set:
+#
+# Add latency histogram to block device. If latency histogram alredy 
exists for


s/latency/a latency/ (twice)
s/alredy/already/

+# the device it will be removed and a new one created. Latency 
histogram may be


s/Latency/The latency/


+# quered through query-blockstats.


s/quered/queried/


+#
+# @device: device name to set latency histogram for.
+#
+# @latency: list of latency points in microseconds. The sequcence 
must be


s/sequcence/sequence/

+#   ascending, elements must be greater than zero. Histogram 
latency

+#   regions would be
+#   [0, @latency[0]), ..., [@latency[i], @latency[i+1]), ...,
+#   [@latency.last_element, +inf)
+#
+# Returns: error if device is not found.
+#
+# Since: 2.12
+#
+# Example:
+#
+# -> { "execute": "block-latency-histogram-set",
+#  "arguments": { "device": "drive0",
+# "latency": [50, 100, 200] } }
+# <- { "return": {} }
+##
+{ 'command': 'block-latency-histogram-set',
+  'data': {'device': 'str', 'latency': ['uint64'] } }


The commit message mentioned that you can set and clear histogram 
tracking; how does this interface let you clear things?  By passing a 
0-length latency array?  If you violate the constraint (pass 
non-ascending points, for example), does the previous latency map get 
wiped out or is it still preserved unchanged?


On error nothing is changed.

By "clear" I mean zeroing statistics, not removing the whole histogram. 
And to zero statistics you can call set with the same latency array.
There is no way to remove histogram at all.. We can add 
block-latency-histogram-unset later if needed.





+
+##
  # @BlockInfo:
  #
  # Block device information.  This structure describes a virtual 
device and

@@ -730,6 +787,8 @@
  # @timed_stats: Statistics specific to the set of previously defined
  #   intervals of time (Since 2.5)
  #
+# @latency-histogram: @BlockLatencyHistogramInfo. (Since 2.12)


I'd mention that this only appears if block-latency-histogram-set has 
been called.


yes




+#
  # Since: 0.14.0
  ##
  { 'struct': 'BlockDeviceStats',
@@ -742,7 +801,8 @@
 'failed_flush_operations': 'int', 
'invalid_rd_operations': 'int',
 'invalid_wr_operations': 'int', 
'invalid_flush_operations': 'int',

 'account_invalid': 'bool', 'account_failed': 'bool',
-   'timed_stats': ['BlockDeviceTimedStats'] } }
+   'timed_stats': ['BlockDeviceTimedStats'],
+   '*latency-histogram': 'BlockLatencyHistogramInfo' } }






--
Best regards,
Vladimir

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

2018-02-06 Thread Eric Blake


On 02/06/2018 08:07 AM, Vladimir Sementsov-Ogievskiy wrote:

Set (and clear) histogram through new command
block-latency-histogram-set and show new statistics in
query-blockstats results.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/block-core.json | 62 +++-
  block/qapi.c | 31 ++
  blockdev.c   | 15 +
  3 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8225308904..4706a934d9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -451,6 +451,63 @@
 'status': 'DirtyBitmapStatus'} }
  
  ##

+# @BlockLatencyHistogramInfo:
+#
+# Block latency histogram.
+#
+# @latency: list of latency points in microseconds. Equals to @latency 
parameter
+#   of last called block-latency-histogram-set.


Second sentence might read better as:

Matches the @latency parameter from the last call to 
block-latency-histogram-set for the given device.



+#
+# @read: list of read-request counts corresponding to latency region.
+#len(@read) = len(@latency) + 1
+#@read[0] corresponds to latency region [0, @latency[0])
+#for 0 < i < len(@latency): @read[i] corresponds to latency region
+#[@latency[i-1], @latency[i])
+#@read.last_element corresponds to latency region
+#[@latency.last_element, +inf)
+#
+# @write: list of write-request counts (see @read semantics)
+#
+# @flush: list of flush-request counts (see @read semantics)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockLatencyHistogramInfo',
+  'data': {'latency': ['uint64'],
+   'read': ['uint64'],
+   'write': ['uint64'],
+   'flush': ['uint64'] } }


Okay, I can see how this interface works.


+
+##
+# @block-latency-histogram-set:
+#
+# Add latency histogram to block device. If latency histogram alredy exists for


s/latency/a latency/ (twice)
s/alredy/already/


+# the device it will be removed and a new one created. Latency histogram may be


s/Latency/The latency/


+# quered through query-blockstats.


s/quered/queried/


+#
+# @device: device name to set latency histogram for.
+#
+# @latency: list of latency points in microseconds. The sequcence must be


s/sequcence/sequence/


+#   ascending, elements must be greater than zero. Histogram latency
+#   regions would be
+#   [0, @latency[0]), ..., [@latency[i], @latency[i+1]), ...,
+#   [@latency.last_element, +inf)
+#
+# Returns: error if device is not found.
+#
+# Since: 2.12
+#
+# Example:
+#
+# -> { "execute": "block-latency-histogram-set",
+#  "arguments": { "device": "drive0",
+# "latency": [50, 100, 200] } }
+# <- { "return": {} }
+##
+{ 'command': 'block-latency-histogram-set',
+  'data': {'device': 'str', 'latency': ['uint64'] } }


The commit message mentioned that you can set and clear histogram 
tracking; how does this interface let you clear things?  By passing a 
0-length latency array?  If you violate the constraint (pass 
non-ascending points, for example), does the previous latency map get 
wiped out or is it still preserved unchanged?



+
+##
  # @BlockInfo:
  #
  # Block device information.  This structure describes a virtual device and
@@ -730,6 +787,8 @@
  # @timed_stats: Statistics specific to the set of previously defined
  #   intervals of time (Since 2.5)
  #
+# @latency-histogram: @BlockLatencyHistogramInfo. (Since 2.12)


I'd mention that this only appears if block-latency-histogram-set has 
been called.



+#
  # Since: 0.14.0
  ##
  { 'struct': 'BlockDeviceStats',
@@ -742,7 +801,8 @@
 'failed_flush_operations': 'int', 'invalid_rd_operations': 'int',
 'invalid_wr_operations': 'int', 'invalid_flush_operations': 'int',
 'account_invalid': 'bool', 'account_failed': 'bool',
-   'timed_stats': ['BlockDeviceTimedStats'] } }
+   'timed_stats': ['BlockDeviceTimedStats'],
+   '*latency-histogram': 'BlockLatencyHistogramInfo' } }
  



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

2018-02-06 Thread Vladimir Sementsov-Ogievskiy

Set (and clear) histogram through new command
block-latency-histogram-set and show new statistics in
query-blockstats results.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/block-core.json | 62 +++-
 block/qapi.c | 31 ++
 blockdev.c   | 15 +
 3 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8225308904..4706a934d9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -451,6 +451,63 @@
'status': 'DirtyBitmapStatus'} }
 
 ##
+# @BlockLatencyHistogramInfo:
+#
+# Block latency histogram.
+#
+# @latency: list of latency points in microseconds. Equals to @latency 
parameter
+#   of last called block-latency-histogram-set.
+#
+# @read: list of read-request counts corresponding to latency region.
+#len(@read) = len(@latency) + 1
+#@read[0] corresponds to latency region [0, @latency[0])
+#for 0 < i < len(@latency): @read[i] corresponds to latency region
+#[@latency[i-1], @latency[i])
+#@read.last_element corresponds to latency region
+#[@latency.last_element, +inf)
+#
+# @write: list of write-request counts (see @read semantics)
+#
+# @flush: list of flush-request counts (see @read semantics)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockLatencyHistogramInfo',
+  'data': {'latency': ['uint64'],
+   'read': ['uint64'],
+   'write': ['uint64'],
+   'flush': ['uint64'] } }
+
+##
+# @block-latency-histogram-set:
+#
+# Add latency histogram to block device. If latency histogram alredy exists for
+# the device it will be removed and a new one created. Latency histogram may be
+# quered through query-blockstats.
+#
+# @device: device name to set latency histogram for.
+#
+# @latency: list of latency points in microseconds. The sequcence must be
+#   ascending, elements must be greater than zero. Histogram latency
+#   regions would be
+#   [0, @latency[0]), ..., [@latency[i], @latency[i+1]), ...,
+#   [@latency.last_element, +inf)
+#
+# Returns: error if device is not found.
+#
+# Since: 2.12
+#
+# Example:
+#
+# -> { "execute": "block-latency-histogram-set",
+#  "arguments": { "device": "drive0",
+# "latency": [50, 100, 200] } }
+# <- { "return": {} }
+##
+{ 'command': 'block-latency-histogram-set',
+  'data': {'device': 'str', 'latency': ['uint64'] } }
+
+##
 # @BlockInfo:
 #
 # Block device information.  This structure describes a virtual device and
@@ -730,6 +787,8 @@
 # @timed_stats: Statistics specific to the set of previously defined
 #   intervals of time (Since 2.5)
 #
+# @latency-histogram: @BlockLatencyHistogramInfo. (Since 2.12)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'BlockDeviceStats',
@@ -742,7 +801,8 @@
'failed_flush_operations': 'int', 'invalid_rd_operations': 'int',
'invalid_wr_operations': 'int', 'invalid_flush_operations': 'int',
'account_invalid': 'bool', 'account_failed': 'bool',
-   'timed_stats': ['BlockDeviceTimedStats'] } }
+   'timed_stats': ['BlockDeviceTimedStats'],
+   '*latency-histogram': 'BlockLatencyHistogramInfo' } }
 
 ##
 # @BlockStats:
diff --git a/block/qapi.c b/block/qapi.c
index fc10f0a565..715ed17a6b 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -389,6 +389,24 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo 
**p_info,
 qapi_free_BlockInfo(info);
 }
 
+static uint64List *uint64_list(uint64_t *list, int size)
+{
+int i;
+uint64List *out_list = NULL;
+uint64List **pout_list = _list;
+
+for (i = 0; i < size; i++) {
+uint64List *entry = g_new(uint64List, 1);
+entry->value = list[i];
+*pout_list = entry;
+pout_list = >next;
+}
+
+*pout_list = NULL;
+
+return out_list;
+}
+
 static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
 {
 BlockAcctStats *stats = blk_get_stats(blk);
@@ -454,6 +472,19 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, 
BlockBackend *blk)
 dev_stats->avg_wr_queue_depth =
 block_acct_queue_depth(ts, BLOCK_ACCT_WRITE);
 }
+
+ds->has_latency_histogram = stats->latency_histogram.points != NULL;
+if (ds->has_latency_histogram) {
+BlockLatencyHistogramInfo *info = g_new0(BlockLatencyHistogramInfo, 1);
+BlockLatencyHistogram *h = >latency_histogram;
+
+ds->latency_histogram = info;
+
+info->latency = uint64_list(h->points, h->size - 1);
+info->read = uint64_list(h->histogram[BLOCK_ACCT_READ], h->size);
+info->write = uint64_list(h->histogram[BLOCK_ACCT_WRITE], h->size);
+info->flush = uint64_list(h->histogram[BLOCK_ACCT_FLUSH], h->size);
+}
 }
 
 static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs,
diff --git a/blockdev.c

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

Re: [Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

[Qemu-block] [PATCH 2/2] qapi: add block latency histogram interface

5 matches

Site Navigation

Mail list logo

Footer information