[Qemu-devel] [PATCH 0/4] migration stats fixes

[Qemu-devel] [PATCH 0/4] migration stats fixes

[Juan Quintela]

Hi

migration expected_downtime calculation was removed on commit
e4ed1541ac9413eac494a03532e34beaf8a7d1c5.

We add the calculation back.  Before doing the calculation we do:

- expected_downtime intial value is max_downtime.  Much, much better
  intial value than 0.

- we move when we measure the time.  We used to measure how much it
  took "before" we really sent the data.

- we introduce sleep_time concept.  While we are sleeping because we
  have sent all the allowed data for this second we shouldn't be
  accounting that time as "sending".

- last patch just introduces the re-calculation of expected_downtime.

It just changes the stats value.  Well, patchs 2 & 3 change the
bandwidth calculation for migration, but I think that we were
undercalculating it enough than it was a bug.

Without the 2 & 3 patches, the "expected_downtime" for an idle gust
was calculated as 80ms (with 30 ms default target value), and we ended
having a downtime of around 15ms.

With this patches applied, we calculate an expected downtime of around
15ms or so, and then we spent aroqund 18ms on downtime.  Notice that
we only calculate how much it takes to sent the rest of the RAM, it
just happens that there is some more data to sent that what we are calculating.

Review, please.

Later, Juan.


The following changes since commit 8a55ebf01507ab73cc458cfcd5b9cb856aba0b9e:

  Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2013-01-31 19:37:33 -0600)

are available in the git repository at:


  git://repo.or.cz/qemu/quintela.git stats.next

for you to fetch changes up to 791128495e3546ccc88dd037ea4dfd31eca14a56:

  migration: calculate expected_downtime (2013-02-01 13:22:37 +0100)

----------------------------------------------------------------
Juan Quintela (4):
      migration: change initial value of expected_downtime
      migration: calculate end time after we have sent the data
      migration: don't account sleep time for calculating bandwidth
      migration: calculate expected_downtime

 arch_init.c                   |  1 +
 include/migration/migration.h |  1 +
 migration.c                   | 15 +++++++++++++--
 3 files changed, 15 insertions(+), 2 deletions(-)

[Qemu-devel] [PATCH 1/4] migration: change initial value of expected_downtime

[Juan Quintela]

0 is a very bad initial value, what we are trying to get is
max_downtime, so that is a much better estimation.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration.c b/migration.c
index 77c1971..d86946e 100644
--- a/migration.c
+++ b/migration.c
@@ -782,6 +782,8 @@ void migrate_fd_connect(MigrationState *s)
     s->buffer = NULL;
     s->buffer_size = 0;
     s->buffer_capacity = 0;
+    /* This is a best 1st approximation. ns to ms */
+    s->expected_downtime = max_downtime/1000000;

     s->xfer_limit = s->bandwidth_limit / XFER_LIMIT_RATIO;
     s->complete = false;
-- 
1.8.1

[Qemu-devel] [PATCH 2/4] migration: calculate end time after we have sent the data

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index d86946e..67abd12 100644
--- a/migration.c
+++ b/migration.c
@@ -681,7 +681,7 @@ static void *buffered_file_thread(void *opaque)
     qemu_mutex_unlock_iothread();

     while (true) {
-        int64_t current_time = qemu_get_clock_ms(rt_clock);
+        int64_t current_time;
         uint64_t pending_size;

         qemu_mutex_lock_iothread();
@@ -735,6 +735,7 @@ static void *buffered_file_thread(void *opaque)
             }
         }
         qemu_mutex_unlock_iothread();
+        current_time = qemu_get_clock_ms(rt_clock);
         if (current_time >= initial_time + BUFFER_DELAY) {
             uint64_t transferred_bytes = s->bytes_xfer;
             uint64_t time_spent = current_time - initial_time;
-- 
1.8.1

[Qemu-devel] [PATCH 3/4] migration: don't account sleep time for calculating bandwidth

[Juan Quintela]

While we are sleeping we are not sending, so we should not use that
time to estimate our bandwidth.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 67abd12..64e75ca 100644
--- a/migration.c
+++ b/migration.c
@@ -666,6 +666,7 @@ static void *buffered_file_thread(void *opaque)
 {
     MigrationState *s = opaque;
     int64_t initial_time = qemu_get_clock_ms(rt_clock);
+    int64_t sleep_time = 0;
     int64_t max_size = 0;
     bool last_round = false;
     int ret;
@@ -738,7 +739,7 @@ static void *buffered_file_thread(void *opaque)
         current_time = qemu_get_clock_ms(rt_clock);
         if (current_time >= initial_time + BUFFER_DELAY) {
             uint64_t transferred_bytes = s->bytes_xfer;
-            uint64_t time_spent = current_time - initial_time;
+            uint64_t time_spent = current_time - initial_time - sleep_time;
             double bandwidth = transferred_bytes / time_spent;
             max_size = bandwidth * migrate_max_downtime() / 1000000;

@@ -747,11 +748,13 @@ static void *buffered_file_thread(void *opaque)
                     transferred_bytes, time_spent, bandwidth, max_size);

             s->bytes_xfer = 0;
+            sleep_time = 0;
             initial_time = current_time;
         }
         if (!last_round && (s->bytes_xfer >= s->xfer_limit)) {
             /* usleep expects microseconds */
             g_usleep((initial_time + BUFFER_DELAY - current_time)*1000);
+            sleep_time += qemu_get_clock_ms(rt_clock) - current_time;
         }
         ret = buffered_flush(s);
         if (ret < 0) {
-- 
1.8.1

[Qemu-devel] [PATCH 4/4] migration: calculate expected_downtime

[Juan Quintela]

We removed the calculation in commit e4ed1541ac9413eac494a03532e34beaf8a7d1c5

Now we add it back.  We need to create dirty_bytes_rate because we
can't include cpu-all.h from migration.c, and there is no other way to
include TARGET_PAGE_SIZE.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 arch_init.c                   | 1 +
 include/migration/migration.h | 1 +
 migration.c                   | 5 +++++
 3 files changed, 7 insertions(+)

diff --git a/arch_init.c b/arch_init.c
index dada6de..634490a 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -414,6 +414,7 @@ static void migration_bitmap_sync(void)
     if (end_time > start_time + 1000) {
         s->dirty_pages_rate = num_dirty_pages_period * 1000
             / (end_time - start_time);
+        s->dirty_bytes_rate = s->dirty_pages_rate * TARGET_PAGE_SIZE;
         start_time = end_time;
         num_dirty_pages_period = 0;
     }
diff --git a/include/migration/migration.h b/include/migration/migration.h
index a8c9639..d121409 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -51,6 +51,7 @@ struct MigrationState
     int64_t downtime;
     int64_t expected_downtime;
     int64_t dirty_pages_rate;
+    int64_t dirty_bytes_rate;
     bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
     int64_t xbzrle_cache_size;
     bool complete;
diff --git a/migration.c b/migration.c
index 64e75ca..4eca42e 100644
--- a/migration.c
+++ b/migration.c
@@ -746,6 +746,11 @@ static void *buffered_file_thread(void *opaque)
             DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
                     " bandwidth %g max_size %" PRId64 "\n",
                     transferred_bytes, time_spent, bandwidth, max_size);
+            /* if we haven't sent anything, we don't want to recalculate
+               10000 is a small enough number for our purposes */
+            if (s->dirty_bytes_rate && transferred_bytes > 10000) {
+                s->expected_downtime = s->dirty_bytes_rate / bandwidth;
+            }

             s->bytes_xfer = 0;
             sleep_time = 0;
-- 
1.8.1