[Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate'

Chao Fan posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170308082819.12196-1-fanc.fnst@cn.fujitsu.com
Test checkpatch passed
Test docker passed
hmp.c                         |  4 ++++
include/migration/migration.h |  1 +
include/qemu/bitmap.h         | 17 ++++++++++++++++
migration/migration.c         |  2 ++
migration/ram.c               | 45 +++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json              |  4 ++++
6 files changed, 73 insertions(+)
[Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate'
Posted by Chao Fan 7 years, 1 month ago
Auto-converge aims to accelerate migration by slowing down the
generation of dirty pages. But user doesn't know how to determine the
throttle value, so, a new item "inst-dirty-pages-rate" in "info migrate"
would be helpful for user's determination.

Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com>

---
v2:
  Update the way to caculate the time.
  Add tag '(since 2.9)' in documentation of qapi-schema.json
---
 hmp.c                         |  4 ++++
 include/migration/migration.h |  1 +
 include/qemu/bitmap.h         | 17 ++++++++++++++++
 migration/migration.c         |  2 ++
 migration/ram.c               | 45 +++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json              |  4 ++++
 6 files changed, 73 insertions(+)

diff --git a/hmp.c b/hmp.c
index 2bc4f06..c7892ea 100644
--- a/hmp.c
+++ b/hmp.c
@@ -219,6 +219,10 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
             monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n",
                            info->ram->dirty_pages_rate);
         }
+        if (info->ram->inst_dirty_pages_rate) {
+            monitor_printf(mon, "inst dirty pages rate: %" PRIu64 " bytes/s\n",
+                           info->ram->inst_dirty_pages_rate);
+        }
         if (info->ram->postcopy_requests) {
             monitor_printf(mon, "postcopy request count: %" PRIu64 "\n",
                            info->ram->postcopy_requests);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 1735d66..95f0453 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -164,6 +164,7 @@ struct MigrationState
     int64_t downtime;
     int64_t expected_downtime;
     int64_t dirty_pages_rate;
+    int64_t inst_dirty_pages_rate;
     int64_t dirty_bytes_rate;
     bool enabled_capabilities[MIGRATION_CAPABILITY__MAX];
     int64_t xbzrle_cache_size;
diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h
index 63ea2d0..dc99f9b 100644
--- a/include/qemu/bitmap.h
+++ b/include/qemu/bitmap.h
@@ -235,4 +235,21 @@ static inline unsigned long *bitmap_zero_extend(unsigned long *old,
     return new;
 }
 
+static inline unsigned long bitmap_weight(const unsigned long *src, long nbits)
+{
+    unsigned long i, count = 0, nlong = nbits / BITS_PER_LONG;
+
+    if (small_nbits(nbits)) {
+        return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
+    }
+    for (i = 0; i < nlong; i++) {
+        count += hweight_long(src[i]);
+    }
+    if (nbits % BITS_PER_LONG) {
+        count += hweight_long(src[i] & BITMAP_LAST_WORD_MASK(nbits));
+    }
+
+    return count;
+}
+
 #endif /* BITMAP_H */
diff --git a/migration/migration.c b/migration/migration.c
index c6ae69d..18fc2ec 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -644,6 +644,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
     if (s->state != MIGRATION_STATUS_COMPLETED) {
         info->ram->remaining = ram_bytes_remaining();
         info->ram->dirty_pages_rate = s->dirty_pages_rate;
+        info->ram->inst_dirty_pages_rate = s->inst_dirty_pages_rate;
     }
 }
 
@@ -1099,6 +1100,7 @@ MigrationState *migrate_init(const MigrationParams *params)
     s->downtime = 0;
     s->expected_downtime = 0;
     s->dirty_pages_rate = 0;
+    s->inst_dirty_pages_rate = 0;
     s->dirty_bytes_rate = 0;
     s->setup_time = 0;
     s->dirty_sync_count = 0;
diff --git a/migration/ram.c b/migration/ram.c
index f289fcd..7b440fd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -44,6 +44,7 @@
 #include "exec/ram_addr.h"
 #include "qemu/rcu_queue.h"
 #include "migration/colo.h"
+#include "hw/boards.h"
 
 static int dirty_rate_high_cnt;
 
@@ -590,6 +591,7 @@ static int64_t bytes_xfer_prev;
 static int64_t num_dirty_pages_period;
 static uint64_t xbzrle_cache_miss_prev;
 static uint64_t iterations_prev;
+static int64_t dirty_pages_time_prev;
 
 static void migration_bitmap_sync_init(void)
 {
@@ -598,6 +600,47 @@ static void migration_bitmap_sync_init(void)
     num_dirty_pages_period = 0;
     xbzrle_cache_miss_prev = 0;
     iterations_prev = 0;
+    dirty_pages_time_prev = 0;
+}
+
+static void migration_inst_rate(void)
+{
+    int64_t dirty_pages_time_now;
+    if (!dirty_pages_time_prev) {
+        dirty_pages_time_prev = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    }
+    dirty_pages_time_now = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    if (dirty_pages_time_now > dirty_pages_time_prev + 1000) {
+        RAMBlock *block;
+        MigrationState *s = migrate_get_current();
+        int64_t inst_dirty_pages = 0;
+        int64_t i;
+        unsigned long *num;
+        unsigned long len = 0;
+
+        rcu_read_lock();
+        DirtyMemoryBlocks *blocks = atomic_rcu_read(
+                         &ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]);
+        QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
+            if (len == 0) {
+                len = block->offset;
+            }
+            len += block->used_length;
+        }
+        ram_addr_t idx = (len >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE;
+        if (((len >> TARGET_PAGE_BITS) % DIRTY_MEMORY_BLOCK_SIZE) != 0) {
+            idx++;
+        }
+        for (i = 0; i < idx; i++) {
+            num = blocks->blocks[i];
+            inst_dirty_pages += bitmap_weight(num, DIRTY_MEMORY_BLOCK_SIZE);
+        }
+        rcu_read_unlock();
+
+        s->inst_dirty_pages_rate = inst_dirty_pages * TARGET_PAGE_SIZE *
+                    1000 / (dirty_pages_time_now - dirty_pages_time_prev);
+    }
+    dirty_pages_time_prev = dirty_pages_time_now;
 }
 
 static void migration_bitmap_sync(void)
@@ -621,6 +664,8 @@ static void migration_bitmap_sync(void)
     trace_migration_bitmap_sync_start();
     memory_global_dirty_log_sync();
 
+    migration_inst_rate();
+
     qemu_mutex_lock(&migration_bitmap_mutex);
     rcu_read_lock();
     QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
diff --git a/qapi-schema.json b/qapi-schema.json
index e9a6364..bf00717 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -575,12 +575,16 @@
 # @postcopy-requests: The number of page requests received from the destination
 #        (since 2.7)
 #
+# @inst-dirty-pages-rate: The number of bytes new dirtied pages by second.
+#        (since 2.9)
+#
 # Since: 0.14.0
 ##
 { 'struct': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
            'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
            'normal-bytes': 'int', 'dirty-pages-rate' : 'int',
+           'inst-dirty-pages-rate' : 'int',
            'mbps' : 'number', 'dirty-sync-count' : 'int',
            'postcopy-requests' : 'int' } }
 
-- 
2.9.3




Re: [Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate'
Posted by Daniel P. Berrange 7 years, 1 month ago
On Wed, Mar 08, 2017 at 04:28:19PM +0800, Chao Fan wrote:
> Auto-converge aims to accelerate migration by slowing down the
> generation of dirty pages. But user doesn't know how to determine the
> throttle value, so, a new item "inst-dirty-pages-rate" in "info migrate"
> would be helpful for user's determination.

The "info migrate" command already reports a "dirty-pages-rate" value.

Maybe I'm mis-understanding what you're calculcating, this this proposal
looks the same, except reporting in bytes rather than page counts.

QEMU in fact already records the bytes count internally too in the
'dirty_pages_bytes' parameter which is calculated from taking
'dirty_pages_size * TARGET_PAGE_SIZE'.

So I wonder if we can just export the existing dirty-pages-bytes
value in info migrate, and avoid needing this new code here:

> +static void migration_inst_rate(void)
> +{
> +    int64_t dirty_pages_time_now;
> +    if (!dirty_pages_time_prev) {
> +        dirty_pages_time_prev = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    }
> +    dirty_pages_time_now = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    if (dirty_pages_time_now > dirty_pages_time_prev + 1000) {
> +        RAMBlock *block;
> +        MigrationState *s = migrate_get_current();
> +        int64_t inst_dirty_pages = 0;
> +        int64_t i;
> +        unsigned long *num;
> +        unsigned long len = 0;
> +
> +        rcu_read_lock();
> +        DirtyMemoryBlocks *blocks = atomic_rcu_read(
> +                         &ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]);
> +        QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
> +            if (len == 0) {
> +                len = block->offset;
> +            }
> +            len += block->used_length;
> +        }
> +        ram_addr_t idx = (len >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE;
> +        if (((len >> TARGET_PAGE_BITS) % DIRTY_MEMORY_BLOCK_SIZE) != 0) {
> +            idx++;
> +        }
> +        for (i = 0; i < idx; i++) {
> +            num = blocks->blocks[i];
> +            inst_dirty_pages += bitmap_weight(num, DIRTY_MEMORY_BLOCK_SIZE);
> +        }
> +        rcu_read_unlock();
> +
> +        s->inst_dirty_pages_rate = inst_dirty_pages * TARGET_PAGE_SIZE *
> +                    1000 / (dirty_pages_time_now - dirty_pages_time_prev);
> +    }
> +    dirty_pages_time_prev = dirty_pages_time_now;
>  }

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate'
Posted by Chao Fan 7 years, 1 month ago
On Wed, Mar 08, 2017 at 01:45:59PM +0000, Daniel P. Berrange wrote:
>On Wed, Mar 08, 2017 at 04:28:19PM +0800, Chao Fan wrote:
>> Auto-converge aims to accelerate migration by slowing down the
>> generation of dirty pages. But user doesn't know how to determine the
>> throttle value, so, a new item "inst-dirty-pages-rate" in "info migrate"
>> would be helpful for user's determination.
Hi Daniel,

Thank you for your reply.
>
>The "info migrate" command already reports a "dirty-pages-rate" value.
>
>Maybe I'm mis-understanding what you're calculcating, this this proposal
>looks the same, except reporting in bytes rather than page counts.
>
>QEMU in fact already records the bytes count internally too in the
>'dirty_pages_bytes' parameter which is calculated from taking
>'dirty_pages_size * TARGET_PAGE_SIZE'.
>
>So I wonder if we can just export the existing dirty-pages-bytes
>value in info migrate, and avoid needing this new code here:
>
It's different, inst-dirty-pages-rate in this patch is greater than
or equal to dirty-pages-bytes. Because in function
cpu_physical_memory_sync_dirty_bitmap, file include/exec/ram_addr.h:

if (src[idx][offset]) {
    unsigned long bits = atomic_xchg(&src[idx][offset], 0);
    unsigned long new_dirty;
    new_dirty = ~dest[k];
    dest[k] |= bits;
    new_dirty &= bits;
    num_dirty += ctpopl(new_dirty);
}

After these codes, only the pages not dirtied in bitmap(dest), but dirtied
in dirty_memory[DIRTY_MEMORY_MIGRATION] will be calculated. For example:
When ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] = 0b00001111,
and atomic_rcu_read(&migration_bitmap_rcu)->bmap = 0b00000011,
the new_dirty will be 0b00001100, and this function will return 2 but not
4 which is expected.

Thanks,
Chao Fan

>> +static void migration_inst_rate(void)
>> +{
>> +    int64_t dirty_pages_time_now;
>> +    if (!dirty_pages_time_prev) {
>> +        dirty_pages_time_prev = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> +    }
>> +    dirty_pages_time_now = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
>> +    if (dirty_pages_time_now > dirty_pages_time_prev + 1000) {
>> +        RAMBlock *block;
>> +        MigrationState *s = migrate_get_current();
>> +        int64_t inst_dirty_pages = 0;
>> +        int64_t i;
>> +        unsigned long *num;
>> +        unsigned long len = 0;
>> +
>> +        rcu_read_lock();
>> +        DirtyMemoryBlocks *blocks = atomic_rcu_read(
>> +                         &ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]);
>> +        QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
>> +            if (len == 0) {
>> +                len = block->offset;
>> +            }
>> +            len += block->used_length;
>> +        }
>> +        ram_addr_t idx = (len >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE;
>> +        if (((len >> TARGET_PAGE_BITS) % DIRTY_MEMORY_BLOCK_SIZE) != 0) {
>> +            idx++;
>> +        }
>> +        for (i = 0; i < idx; i++) {
>> +            num = blocks->blocks[i];
>> +            inst_dirty_pages += bitmap_weight(num, DIRTY_MEMORY_BLOCK_SIZE);
>> +        }
>> +        rcu_read_unlock();
>> +
>> +        s->inst_dirty_pages_rate = inst_dirty_pages * TARGET_PAGE_SIZE *
>> +                    1000 / (dirty_pages_time_now - dirty_pages_time_prev);
>> +    }
>> +    dirty_pages_time_prev = dirty_pages_time_now;
>>  }
>
>Regards,
>Daniel
>-- 
>|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
>|: http://libvirt.org              -o-             http://virt-manager.org :|
>|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|
>
>



Re: [Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate'
Posted by Daniel P. Berrange 7 years, 1 month ago
On Thu, Mar 09, 2017 at 06:05:38PM +0800, Chao Fan wrote:
> On Wed, Mar 08, 2017 at 01:45:59PM +0000, Daniel P. Berrange wrote:
> >On Wed, Mar 08, 2017 at 04:28:19PM +0800, Chao Fan wrote:
> >> Auto-converge aims to accelerate migration by slowing down the
> >> generation of dirty pages. But user doesn't know how to determine the
> >> throttle value, so, a new item "inst-dirty-pages-rate" in "info migrate"
> >> would be helpful for user's determination.
> Hi Daniel,
> 
> Thank you for your reply.
> >
> >The "info migrate" command already reports a "dirty-pages-rate" value.
> >
> >Maybe I'm mis-understanding what you're calculcating, this this proposal
> >looks the same, except reporting in bytes rather than page counts.
> >
> >QEMU in fact already records the bytes count internally too in the
> >'dirty_pages_bytes' parameter which is calculated from taking
> >'dirty_pages_size * TARGET_PAGE_SIZE'.
> >
> >So I wonder if we can just export the existing dirty-pages-bytes
> >value in info migrate, and avoid needing this new code here:
> >
> It's different, inst-dirty-pages-rate in this patch is greater than
> or equal to dirty-pages-bytes. Because in function
> cpu_physical_memory_sync_dirty_bitmap, file include/exec/ram_addr.h:
> 
> if (src[idx][offset]) {
>     unsigned long bits = atomic_xchg(&src[idx][offset], 0);
>     unsigned long new_dirty;
>     new_dirty = ~dest[k];
>     dest[k] |= bits;
>     new_dirty &= bits;
>     num_dirty += ctpopl(new_dirty);
> }
> 
> After these codes, only the pages not dirtied in bitmap(dest), but dirtied
> in dirty_memory[DIRTY_MEMORY_MIGRATION] will be calculated. For example:
> When ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] = 0b00001111,
> and atomic_rcu_read(&migration_bitmap_rcu)->bmap = 0b00000011,
> the new_dirty will be 0b00001100, and this function will return 2 but not
> 4 which is expected.

Perhaps this is a reason to change how the current dirty-pages-rate and
dirty-pages-bytes values are calculated, rather than adding a new parameter
inst-dirty-pages-rate.

IMHO it will be pretty confusing to have two very similar parameters exposed
in the 'info migration' output, with subtley different calculations behind
them. You can only really understand the difference by looking at QEMU
code which is bad for apps consuming QEMU.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|