[Qemu-devel] [PATCH v4 0/1] migration: calculate expected_downtime with ram_bytes_remaining()

[Qemu-devel] [PATCH v4 0/1] migration: calculate expected_downtime with ram_bytes_remaining()

[Balamuruhan S]

V4:

Read the remaining ram after updating dirty pages count and reuse
existing `remaining` field in ram_counters to hold ram_bytes_remaining()

Thank you Laurent for your time and help to investigate and correct my
changes.

V3:

* commit message to be updated with the changes done by the patch since
v1, review comment by David Gibson.

* Included Michael Roth as ``Reported-by:`` for bringing up the concern.

v2:

There is some difference in expected_downtime value due to following
reason,

1. bandwidth and expected_downtime value are calculated in
migration_update_counters() during each iteration from
migration_thread()

2. remaining ram is calculated in qmp_query_migrate() when we actually
call "info migrate"

This v2 patch where bandwidth, expected_downtime and remaining ram are
calculated in migration_update_counters(), retrieve the same value
during
"info migrate". By this approach we get almost close enough value,

(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
capabilities: xbzrle: off rdma-pin-all: off auto-converge: off
zero-blocks: off compress: off events: off postcopy-ram: off x-colo: off
release-ram: off block: off return-path: off pause-before-switchover:
off x-multifd: off dirty-bitmaps: off
Migration status: active
total time: 319737 milliseconds
expected downtime: 1054 milliseconds
setup: 16 milliseconds
transferred ram: 3669862 kbytes
throughput: 108.92 mbps
remaining ram: 14016 kbytes
total ram: 8388864 kbytes
duplicate: 2296276 pages
skipped: 0 pages
normal: 910639 pages
normal bytes: 3642556 kbytes
dirty sync count: 249
page size: 4 kbytes
dirty pages rate: 4626 pages

Calculation:
calculated value = (14016 * 8 ) / 108.92 = 1029.452809401 milliseconds
actual value = 1054 milliseconds

since v1:
use ram_bytes_remaining() instead of dirty_pages_rate * page_size to
calculate expected_downtime to be more accurate.

Regards,
Bala

Balamuruhan S (1):
  migration: calculate expected_downtime with ram_bytes_remaining()

 migration/migration.c | 3 +--
 migration/ram.c       | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v4 1/1] migration: calculate expected_downtime with ram_bytes_remaining()

[Balamuruhan S]

expected_downtime value is not accurate with dirty_pages_rate * page_size,
using ram_bytes_remaining() would yeild it resonable.

consider to read the remaining ram just after having updated the dirty
pages count later migration_bitmap_sync_range() in migration_bitmap_sync()
and reuse the `remaining` field in ram_counters to hold ram_bytes_remaining()
for calculating expected_downtime.

Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Balamuruhan S <bala24@linux.vnet.ibm.com>
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 migration/migration.c | 3 +--
 migration/ram.c       | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index ea9a6cbb87..cb14dadb26 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2739,8 +2739,7 @@ static void migration_update_counters(MigrationState *s,
      * recalculate. 10000 is a small enough number for our purposes
      */
     if (ram_counters.dirty_pages_rate && transferred > 10000) {
-        s->expected_downtime = ram_counters.dirty_pages_rate *
-            qemu_target_page_size() / bandwidth;
+        s->expected_downtime = ram_counters.remaining / bandwidth;
     }
 
     qemu_file_reset_rate_limit(s->to_dst_file);
diff --git a/migration/ram.c b/migration/ram.c
index a500015a2f..a94a2b829e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1159,6 +1159,7 @@ static void migration_bitmap_sync(RAMState *rs)
     RAMBLOCK_FOREACH_MIGRATABLE(block) {
         migration_bitmap_sync_range(rs, block, 0, block->used_length);
     }
+    ram_counters.remaining = ram_bytes_remaining();
     rcu_read_unlock();
     qemu_mutex_unlock(&rs->bitmap_mutex);
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH v4 1/1] migration: calculate expected_downtime with ram_bytes_remaining()

[Dr. David Alan Gilbert]

* Balamuruhan S (bala24@linux.vnet.ibm.com) wrote:
> expected_downtime value is not accurate with dirty_pages_rate * page_size,
> using ram_bytes_remaining() would yeild it resonable.
> 
> consider to read the remaining ram just after having updated the dirty
> pages count later migration_bitmap_sync_range() in migration_bitmap_sync()
> and reuse the `remaining` field in ram_counters to hold ram_bytes_remaining()
> for calculating expected_downtime.
> 
> Reported-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Balamuruhan S <bala24@linux.vnet.ibm.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  migration/migration.c | 3 +--
>  migration/ram.c       | 1 +
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index ea9a6cbb87..cb14dadb26 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2739,8 +2739,7 @@ static void migration_update_counters(MigrationState *s,
>       * recalculate. 10000 is a small enough number for our purposes
>       */
>      if (ram_counters.dirty_pages_rate && transferred > 10000) {
> -        s->expected_downtime = ram_counters.dirty_pages_rate *
> -            qemu_target_page_size() / bandwidth;
> +        s->expected_downtime = ram_counters.remaining / bandwidth;
>      }
>  
>      qemu_file_reset_rate_limit(s->to_dst_file);
> diff --git a/migration/ram.c b/migration/ram.c
> index a500015a2f..a94a2b829e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1159,6 +1159,7 @@ static void migration_bitmap_sync(RAMState *rs)
>      RAMBLOCK_FOREACH_MIGRATABLE(block) {
>          migration_bitmap_sync_range(rs, block, 0, block->used_length);
>      }
> +    ram_counters.remaining = ram_bytes_remaining();
>      rcu_read_unlock();
>      qemu_mutex_unlock(&rs->bitmap_mutex);

OK, that's interesting.
One thing to note is that migration_bitmap_sync isn't just called when
we've run out of dirty pages and need to see if there are any fresh
ones.
In the case where we hit the bandwidth limit, then we call it whenever
we go back through ram_save_pending even though the current 
remaining isn't 0 yet (it's got to be below the threshold for us to
resync) - so you may over estimate a bit?

Anyway, I  think it's worth a go;


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> -- 
> 2.14.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK