[Qemu-devel] [PATCH] Improve accuracy of block migration bandwidth calculation

[Avishay Traeger]

The current bandwidth calculation looks like this:
(block_mig_state.reads * BLOCK_SIZE)/ block_mig_state.total_time

"total_time" is currently the sum of the read request latencies.  This is
not very accurate because block migration uses aio and so several requests
can be submitted at once.  Bandwidth should be computed with wall-clock
time, not by adding the latencies.  In this case, "total_time" has a higher
value than it should, and so the computed bandwidth is lower than it is in
reality.  This means that migration can take longer than it needs to.

However, we don't want to use pure wall-clock time here.  We are computing
bandwidth in the asynchronous phase, where the migration repeatedly wakes
up and sends some aio requests.  The computed bandwidth will be used for
synchronous transfer.

So my solution is to use the total wall-clock time when I/Os are actually
in flight.

I hope my explanation was clear :)

Signed-off-by: Avishay Traeger <address@hidden>
---
 block-migration.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 8218bac..833d25a 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -62,7 +62,6 @@ typedef struct BlkMigBlock {
     QEMUIOVector qiov;
     BlockDriverAIOCB *aiocb;
     int ret;
-    int64_t time;
     QSIMPLEQ_ENTRY(BlkMigBlock) entry;
 } BlkMigBlock;

@@ -77,6 +76,7 @@ typedef struct BlkMigState {
     int64_t total_sector_sum;
     int prev_progress;
     int bulk_completed;
+    int64_t interval_start_time;
     long double total_time;
     int reads;
 } BlkMigState;
@@ -131,12 +131,6 @@ uint64_t blk_mig_bytes_total(void)
     return sum << BDRV_SECTOR_BITS;
 }

-static inline void add_avg_read_time(int64_t time)
-{
-    block_mig_state.reads++;
-    block_mig_state.total_time += time;
-}
-
 static inline long double compute_read_bwidth(void)
 {
     assert(block_mig_state.total_time != 0);
@@ -195,9 +189,13 @@ static void blk_mig_read_cb(void *opaque, int ret)

     blk->ret = ret;

-    blk->time = qemu_get_clock_ns(rt_clock) - blk->time;
+    /* If this is the last outstanding callback, we end the interval */
+    if (block_mig_state.submitted == 1) {
+       block_mig_state.total_time +=
+           (qemu_get_clock_ns(rt_clock) -
block_mig_state.interval_start_time);
+    }

-    add_avg_read_time(blk->time);
+    block_mig_state.reads++;

     QSIMPLEQ_INSERT_TAIL(&block_mig_state.blk_list, blk, entry);
     bmds_set_aio_inflight(blk->bmds, blk->sector, blk->nr_sectors, 0);
@@ -250,7 +248,9 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile
*f,
     blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
     qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);

-    blk->time = qemu_get_clock_ns(rt_clock);
+    /* If there are no outstanding requests, start an interval */
+    if (block_mig_state.submitted == 0)
+       block_mig_state.interval_start_time = qemu_get_clock_ns(rt_clock);

     blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
                                 nr_sectors, blk_mig_read_cb, blk);
@@ -409,7 +409,9 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile
*f,
                 blk->iov.iov_len = nr_sectors * BDRV_SECTOR_SIZE;
                 qemu_iovec_init_external(&blk->qiov, &blk->iov, 1);

-                blk->time = qemu_get_clock_ns(rt_clock);
+               /* If there are no outstanding requests, start an interval */
+               if (block_mig_state.submitted == 0)
+                   block_mig_state.interval_start_time = qemu_get_clock_ns
(rt_clock);

                 blk->aiocb = bdrv_aio_readv(bmds->bs, sector, &blk->qiov,
                                             nr_sectors, blk_mig_read_cb,
blk);
--
1.7.0.4