Re: [Qemu-block] [PATCH v3 08/21] block: Compute minimum, maximum and average I/O latencies

2015-10-23 Thread Stefan Hajnoczi
On Thu, Oct 22, 2015 at 11:11:18AM +0300, Alberto Garcia wrote:
> +struct BlockAcctTimedStats {
> +TimedAverage latency[BLOCK_MAX_IOTYPE];
> +unsigned interval_length;

/* in seconds */ would be nice here so the units are clear.  Or even
interval_length_secs.



[Qemu-block] [PATCH v3 08/21] block: Compute minimum, maximum and average I/O latencies

2015-10-22 Thread Alberto Garcia
This patch keeps track of the minimum, maximum and average latencies
of I/O operations during a certain interval of time.

The values are exposed in the BlockDeviceTimedStats structure.

An option to define the intervals to collect these statistics will be
added in a separate patch.

Signed-off-by: Alberto Garcia 
---
 block/accounting.c | 43 ++
 block/block-backend.c  |  1 +
 block/qapi.c   | 28 +
 include/block/accounting.h | 14 +
 qapi/block-core.json   | 52 +-
 qmp-commands.hx| 31 +++
 6 files changed, 168 insertions(+), 1 deletion(-)

diff --git a/block/accounting.c b/block/accounting.c
index 923aeaf..61de8ce 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -35,6 +35,39 @@ void block_acct_init(BlockAcctStats *stats, bool 
account_invalid,
 stats->account_failed = account_failed;
 }
 
+void block_acct_cleanup(BlockAcctStats *stats)
+{
+BlockAcctTimedStats *s, *next;
+QSLIST_FOREACH_SAFE(s, >intervals, entries, next) {
+g_free(s);
+}
+}
+
+void block_acct_add_interval(BlockAcctStats *stats, unsigned interval_length)
+{
+BlockAcctTimedStats *s;
+unsigned i;
+
+s = g_new0(BlockAcctTimedStats, 1);
+s->interval_length = interval_length;
+QSLIST_INSERT_HEAD(>intervals, s, entries);
+
+for (i = 0; i < BLOCK_MAX_IOTYPE; i++) {
+timed_average_init(>latency[i], clock_type,
+   (uint64_t) interval_length * 
NANOSECONDS_PER_SECOND);
+}
+}
+
+BlockAcctTimedStats *block_acct_interval_next(BlockAcctStats *stats,
+  BlockAcctTimedStats *s)
+{
+if (s == NULL) {
+return QSLIST_FIRST(>intervals);
+} else {
+return QSLIST_NEXT(s, entries);
+}
+}
+
 void block_acct_start(BlockAcctStats *stats, BlockAcctCookie *cookie,
   int64_t bytes, enum BlockAcctType type)
 {
@@ -47,6 +80,7 @@ void block_acct_start(BlockAcctStats *stats, BlockAcctCookie 
*cookie,
 
 void block_acct_done(BlockAcctStats *stats, BlockAcctCookie *cookie)
 {
+BlockAcctTimedStats *s;
 int64_t time_ns = qemu_clock_get_ns(clock_type);
 int64_t latency_ns = time_ns - cookie->start_time_ns;
 
@@ -56,6 +90,10 @@ void block_acct_done(BlockAcctStats *stats, BlockAcctCookie 
*cookie)
 stats->nr_ops[cookie->type]++;
 stats->total_time_ns[cookie->type] += latency_ns;
 stats->last_access_time_ns = time_ns;
+
+QSLIST_FOREACH(s, >intervals, entries) {
+timed_average_account(>latency[cookie->type], latency_ns);
+}
 }
 
 void block_acct_failed(BlockAcctStats *stats, BlockAcctCookie *cookie)
@@ -65,11 +103,16 @@ void block_acct_failed(BlockAcctStats *stats, 
BlockAcctCookie *cookie)
 stats->failed_ops[cookie->type]++;
 
 if (stats->account_failed) {
+BlockAcctTimedStats *s;
 int64_t time_ns = qemu_clock_get_ns(clock_type);
 int64_t latency_ns = time_ns - cookie->start_time_ns;
 
 stats->total_time_ns[cookie->type] += latency_ns;
 stats->last_access_time_ns = time_ns;
+
+QSLIST_FOREACH(s, >intervals, entries) {
+timed_average_account(>latency[cookie->type], latency_ns);
+}
 }
 }
 
diff --git a/block/block-backend.c b/block/block-backend.c
index 10e4d71..c3490fb 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -176,6 +176,7 @@ static void blk_delete(BlockBackend *blk)
 }
 g_free(blk->name);
 drive_info_del(blk->legacy_dinfo);
+block_acct_cleanup(>stats);
 g_free(blk);
 }
 
diff --git a/block/qapi.c b/block/qapi.c
index 56c8139..4baf6e1 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -346,6 +346,7 @@ static BlockStats *bdrv_query_stats(const BlockDriverState 
*bs,
 s->stats = g_malloc0(sizeof(*s->stats));
 if (bs->blk) {
 BlockAcctStats *stats = blk_get_stats(bs->blk);
+BlockAcctTimedStats *ts = NULL;
 
 s->stats->rd_bytes = stats->nr_bytes[BLOCK_ACCT_READ];
 s->stats->wr_bytes = stats->nr_bytes[BLOCK_ACCT_WRITE];
@@ -375,6 +376,33 @@ static BlockStats *bdrv_query_stats(const BlockDriverState 
*bs,
 
 s->stats->account_invalid = stats->account_invalid;
 s->stats->account_failed = stats->account_failed;
+
+while ((ts = block_acct_interval_next(stats, ts))) {
+BlockDeviceTimedStatsList *timed_stats =
+g_malloc0(sizeof(*timed_stats));
+BlockDeviceTimedStats *dev_stats = g_malloc0(sizeof(*dev_stats));
+timed_stats->next = s->stats->timed_stats;
+timed_stats->value = dev_stats;
+s->stats->timed_stats = timed_stats;
+
+TimedAverage *rd = >latency[BLOCK_ACCT_READ];
+TimedAverage *wr = >latency[BLOCK_ACCT_WRITE];
+TimedAverage *fl = >latency[BLOCK_ACCT_FLUSH];
+
+