[PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[Chuan Zheng]

v9 -> v10:
    rename find_page_matched as find_block_matched
    fix wrong termination condition in find_block_matched
    add review-by for patches

v8 -> v9:
    fix wrong index return of record_ramblock_hash_info
    optimize variable name according to review
    reset dirty_rate as -1
    change returns of compare_page_hash_info to bool

v7 -> v8:
    add atomic_read for dirtyrate status
    add error_report if set dirtyrate state failed
    change returns of save_ramblock_hash and record_ramblock_hash_info to bool
    alloc ramblock dirtyinfo array at one time
    add review-by for patches

v6 -> v7:
    fix minior comments and coding style by review
    add review-by for patches

v5 -> v6:
    fix coding style according to review
    use TARGET_PAGE_SIZE and TARGET_PAGE_BITS instead of self-defined macros
    return start-time and calc-time by qmp command

v4 -> v5:
    fix git apply failed due to meson-build
    add review-by for patches in v3

v3 -> v4:
    use crc32 to get hash result instead of md5
    add DirtyRateStatus to denote calculation status
    add some trace_calls to make it easier to debug
    fix some comments accroding to review

v2 -> v3:
    fix size_t compile warning
    fix codestyle checked by checkpatch.pl

v1 -> v2:
    use g_rand_new() to generate rand_buf
    move RAMBLOCK_FOREACH_MIGRATABLE into migration/ram.h
    add skip_sample_ramblock to filter sampled ramblock
    fix multi-numa vm coredump when query dirtyrate
    rename qapi interface and rename some structures and functions
    succeed to compile by appling each patch
    add test for migrating vm

Sometimes it is neccessary to evaluate dirty page rate before migration.
Users could decide whether to proceed migration based on the evaluation
in case of vm performance loss due to heavy workload.
Unlikey simulating dirtylog sync which could do harm on runnning vm,
we provide a sample-hash method to compare hash results for samping page.
In this way, it would have hardly no impact on vm performance.

Evaluate the dirtypage rate both on running and migration vm.
The VM specifications for migration are as follows:
- VM use 4-K page;
- the number of VCPU is 32;
- the total memory is 32Gigabit;
- use 'mempress' tool to pressurize VM(mempress 4096 1024);
- migration bandwidth is 1GB/s

+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
|                      |  running  |                  migrating                           |
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
| no mempress          |   4MB/s   |          8MB/s      (migrated success)               |
-------------------------------------------------------------------------------------------
| mempress 4096 1024   |  1060MB/s |     456MB/s ~ 1142MB/s (cpu throttle triggered)      |
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
| mempress 4096 4096   |  4114MB/s |     688MB/s ~ 4132MB/s (cpu throttle triggered)      |
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Test dirtyrate by qmp command like this:
1.  virsh qemu-monitor-command [vmname] '{"execute":"calc-dirty-rate", "arguments": {"calc-time": [sleep-time]}}'; 
2.  sleep specific time which is a bit larger than sleep-time
3.  virsh qemu-monitor-command [vmname] '{"execute":"query-dirty-rate"}'

The qmp command returns like this:
{"return":{"status":"measured","dirty-rate":374,"start-time":3718293,"calc-time":1},"id":"libvirt-15"}

Further test dirtyrate by libvirt api like this:
virsh getdirtyrate [vmname] [sleep-time]

Chuan Zheng (12):
  migration/dirtyrate: setup up query-dirtyrate framwork
  migration/dirtyrate: add DirtyRateStatus to denote calculation status
  migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info
  migration/dirtyrate: Add dirtyrate statistics series functions
  migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h
  migration/dirtyrate: Record hash results for each sampled page
  migration/dirtyrate: Compare page hash results for recorded sampled
    page
  migration/dirtyrate: skip sampling ramblock with size below
    MIN_RAMBLOCK_SIZE
  migration/dirtyrate: Implement set_sample_page_period() and
    is_sample_period_valid()
  migration/dirtyrate: Implement calculate_dirtyrate() function
  migration/dirtyrate: Implement
    qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
  migration/dirtyrate: Add trace_calls to make it easier to debug

 migration/dirtyrate.c  | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
 migration/dirtyrate.h  |  70 ++++++++
 migration/meson.build  |   2 +-
 migration/ram.c        |  11 +-
 migration/ram.h        |  10 ++
 migration/trace-events |   8 +
 qapi/migration.json    |  67 ++++++++
 7 files changed, 583 insertions(+), 11 deletions(-)
 create mode 100644 migration/dirtyrate.c
 create mode 100644 migration/dirtyrate.h

-- 
1.8.3.1

[PATCH v10 01/12] migration/dirtyrate: setup up query-dirtyrate framwork

[Chuan Zheng]

Add get_dirtyrate_thread() functions to setup query-dirtyrate
framework.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c | 38 ++++++++++++++++++++++++++++++++++++++
 migration/dirtyrate.h | 29 +++++++++++++++++++++++++++++
 migration/meson.build |  2 +-
 3 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 migration/dirtyrate.c
 create mode 100644 migration/dirtyrate.h

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
new file mode 100644
index 0000000..bf7fd24
--- /dev/null
+++ b/migration/dirtyrate.c
@@ -0,0 +1,38 @@
+/*
+ * Dirtyrate implement code
+ *
+ * Copyright (c) 2020 HUAWEI TECHNOLOGIES CO.,LTD.
+ *
+ * Authors:
+ *  Chuan Zheng <zhengchuan@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "qemu/config-file.h"
+#include "exec/memory.h"
+#include "exec/ramblock.h"
+#include "exec/target_page.h"
+#include "qemu/rcu_queue.h"
+#include "qapi/qapi-commands-migration.h"
+#include "migration.h"
+#include "dirtyrate.h"
+
+static void calculate_dirtyrate(struct DirtyRateConfig config)
+{
+    /* todo */
+    return;
+}
+
+void *get_dirtyrate_thread(void *arg)
+{
+    struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
+
+    calculate_dirtyrate(config);
+
+    return NULL;
+}
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
new file mode 100644
index 0000000..5be9714
--- /dev/null
+++ b/migration/dirtyrate.h
@@ -0,0 +1,29 @@
+/*
+ *  Dirtyrate common functions
+ *
+ *  Copyright (c) 2020 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ *  Authors:
+ *  Chuan Zheng <zhengchuan@huawei.com>
+ *
+ *  This work is licensed under the terms of the GNU GPL, version 2 or later.
+ *  See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_MIGRATION_DIRTYRATE_H
+#define QEMU_MIGRATION_DIRTYRATE_H
+
+/*
+ * Sample 512 pages per GB as default.
+ * TODO: Make it configurable.
+ */
+#define DIRTYRATE_DEFAULT_SAMPLE_PAGES            512
+
+struct DirtyRateConfig {
+    uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
+    int64_t sample_period_seconds; /* time duration between two sampling */
+};
+
+void *get_dirtyrate_thread(void *arg);
+#endif
+
diff --git a/migration/meson.build b/migration/meson.build
index ac8ff14..b5b71c8 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -37,4 +37,4 @@ softmmu_ss.add(when: ['CONFIG_RDMA', rdma], if_true: files('rdma.c'))
 softmmu_ss.add(when: 'CONFIG_LIVE_BLOCK_MIGRATION', if_true: files('block.c'))
 softmmu_ss.add(when: 'CONFIG_ZSTD', if_true: [files('multifd-zstd.c'), zstd])
 
-specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files('ram.c'))
+specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files('dirtyrate.c', 'ram.c'))
-- 
1.8.3.1

[PATCH v10 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status

[Chuan Zheng]

add DirtyRateStatus to denote calculating status.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c | 26 ++++++++++++++++++++++++++
 qapi/migration.json   | 17 +++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index bf7fd24..7bea8ff 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -22,6 +22,19 @@
 #include "migration.h"
 #include "dirtyrate.h"
 
+static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
+
+static int dirtyrate_set_state(int *state, int old_state, int new_state)
+{
+    assert(new_state < DIRTY_RATE_STATUS__MAX);
+    if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
+        return 0;
+    } else {
+        return -1;
+    }
+}
+
+
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
     /* todo */
@@ -31,8 +44,21 @@ static void calculate_dirtyrate(struct DirtyRateConfig config)
 void *get_dirtyrate_thread(void *arg)
 {
     struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
+    int ret;
+
+    ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_UNSTARTED,
+                              DIRTY_RATE_STATUS_MEASURING);
+    if (ret == -1) {
+        error_report("change dirtyrate state failed.");
+        return NULL;
+    }
 
     calculate_dirtyrate(config);
 
+    ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_MEASURING,
+                              DIRTY_RATE_STATUS_MEASURED);
+    if (ret == -1) {
+        error_report("change dirtyrate state failed.");
+    }
     return NULL;
 }
diff --git a/qapi/migration.json b/qapi/migration.json
index 5f6b061..061ff25 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1720,3 +1720,20 @@
 ##
 { 'event': 'UNPLUG_PRIMARY',
   'data': { 'device-id': 'str' } }
+
+##
+# @DirtyRateStatus:
+#
+# An enumeration of dirtyrate status.
+#
+# @unstarted: the dirtyrate thread has not been started.
+#
+# @measuring: the dirtyrate thread is measuring.
+#
+# @measured: the dirtyrate thread has measured and results are available.
+#
+# Since: 5.2
+#
+##
+{ 'enum': 'DirtyRateStatus',
+  'data': [ 'unstarted', 'measuring', 'measured'] }
-- 
1.8.3.1

[PATCH v10 03/12] migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info

[Chuan Zheng]

Add RamblockDirtyInfo to store sampled page info of each ramblock.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 5be9714..479e222 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -19,11 +19,29 @@
  */
 #define DIRTYRATE_DEFAULT_SAMPLE_PAGES            512
 
+/*
+ * Record ramblock idstr
+ */
+#define RAMBLOCK_INFO_MAX_LEN                     256
+
 struct DirtyRateConfig {
     uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
     int64_t sample_period_seconds; /* time duration between two sampling */
 };
 
+/*
+ * Store dirtypage info for each ramblock.
+ */
+struct RamblockDirtyInfo {
+    char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
+    uint8_t *ramblock_addr; /* base address of ramblock we measure */
+    uint64_t ramblock_pages; /* ramblock size in TARGET_PAGE_SIZE */
+    uint64_t *sample_page_vfn; /* relative offset address for sampled page */
+    uint64_t sample_pages_count; /* count of sampled pages */
+    uint64_t sample_dirty_count; /* count of dirty pages we measure */
+    uint32_t *hash_result; /* array of hash result for sampled pages */
+};
+
 void *get_dirtyrate_thread(void *arg);
 #endif
 
-- 
1.8.3.1

[PATCH v10 04/12] migration/dirtyrate: Add dirtyrate statistics series functions

[Chuan Zheng]

Add dirtyrate statistics functions to record/update dirtyrate info.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c | 32 ++++++++++++++++++++++++++++++++
 migration/dirtyrate.h | 12 ++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 7bea8ff..ab372ba 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -23,6 +23,7 @@
 #include "dirtyrate.h"
 
 static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
+static struct DirtyRateStat DirtyStat;
 
 static int dirtyrate_set_state(int *state, int old_state, int new_state)
 {
@@ -34,6 +35,37 @@ static int dirtyrate_set_state(int *state, int old_state, int new_state)
     }
 }
 
+static void reset_dirtyrate_stat(void)
+{
+    DirtyStat.total_dirty_samples = 0;
+    DirtyStat.total_sample_count = 0;
+    DirtyStat.total_block_mem_MB = 0;
+    DirtyStat.dirty_rate = -1;
+    DirtyStat.start_time = 0;
+    DirtyStat.calc_time = 0;
+}
+
+static void update_dirtyrate_stat(struct RamblockDirtyInfo *info)
+{
+    DirtyStat.total_dirty_samples += info->sample_dirty_count;
+    DirtyStat.total_sample_count += info->sample_pages_count;
+    /* size of total pages in MB */
+    DirtyStat.total_block_mem_MB += (info->ramblock_pages *
+                                     TARGET_PAGE_SIZE) >> 20;
+}
+
+static void update_dirtyrate(uint64_t msec)
+{
+    uint64_t dirtyrate;
+    uint64_t total_dirty_samples = DirtyStat.total_dirty_samples;
+    uint64_t total_sample_count = DirtyStat.total_sample_count;
+    uint64_t total_block_mem_MB = DirtyStat.total_block_mem_MB;
+
+    dirtyrate = total_dirty_samples * total_block_mem_MB *
+                1000 / (total_sample_count * msec);
+
+    DirtyStat.dirty_rate = dirtyrate;
+}
 
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 479e222..a3ee305 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -42,6 +42,18 @@ struct RamblockDirtyInfo {
     uint32_t *hash_result; /* array of hash result for sampled pages */
 };
 
+/*
+ * Store calculation statistics for each measure.
+ */
+struct DirtyRateStat {
+    uint64_t total_dirty_samples; /* total dirty sampled page */
+    uint64_t total_sample_count; /* total sampled pages */
+    uint64_t total_block_mem_MB; /* size of total sampled pages in MB */
+    int64_t dirty_rate; /* dirty rate in MB/s */
+    int64_t start_time; /* calculation start time in units of second */
+    int64_t calc_time; /* time duration of two sampling in units of second */
+};
+
 void *get_dirtyrate_thread(void *arg);
 #endif
 
-- 
1.8.3.1

[PATCH v10 05/12] migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h

[Chuan Zheng]

RAMBLOCK_FOREACH_MIGRATABLE is need in dirtyrate measure,
move the existing definition up into migration/ram.h

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c |  1 +
 migration/ram.c       | 11 +----------
 migration/ram.h       | 10 ++++++++++
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ab372ba..7366bf3 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -20,6 +20,7 @@
 #include "qemu/rcu_queue.h"
 #include "qapi/qapi-commands-migration.h"
 #include "migration.h"
+#include "ram.h"
 #include "dirtyrate.h"
 
 static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
diff --git a/migration/ram.c b/migration/ram.c
index 76d4fee..37ef0da 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -158,21 +158,12 @@ out:
     return ret;
 }
 
-static bool ramblock_is_ignored(RAMBlock *block)
+bool ramblock_is_ignored(RAMBlock *block)
 {
     return !qemu_ram_is_migratable(block) ||
            (migrate_ignore_shared() && qemu_ram_is_shared(block));
 }
 
-/* Should be holding either ram_list.mutex, or the RCU lock. */
-#define RAMBLOCK_FOREACH_NOT_IGNORED(block)            \
-    INTERNAL_RAMBLOCK_FOREACH(block)                   \
-        if (ramblock_is_ignored(block)) {} else
-
-#define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
-    INTERNAL_RAMBLOCK_FOREACH(block)                   \
-        if (!qemu_ram_is_migratable(block)) {} else
-
 #undef RAMBLOCK_FOREACH
 
 int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque)
diff --git a/migration/ram.h b/migration/ram.h
index 2eeaacf..011e854 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -37,6 +37,16 @@ extern MigrationStats ram_counters;
 extern XBZRLECacheStats xbzrle_counters;
 extern CompressionStats compression_counters;
 
+bool ramblock_is_ignored(RAMBlock *block);
+/* Should be holding either ram_list.mutex, or the RCU lock. */
+#define RAMBLOCK_FOREACH_NOT_IGNORED(block)            \
+    INTERNAL_RAMBLOCK_FOREACH(block)                   \
+        if (ramblock_is_ignored(block)) {} else
+
+#define RAMBLOCK_FOREACH_MIGRATABLE(block)             \
+    INTERNAL_RAMBLOCK_FOREACH(block)                   \
+        if (!qemu_ram_is_migratable(block)) {} else
+
 int xbzrle_cache_resize(int64_t new_size, Error **errp);
 uint64_t ram_bytes_remaining(void);
 uint64_t ram_bytes_total(void);
-- 
1.8.3.1

[PATCH v10 06/12] migration/dirtyrate: Record hash results for each sampled page

[Chuan Zheng]

Record hash results for each sampled page, crc32 is taken to calculate
hash results for each sampled length in TARGET_PAGE_SIZE.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 109 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 7366bf3..5e6eedf 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -10,6 +10,7 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include <zlib.h>
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "cpu.h"
@@ -68,6 +69,114 @@ static void update_dirtyrate(uint64_t msec)
     DirtyStat.dirty_rate = dirtyrate;
 }
 
+/*
+ * get hash result for the sampled memory with length of TARGET_PAGE_SIZE
+ * in ramblock, which starts from ramblock base address.
+ */
+static uint32_t get_ramblock_vfn_hash(struct RamblockDirtyInfo *info,
+                                      uint64_t vfn)
+{
+    uint32_t crc;
+
+    crc = crc32(0, (info->ramblock_addr +
+                vfn * TARGET_PAGE_SIZE), TARGET_PAGE_SIZE);
+
+    return crc;
+}
+
+static bool save_ramblock_hash(struct RamblockDirtyInfo *info)
+{
+    unsigned int sample_pages_count;
+    int i;
+    GRand *rand;
+
+    sample_pages_count = info->sample_pages_count;
+
+    /* ramblock size less than one page, return success to skip this ramblock */
+    if (unlikely(info->ramblock_pages == 0 || sample_pages_count == 0)) {
+        return true;
+    }
+
+    info->hash_result = g_try_malloc0_n(sample_pages_count,
+                                        sizeof(uint32_t));
+    if (!info->hash_result) {
+        return false;
+    }
+
+    info->sample_page_vfn = g_try_malloc0_n(sample_pages_count,
+                                            sizeof(uint64_t));
+    if (!info->sample_page_vfn) {
+        g_free(info->hash_result);
+        return false;
+    }
+
+    rand  = g_rand_new();
+    for (i = 0; i < sample_pages_count; i++) {
+        info->sample_page_vfn[i] = g_rand_int_range(rand, 0,
+                                                    info->ramblock_pages - 1);
+        info->hash_result[i] = get_ramblock_vfn_hash(info,
+                                                     info->sample_page_vfn[i]);
+    }
+    g_rand_free(rand);
+
+    return true;
+}
+
+static void get_ramblock_dirty_info(RAMBlock *block,
+                                    struct RamblockDirtyInfo *info,
+                                    struct DirtyRateConfig *config)
+{
+    uint64_t sample_pages_per_gigabytes = config->sample_pages_per_gigabytes;
+
+    /* Right shift 30 bits to calc ramblock size in GB */
+    info->sample_pages_count = (qemu_ram_get_used_length(block) *
+                                sample_pages_per_gigabytes) >> 30;
+    /* Right shift TARGET_PAGE_BITS to calc page count */
+    info->ramblock_pages = qemu_ram_get_used_length(block) >>
+                           TARGET_PAGE_BITS;
+    info->ramblock_addr = qemu_ram_get_host_addr(block);
+    strcpy(info->idstr, qemu_ram_get_idstr(block));
+}
+
+static bool record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
+                                      struct DirtyRateConfig config,
+                                      int *block_count)
+{
+    struct RamblockDirtyInfo *info = NULL;
+    struct RamblockDirtyInfo *dinfo = NULL;
+    RAMBlock *block = NULL;
+    int total_count = 0;
+    int index = 0;
+    bool ret = false;
+
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        total_count++;
+    }
+
+    dinfo = g_try_malloc0_n(total_count, sizeof(struct RamblockDirtyInfo));
+    if (dinfo == NULL) {
+        goto out;
+    }
+
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        if (index >= total_count) {
+            break;
+        }
+        info = &dinfo[index];
+        get_ramblock_dirty_info(block, info, &config);
+        if (!save_ramblock_hash(info)) {
+            goto out;
+        }
+        index++;
+    }
+    ret = true;
+
+out:
+    *block_count = index;
+    *block_dinfo = dinfo;
+    return ret;
+}
+
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
     /* todo */
-- 
1.8.3.1

[PATCH v10 07/12] migration/dirtyrate: Compare page hash results for recorded sampled page

[Chuan Zheng]

Compare page hash results for recorded sampled page.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/dirtyrate.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 5e6eedf..68561af 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -177,6 +177,69 @@ out:
     return ret;
 }
 
+static void calc_page_dirty_rate(struct RamblockDirtyInfo *info)
+{
+    uint32_t crc;
+    int i;
+
+    for (i = 0; i < info->sample_pages_count; i++) {
+        crc = get_ramblock_vfn_hash(info, info->sample_page_vfn[i]);
+        if (crc != info->hash_result[i]) {
+            info->sample_dirty_count++;
+        }
+    }
+}
+
+static struct RamblockDirtyInfo *
+find_block_matched(RAMBlock *block, int count,
+                  struct RamblockDirtyInfo *infos)
+{
+    int i;
+    struct RamblockDirtyInfo *matched;
+
+    for (i = 0; i < count; i++) {
+        if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
+            break;
+        }
+    }
+
+    if (i == count) {
+        return NULL;
+    }
+
+    if (infos[i].ramblock_addr != qemu_ram_get_host_addr(block) ||
+        infos[i].ramblock_pages !=
+            (qemu_ram_get_used_length(block) >> TARGET_PAGE_BITS)) {
+        return NULL;
+    }
+
+    matched = &infos[i];
+
+    return matched;
+}
+
+static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
+                                  int block_count)
+{
+    struct RamblockDirtyInfo *block_dinfo = NULL;
+    RAMBlock *block = NULL;
+
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        block_dinfo = find_block_matched(block, block_count, info);
+        if (block_dinfo == NULL) {
+            continue;
+        }
+        calc_page_dirty_rate(block_dinfo);
+        update_dirtyrate_stat(block_dinfo);
+    }
+
+    if (DirtyStat.total_sample_count == 0) {
+        return false;
+    }
+
+    return true;
+}
+
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
     /* todo */
-- 
1.8.3.1

[PATCH v10 08/12] migration/dirtyrate: skip sampling ramblock with size below MIN_RAMBLOCK_SIZE

[Chuan Zheng]

In order to sample real RAM, skip ramblock with size below MIN_RAMBLOCK_SIZE
which is set as 128M.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c | 21 +++++++++++++++++++++
 migration/dirtyrate.h |  5 +++++
 2 files changed, 26 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 68561af..ede8063 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -138,6 +138,18 @@ static void get_ramblock_dirty_info(RAMBlock *block,
     strcpy(info->idstr, qemu_ram_get_idstr(block));
 }
 
+static bool skip_sample_ramblock(RAMBlock *block)
+{
+    /*
+     * Sample only blocks larger than MIN_RAMBLOCK_SIZE.
+     */
+    if (qemu_ram_get_used_length(block) < (MIN_RAMBLOCK_SIZE << 10)) {
+        return true;
+    }
+
+    return false;
+}
+
 static bool record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
                                       struct DirtyRateConfig config,
                                       int *block_count)
@@ -150,6 +162,9 @@ static bool record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
     bool ret = false;
 
     RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        if (skip_sample_ramblock(block)) {
+            continue;
+        }
         total_count++;
     }
 
@@ -159,6 +174,9 @@ static bool record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
     }
 
     RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        if (skip_sample_ramblock(block)) {
+            continue;
+        }
         if (index >= total_count) {
             break;
         }
@@ -225,6 +243,9 @@ static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
     RAMBlock *block = NULL;
 
     RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        if (skip_sample_ramblock(block)) {
+            continue;
+        }
         block_dinfo = find_block_matched(block, block_count, info);
         if (block_dinfo == NULL) {
             continue;
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index a3ee305..faaf9da 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -24,6 +24,11 @@
  */
 #define RAMBLOCK_INFO_MAX_LEN                     256
 
+/*
+ * Minimum RAMBlock size to sample, in megabytes.
+ */
+#define MIN_RAMBLOCK_SIZE                         128
+
 struct DirtyRateConfig {
     uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
     int64_t sample_period_seconds; /* time duration between two sampling */
-- 
1.8.3.1

[PATCH v10 09/12] migration/dirtyrate: Implement set_sample_page_period() and is_sample_period_valid()

[Chuan Zheng]

Implement is_sample_period_valid() to check if the sample period is vaild and
do set_sample_page_period() to sleep specific time between sample actions.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c | 24 ++++++++++++++++++++++++
 migration/dirtyrate.h |  6 ++++++
 2 files changed, 30 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ede8063..ef77483 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -27,6 +27,30 @@
 static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
 static struct DirtyRateStat DirtyStat;
 
+static int64_t set_sample_page_period(int64_t msec, int64_t initial_time)
+{
+    int64_t current_time;
+
+    current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    if ((current_time - initial_time) >= msec) {
+        msec = current_time - initial_time;
+    } else {
+        g_usleep((msec + initial_time - current_time) * 1000);
+    }
+
+    return msec;
+}
+
+static bool is_sample_period_valid(int64_t sec)
+{
+    if (sec < MIN_FETCH_DIRTYRATE_TIME_SEC ||
+        sec > MAX_FETCH_DIRTYRATE_TIME_SEC) {
+        return false;
+    }
+
+    return true;
+}
+
 static int dirtyrate_set_state(int *state, int old_state, int new_state)
 {
     assert(new_state < DIRTY_RATE_STATUS__MAX);
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index faaf9da..8f9bc80 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -29,6 +29,12 @@
  */
 #define MIN_RAMBLOCK_SIZE                         128
 
+/*
+ * Take 1s as minimum time for calculation duration
+ */
+#define MIN_FETCH_DIRTYRATE_TIME_SEC              1
+#define MAX_FETCH_DIRTYRATE_TIME_SEC              60
+
 struct DirtyRateConfig {
     uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
     int64_t sample_period_seconds; /* time duration between two sampling */
-- 
1.8.3.1

[PATCH v10 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function

[Chuan Zheng]

Implement calculate_dirtyrate() function.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
---
 migration/dirtyrate.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ef77483..2a1a7fa 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -162,6 +162,21 @@ static void get_ramblock_dirty_info(RAMBlock *block,
     strcpy(info->idstr, qemu_ram_get_idstr(block));
 }
 
+static void free_ramblock_dirty_info(struct RamblockDirtyInfo *infos, int count)
+{
+    int i;
+
+    if (!infos) {
+        return;
+    }
+
+    for (i = 0; i < count; i++) {
+        g_free(infos[i].sample_page_vfn);
+        g_free(infos[i].hash_result);
+    }
+    g_free(infos);
+}
+
 static bool skip_sample_ramblock(RAMBlock *block)
 {
     /*
@@ -287,8 +302,34 @@ static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
 
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
-    /* todo */
-    return;
+    struct RamblockDirtyInfo *block_dinfo = NULL;
+    int block_count = 0;
+    int64_t msec = 0;
+    int64_t initial_time;
+
+    rcu_register_thread();
+    reset_dirtyrate_stat();
+    rcu_read_lock();
+    initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    if (!record_ramblock_hash_info(&block_dinfo, config, &block_count)) {
+        goto out;
+    }
+    rcu_read_unlock();
+
+    msec = config.sample_period_seconds * 1000;
+    msec = set_sample_page_period(msec, initial_time);
+
+    rcu_read_lock();
+    if (!compare_page_hash_info(block_dinfo, block_count)) {
+        goto out;
+    }
+
+    update_dirtyrate(msec);
+
+out:
+    rcu_read_unlock();
+    free_ramblock_dirty_info(block_dinfo, block_count);
+    rcu_unregister_thread();
 }
 
 void *get_dirtyrate_thread(void *arg)
-- 
1.8.3.1

[PATCH v10 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[Chuan Zheng]

Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/dirtyrate.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/migration.json   | 50 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 2a1a7fa..06f455d 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -61,6 +61,24 @@ static int dirtyrate_set_state(int *state, int old_state, int new_state)
     }
 }
 
+static struct DirtyRateInfo *query_dirty_rate_info(void)
+{
+    int64_t dirty_rate = DirtyStat.dirty_rate;
+    struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
+
+    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) {
+        info->dirty_rate = dirty_rate;
+    } else {
+        info->dirty_rate = -1;
+    }
+
+    info->status = CalculatingState;
+    info->start_time = DirtyStat.start_time;
+    info->calc_time = DirtyStat.calc_time;
+
+    return info;
+}
+
 static void reset_dirtyrate_stat(void)
 {
     DirtyStat.total_dirty_samples = 0;
@@ -318,6 +336,8 @@ static void calculate_dirtyrate(struct DirtyRateConfig config)
 
     msec = config.sample_period_seconds * 1000;
     msec = set_sample_page_period(msec, initial_time);
+    DirtyStat.start_time = initial_time / 1000;
+    DirtyStat.calc_time = msec / 1000;
 
     rcu_read_lock();
     if (!compare_page_hash_info(block_dinfo, block_count)) {
@@ -353,3 +373,45 @@ void *get_dirtyrate_thread(void *arg)
     }
     return NULL;
 }
+
+void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
+{
+    static struct DirtyRateConfig config;
+    QemuThread thread;
+    int ret;
+
+    /*
+     * If the dirty rate is already being measured, don't attempt to start.
+     */
+    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURING) {
+        error_setg(errp, "the dirty rate is already being measured.");
+        return;
+    }
+
+    if (!is_sample_period_valid(calc_time)) {
+        error_setg(errp, "calc-time is out of range[%d, %d].",
+                         MIN_FETCH_DIRTYRATE_TIME_SEC,
+                         MAX_FETCH_DIRTYRATE_TIME_SEC);
+        return;
+    }
+
+    /*
+     * Init calculation state as unstarted.
+     */
+    ret = dirtyrate_set_state(&CalculatingState, CalculatingState,
+                              DIRTY_RATE_STATUS_UNSTARTED);
+    if (ret == -1) {
+        error_setg(errp, "init dirty rate calculation state failed.");
+        return;
+    }
+
+    config.sample_period_seconds = calc_time;
+    config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+    qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
+                       (void *)&config, QEMU_THREAD_DETACHED);
+}
+
+struct DirtyRateInfo *qmp_query_dirty_rate(Error **errp)
+{
+    return query_dirty_rate_info();
+}
diff --git a/qapi/migration.json b/qapi/migration.json
index 061ff25..4b980a0 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1737,3 +1737,53 @@
 ##
 { 'enum': 'DirtyRateStatus',
   'data': [ 'unstarted', 'measuring', 'measured'] }
+
+##
+# @DirtyRateInfo:
+#
+# Information about current dirty page rate of vm.
+#
+# @dirty-rate: @dirtyrate describing the dirty page rate of vm
+#          in units of MB/s.
+#          If this field return '-1', it means querying is not
+#          start or not complete.
+#
+# @status: status containing dirtyrate query status includes
+#          'unstarted' or 'measuring' or 'measured'
+#
+# @start-time: start time in units of second for calculation
+#
+# @calc-time: time in units of second for sample dirty pages
+#
+# Since: 5.2
+#
+##
+{ 'struct': 'DirtyRateInfo',
+  'data': {'dirty-rate': 'int64',
+           'status': 'DirtyRateStatus',
+           'start-time': 'int64',
+           'calc-time': 'int64'} }
+
+##
+# @calc-dirty-rate:
+#
+# start calculating dirty page rate for vm
+#
+# @calc-time: time in units of second for sample dirty pages
+#
+# Since: 5.2
+#
+# Example:
+#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
+#
+##
+{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
+
+##
+# @query-dirty-rate:
+#
+# query dirty page rate in units of MB/s for vm
+#
+# Since: 5.2
+##
+{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
-- 
1.8.3.1

[PATCH v10 12/12] migration/dirtyrate: Add trace_calls to make it easier to debug

[Chuan Zheng]

Add trace_calls to make it easier to debug

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: David Edmondson <david.edmondson@oracle.com>
---
 migration/dirtyrate.c  | 9 +++++++++
 migration/trace-events | 8 ++++++++
 2 files changed, 17 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 06f455d..f432840 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -22,6 +22,7 @@
 #include "qapi/qapi-commands-migration.h"
 #include "migration.h"
 #include "ram.h"
+#include "trace.h"
 #include "dirtyrate.h"
 
 static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
@@ -54,6 +55,7 @@ static bool is_sample_period_valid(int64_t sec)
 static int dirtyrate_set_state(int *state, int old_state, int new_state)
 {
     assert(new_state < DIRTY_RATE_STATUS__MAX);
+    trace_dirtyrate_set_state(DirtyRateStatus_str(new_state));
     if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
         return 0;
     } else {
@@ -76,6 +78,8 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
     info->start_time = DirtyStat.start_time;
     info->calc_time = DirtyStat.calc_time;
 
+    trace_query_dirty_rate_info(DirtyRateStatus_str(CalculatingState));
+
     return info;
 }
 
@@ -123,6 +127,7 @@ static uint32_t get_ramblock_vfn_hash(struct RamblockDirtyInfo *info,
     crc = crc32(0, (info->ramblock_addr +
                 vfn * TARGET_PAGE_SIZE), TARGET_PAGE_SIZE);
 
+    trace_get_ramblock_vfn_hash(info->idstr, vfn, crc);
     return crc;
 }
 
@@ -201,6 +206,8 @@ static bool skip_sample_ramblock(RAMBlock *block)
      * Sample only blocks larger than MIN_RAMBLOCK_SIZE.
      */
     if (qemu_ram_get_used_length(block) < (MIN_RAMBLOCK_SIZE << 10)) {
+        trace_skip_sample_ramblock(block->idstr,
+                                   qemu_ram_get_used_length(block));
         return true;
     }
 
@@ -260,6 +267,7 @@ static void calc_page_dirty_rate(struct RamblockDirtyInfo *info)
     for (i = 0; i < info->sample_pages_count; i++) {
         crc = get_ramblock_vfn_hash(info, info->sample_page_vfn[i]);
         if (crc != info->hash_result[i]) {
+            trace_calc_page_dirty_rate(info->idstr, crc, info->hash_result[i]);
             info->sample_dirty_count++;
         }
     }
@@ -285,6 +293,7 @@ find_block_matched(RAMBlock *block, int count,
     if (infos[i].ramblock_addr != qemu_ram_get_host_addr(block) ||
         infos[i].ramblock_pages !=
             (qemu_ram_get_used_length(block) >> TARGET_PAGE_BITS)) {
+        trace_find_page_matched(block->idstr);
         return NULL;
     }
 
diff --git a/migration/trace-events b/migration/trace-events
index 4ab0a50..8c2b58f 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -312,3 +312,11 @@ dirty_bitmap_load_bits_zeroes(void) ""
 dirty_bitmap_load_header(uint32_t flags) "flags 0x%x"
 dirty_bitmap_load_enter(void) ""
 dirty_bitmap_load_success(void) ""
+
+# dirtyrate.c
+dirtyrate_set_state(const char *new_state) "new state %s"
+query_dirty_rate_info(const char *new_state) "current state %s"
+get_ramblock_vfn_hash(const char *idstr, uint64_t vfn, uint32_t crc) "ramblock name: %s, vfn: %"PRIu64 ", crc: %" PRIu32
+calc_page_dirty_rate(const char *idstr, uint32_t new_crc, uint32_t old_crc) "ramblock name: %s, new crc: %" PRIu32 ", old crc: %" PRIu32
+skip_sample_ramblock(const char *idstr, uint64_t ramblock_size) "ramblock name: %s, ramblock size: %" PRIu64
+find_page_matched(const char *idstr) "ramblock %s addr or size changed"
-- 
1.8.3.1

Re: [PATCH v10 07/12] migration/dirtyrate: Compare page hash results for recorded sampled page

[Li Qiang]

Chuan Zheng <zhengchuan@huawei.com> 于2020年9月16日周三 下午2:11写道：
>
> Compare page hash results for recorded sampled page.
>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Li Qiang <liq3ea@gmail.com>

> ---
>  migration/dirtyrate.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 5e6eedf..68561af 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -177,6 +177,69 @@ out:
>      return ret;
>  }
>
> +static void calc_page_dirty_rate(struct RamblockDirtyInfo *info)
> +{
> +    uint32_t crc;
> +    int i;
> +
> +    for (i = 0; i < info->sample_pages_count; i++) {
> +        crc = get_ramblock_vfn_hash(info, info->sample_page_vfn[i]);
> +        if (crc != info->hash_result[i]) {
> +            info->sample_dirty_count++;
> +        }
> +    }
> +}
> +
> +static struct RamblockDirtyInfo *
> +find_block_matched(RAMBlock *block, int count,
> +                  struct RamblockDirtyInfo *infos)
> +{
> +    int i;
> +    struct RamblockDirtyInfo *matched;
> +
> +    for (i = 0; i < count; i++) {
> +        if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
> +            break;
> +        }
> +    }
> +
> +    if (i == count) {
> +        return NULL;
> +    }
> +
> +    if (infos[i].ramblock_addr != qemu_ram_get_host_addr(block) ||
> +        infos[i].ramblock_pages !=
> +            (qemu_ram_get_used_length(block) >> TARGET_PAGE_BITS)) {
> +        return NULL;
> +    }
> +
> +    matched = &infos[i];
> +
> +    return matched;
> +}
> +
> +static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
> +                                  int block_count)
> +{
> +    struct RamblockDirtyInfo *block_dinfo = NULL;
> +    RAMBlock *block = NULL;
> +
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +        block_dinfo = find_block_matched(block, block_count, info);
> +        if (block_dinfo == NULL) {
> +            continue;
> +        }
> +        calc_page_dirty_rate(block_dinfo);
> +        update_dirtyrate_stat(block_dinfo);
> +    }
> +
> +    if (DirtyStat.total_sample_count == 0) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
>      /* todo */
> --
> 1.8.3.1
>

Re: [PATCH v10 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>

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

> ---
>  migration/dirtyrate.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/migration.json   | 50 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 112 insertions(+)
> 
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 2a1a7fa..06f455d 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -61,6 +61,24 @@ static int dirtyrate_set_state(int *state, int old_state, int new_state)
>      }
>  }
>  
> +static struct DirtyRateInfo *query_dirty_rate_info(void)
> +{
> +    int64_t dirty_rate = DirtyStat.dirty_rate;
> +    struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
> +
> +    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) {
> +        info->dirty_rate = dirty_rate;
> +    } else {
> +        info->dirty_rate = -1;
> +    }
> +
> +    info->status = CalculatingState;
> +    info->start_time = DirtyStat.start_time;
> +    info->calc_time = DirtyStat.calc_time;
> +
> +    return info;
> +}
> +
>  static void reset_dirtyrate_stat(void)
>  {
>      DirtyStat.total_dirty_samples = 0;
> @@ -318,6 +336,8 @@ static void calculate_dirtyrate(struct DirtyRateConfig config)
>  
>      msec = config.sample_period_seconds * 1000;
>      msec = set_sample_page_period(msec, initial_time);
> +    DirtyStat.start_time = initial_time / 1000;
> +    DirtyStat.calc_time = msec / 1000;
>  
>      rcu_read_lock();
>      if (!compare_page_hash_info(block_dinfo, block_count)) {
> @@ -353,3 +373,45 @@ void *get_dirtyrate_thread(void *arg)
>      }
>      return NULL;
>  }
> +
> +void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
> +{
> +    static struct DirtyRateConfig config;
> +    QemuThread thread;
> +    int ret;
> +
> +    /*
> +     * If the dirty rate is already being measured, don't attempt to start.
> +     */
> +    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURING) {
> +        error_setg(errp, "the dirty rate is already being measured.");
> +        return;
> +    }
> +
> +    if (!is_sample_period_valid(calc_time)) {
> +        error_setg(errp, "calc-time is out of range[%d, %d].",
> +                         MIN_FETCH_DIRTYRATE_TIME_SEC,
> +                         MAX_FETCH_DIRTYRATE_TIME_SEC);
> +        return;
> +    }
> +
> +    /*
> +     * Init calculation state as unstarted.
> +     */
> +    ret = dirtyrate_set_state(&CalculatingState, CalculatingState,
> +                              DIRTY_RATE_STATUS_UNSTARTED);
> +    if (ret == -1) {
> +        error_setg(errp, "init dirty rate calculation state failed.");
> +        return;
> +    }
> +
> +    config.sample_period_seconds = calc_time;
> +    config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
> +    qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
> +                       (void *)&config, QEMU_THREAD_DETACHED);
> +}
> +
> +struct DirtyRateInfo *qmp_query_dirty_rate(Error **errp)
> +{
> +    return query_dirty_rate_info();
> +}
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 061ff25..4b980a0 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1737,3 +1737,53 @@
>  ##
>  { 'enum': 'DirtyRateStatus',
>    'data': [ 'unstarted', 'measuring', 'measured'] }
> +
> +##
> +# @DirtyRateInfo:
> +#
> +# Information about current dirty page rate of vm.
> +#
> +# @dirty-rate: @dirtyrate describing the dirty page rate of vm
> +#          in units of MB/s.
> +#          If this field return '-1', it means querying is not
> +#          start or not complete.
> +#
> +# @status: status containing dirtyrate query status includes
> +#          'unstarted' or 'measuring' or 'measured'
> +#
> +# @start-time: start time in units of second for calculation
> +#
> +# @calc-time: time in units of second for sample dirty pages
> +#
> +# Since: 5.2
> +#
> +##
> +{ 'struct': 'DirtyRateInfo',
> +  'data': {'dirty-rate': 'int64',
> +           'status': 'DirtyRateStatus',
> +           'start-time': 'int64',
> +           'calc-time': 'int64'} }
> +
> +##
> +# @calc-dirty-rate:
> +#
> +# start calculating dirty page rate for vm
> +#
> +# @calc-time: time in units of second for sample dirty pages
> +#
> +# Since: 5.2
> +#
> +# Example:
> +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
> +#
> +##
> +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
> +
> +##
> +# @query-dirty-rate:
> +#
> +# query dirty page rate in units of MB/s for vm
> +#
> +# Since: 5.2
> +##
> +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[Dr. David Alan Gilbert]

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> v9 -> v10:
>     rename find_page_matched as find_block_matched
>     fix wrong termination condition in find_block_matched
>     add review-by for patches
> 
> v8 -> v9:
>     fix wrong index return of record_ramblock_hash_info
>     optimize variable name according to review
>     reset dirty_rate as -1
>     change returns of compare_page_hash_info to bool
> 
> v7 -> v8:
>     add atomic_read for dirtyrate status
>     add error_report if set dirtyrate state failed
>     change returns of save_ramblock_hash and record_ramblock_hash_info to bool
>     alloc ramblock dirtyinfo array at one time
>     add review-by for patches
> 
> v6 -> v7:
>     fix minior comments and coding style by review
>     add review-by for patches
> 
> v5 -> v6:
>     fix coding style according to review
>     use TARGET_PAGE_SIZE and TARGET_PAGE_BITS instead of self-defined macros
>     return start-time and calc-time by qmp command
> 
> v4 -> v5:
>     fix git apply failed due to meson-build
>     add review-by for patches in v3
> 
> v3 -> v4:
>     use crc32 to get hash result instead of md5
>     add DirtyRateStatus to denote calculation status
>     add some trace_calls to make it easier to debug
>     fix some comments accroding to review
> 
> v2 -> v3:
>     fix size_t compile warning
>     fix codestyle checked by checkpatch.pl
> 
> v1 -> v2:
>     use g_rand_new() to generate rand_buf
>     move RAMBLOCK_FOREACH_MIGRATABLE into migration/ram.h
>     add skip_sample_ramblock to filter sampled ramblock
>     fix multi-numa vm coredump when query dirtyrate
>     rename qapi interface and rename some structures and functions
>     succeed to compile by appling each patch
>     add test for migrating vm
> 
> Sometimes it is neccessary to evaluate dirty page rate before migration.
> Users could decide whether to proceed migration based on the evaluation
> in case of vm performance loss due to heavy workload.
> Unlikey simulating dirtylog sync which could do harm on runnning vm,
> we provide a sample-hash method to compare hash results for samping page.
> In this way, it would have hardly no impact on vm performance.
> 
> Evaluate the dirtypage rate both on running and migration vm.
> The VM specifications for migration are as follows:
> - VM use 4-K page;
> - the number of VCPU is 32;
> - the total memory is 32Gigabit;
> - use 'mempress' tool to pressurize VM(mempress 4096 1024);
> - migration bandwidth is 1GB/s
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> |                      |  running  |                  migrating                           |
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> | no mempress          |   4MB/s   |          8MB/s      (migrated success)               |
> -------------------------------------------------------------------------------------------
> | mempress 4096 1024   |  1060MB/s |     456MB/s ~ 1142MB/s (cpu throttle triggered)      |
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> | mempress 4096 4096   |  4114MB/s |     688MB/s ~ 4132MB/s (cpu throttle triggered)      |
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> Test dirtyrate by qmp command like this:
> 1.  virsh qemu-monitor-command [vmname] '{"execute":"calc-dirty-rate", "arguments": {"calc-time": [sleep-time]}}'; 
> 2.  sleep specific time which is a bit larger than sleep-time
> 3.  virsh qemu-monitor-command [vmname] '{"execute":"query-dirty-rate"}'

Thanks; it looks like we have a full set of reviews; I'll try and add
this for the next migration pull.

> The qmp command returns like this:
> {"return":{"status":"measured","dirty-rate":374,"start-time":3718293,"calc-time":1},"id":"libvirt-15"}
> 
> Further test dirtyrate by libvirt api like this:
> virsh getdirtyrate [vmname] [sleep-time]

So do you have some libvirt patches you're going to post?

Dave

> Chuan Zheng (12):
>   migration/dirtyrate: setup up query-dirtyrate framwork
>   migration/dirtyrate: add DirtyRateStatus to denote calculation status
>   migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info
>   migration/dirtyrate: Add dirtyrate statistics series functions
>   migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h
>   migration/dirtyrate: Record hash results for each sampled page
>   migration/dirtyrate: Compare page hash results for recorded sampled
>     page
>   migration/dirtyrate: skip sampling ramblock with size below
>     MIN_RAMBLOCK_SIZE
>   migration/dirtyrate: Implement set_sample_page_period() and
>     is_sample_period_valid()
>   migration/dirtyrate: Implement calculate_dirtyrate() function
>   migration/dirtyrate: Implement
>     qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
>   migration/dirtyrate: Add trace_calls to make it easier to debug
> 
>  migration/dirtyrate.c  | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
>  migration/dirtyrate.h  |  70 ++++++++
>  migration/meson.build  |   2 +-
>  migration/ram.c        |  11 +-
>  migration/ram.h        |  10 ++
>  migration/trace-events |   8 +
>  qapi/migration.json    |  67 ++++++++
>  7 files changed, 583 insertions(+), 11 deletions(-)
>  create mode 100644 migration/dirtyrate.c
>  create mode 100644 migration/dirtyrate.h
> 
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[Zheng Chuan]

On 2020/9/17 17:15, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> v9 -> v10:
>>     rename find_page_matched as find_block_matched
>>     fix wrong termination condition in find_block_matched
>>     add review-by for patches
>>
>> v8 -> v9:
>>     fix wrong index return of record_ramblock_hash_info
>>     optimize variable name according to review
>>     reset dirty_rate as -1
>>     change returns of compare_page_hash_info to bool
>>
>> v7 -> v8:
>>     add atomic_read for dirtyrate status
>>     add error_report if set dirtyrate state failed
>>     change returns of save_ramblock_hash and record_ramblock_hash_info to bool
>>     alloc ramblock dirtyinfo array at one time
>>     add review-by for patches
>>
>> v6 -> v7:
>>     fix minior comments and coding style by review
>>     add review-by for patches
>>
>> v5 -> v6:
>>     fix coding style according to review
>>     use TARGET_PAGE_SIZE and TARGET_PAGE_BITS instead of self-defined macros
>>     return start-time and calc-time by qmp command
>>
>> v4 -> v5:
>>     fix git apply failed due to meson-build
>>     add review-by for patches in v3
>>
>> v3 -> v4:
>>     use crc32 to get hash result instead of md5
>>     add DirtyRateStatus to denote calculation status
>>     add some trace_calls to make it easier to debug
>>     fix some comments accroding to review
>>
>> v2 -> v3:
>>     fix size_t compile warning
>>     fix codestyle checked by checkpatch.pl
>>
>> v1 -> v2:
>>     use g_rand_new() to generate rand_buf
>>     move RAMBLOCK_FOREACH_MIGRATABLE into migration/ram.h
>>     add skip_sample_ramblock to filter sampled ramblock
>>     fix multi-numa vm coredump when query dirtyrate
>>     rename qapi interface and rename some structures and functions
>>     succeed to compile by appling each patch
>>     add test for migrating vm
>>
>> Sometimes it is neccessary to evaluate dirty page rate before migration.
>> Users could decide whether to proceed migration based on the evaluation
>> in case of vm performance loss due to heavy workload.
>> Unlikey simulating dirtylog sync which could do harm on runnning vm,
>> we provide a sample-hash method to compare hash results for samping page.
>> In this way, it would have hardly no impact on vm performance.
>>
>> Evaluate the dirtypage rate both on running and migration vm.
>> The VM specifications for migration are as follows:
>> - VM use 4-K page;
>> - the number of VCPU is 32;
>> - the total memory is 32Gigabit;
>> - use 'mempress' tool to pressurize VM(mempress 4096 1024);
>> - migration bandwidth is 1GB/s
>>
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> |                      |  running  |                  migrating                           |
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> | no mempress          |   4MB/s   |          8MB/s      (migrated success)               |
>> -------------------------------------------------------------------------------------------
>> | mempress 4096 1024   |  1060MB/s |     456MB/s ~ 1142MB/s (cpu throttle triggered)      |
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> | mempress 4096 4096   |  4114MB/s |     688MB/s ~ 4132MB/s (cpu throttle triggered)      |
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>
>> Test dirtyrate by qmp command like this:
>> 1.  virsh qemu-monitor-command [vmname] '{"execute":"calc-dirty-rate", "arguments": {"calc-time": [sleep-time]}}'; 
>> 2.  sleep specific time which is a bit larger than sleep-time
>> 3.  virsh qemu-monitor-command [vmname] '{"execute":"query-dirty-rate"}'
> 
> Thanks; it looks like we have a full set of reviews; I'll try and add
> this for the next migration pull.
> 
>> The qmp command returns like this:
>> {"return":{"status":"measured","dirty-rate":374,"start-time":3718293,"calc-time":1},"id":"libvirt-15"}
>>
>> Further test dirtyrate by libvirt api like this:
>> virsh getdirtyrate [vmname] [sleep-time]
> 
> So do you have some libvirt patches you're going to post?
> 
> Dave
> 

Hi, Dave.

Yes, libvirt(including libvirt-python) interface will come after this qemu series patch:)

>> Chuan Zheng (12):
>>   migration/dirtyrate: setup up query-dirtyrate framwork
>>   migration/dirtyrate: add DirtyRateStatus to denote calculation status
>>   migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info
>>   migration/dirtyrate: Add dirtyrate statistics series functions
>>   migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h
>>   migration/dirtyrate: Record hash results for each sampled page
>>   migration/dirtyrate: Compare page hash results for recorded sampled
>>     page
>>   migration/dirtyrate: skip sampling ramblock with size below
>>     MIN_RAMBLOCK_SIZE
>>   migration/dirtyrate: Implement set_sample_page_period() and
>>     is_sample_period_valid()
>>   migration/dirtyrate: Implement calculate_dirtyrate() function
>>   migration/dirtyrate: Implement
>>     qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
>>   migration/dirtyrate: Add trace_calls to make it easier to debug
>>
>>  migration/dirtyrate.c  | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  migration/dirtyrate.h  |  70 ++++++++
>>  migration/meson.build  |   2 +-
>>  migration/ram.c        |  11 +-
>>  migration/ram.h        |  10 ++
>>  migration/trace-events |   8 +
>>  qapi/migration.json    |  67 ++++++++
>>  7 files changed, 583 insertions(+), 11 deletions(-)
>>  create mode 100644 migration/dirtyrate.c
>>  create mode 100644 migration/dirtyrate.h
>>
>> -- 
>> 1.8.3.1
>>

Re: [PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[Dr. David Alan Gilbert]

* Zheng Chuan (zhengchuan@huawei.com) wrote:
> 
> 
> On 2020/9/17 17:15, Dr. David Alan Gilbert wrote:
> > * Chuan Zheng (zhengchuan@huawei.com) wrote:
> >> v9 -> v10:
> >>     rename find_page_matched as find_block_matched
> >>     fix wrong termination condition in find_block_matched
> >>     add review-by for patches
> >>
> >> v8 -> v9:
> >>     fix wrong index return of record_ramblock_hash_info
> >>     optimize variable name according to review
> >>     reset dirty_rate as -1
> >>     change returns of compare_page_hash_info to bool
> >>
> >> v7 -> v8:
> >>     add atomic_read for dirtyrate status
> >>     add error_report if set dirtyrate state failed
> >>     change returns of save_ramblock_hash and record_ramblock_hash_info to bool
> >>     alloc ramblock dirtyinfo array at one time
> >>     add review-by for patches
> >>
> >> v6 -> v7:
> >>     fix minior comments and coding style by review
> >>     add review-by for patches
> >>
> >> v5 -> v6:
> >>     fix coding style according to review
> >>     use TARGET_PAGE_SIZE and TARGET_PAGE_BITS instead of self-defined macros
> >>     return start-time and calc-time by qmp command
> >>
> >> v4 -> v5:
> >>     fix git apply failed due to meson-build
> >>     add review-by for patches in v3
> >>
> >> v3 -> v4:
> >>     use crc32 to get hash result instead of md5
> >>     add DirtyRateStatus to denote calculation status
> >>     add some trace_calls to make it easier to debug
> >>     fix some comments accroding to review
> >>
> >> v2 -> v3:
> >>     fix size_t compile warning
> >>     fix codestyle checked by checkpatch.pl
> >>
> >> v1 -> v2:
> >>     use g_rand_new() to generate rand_buf
> >>     move RAMBLOCK_FOREACH_MIGRATABLE into migration/ram.h
> >>     add skip_sample_ramblock to filter sampled ramblock
> >>     fix multi-numa vm coredump when query dirtyrate
> >>     rename qapi interface and rename some structures and functions
> >>     succeed to compile by appling each patch
> >>     add test for migrating vm
> >>
> >> Sometimes it is neccessary to evaluate dirty page rate before migration.
> >> Users could decide whether to proceed migration based on the evaluation
> >> in case of vm performance loss due to heavy workload.
> >> Unlikey simulating dirtylog sync which could do harm on runnning vm,
> >> we provide a sample-hash method to compare hash results for samping page.
> >> In this way, it would have hardly no impact on vm performance.
> >>
> >> Evaluate the dirtypage rate both on running and migration vm.
> >> The VM specifications for migration are as follows:
> >> - VM use 4-K page;
> >> - the number of VCPU is 32;
> >> - the total memory is 32Gigabit;
> >> - use 'mempress' tool to pressurize VM(mempress 4096 1024);
> >> - migration bandwidth is 1GB/s
> >>
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> |                      |  running  |                  migrating                           |
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> | no mempress          |   4MB/s   |          8MB/s      (migrated success)               |
> >> -------------------------------------------------------------------------------------------
> >> | mempress 4096 1024   |  1060MB/s |     456MB/s ~ 1142MB/s (cpu throttle triggered)      |
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> | mempress 4096 4096   |  4114MB/s |     688MB/s ~ 4132MB/s (cpu throttle triggered)      |
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>
> >> Test dirtyrate by qmp command like this:
> >> 1.  virsh qemu-monitor-command [vmname] '{"execute":"calc-dirty-rate", "arguments": {"calc-time": [sleep-time]}}'; 
> >> 2.  sleep specific time which is a bit larger than sleep-time
> >> 3.  virsh qemu-monitor-command [vmname] '{"execute":"query-dirty-rate"}'
> > 
> > Thanks; it looks like we have a full set of reviews; I'll try and add
> > this for the next migration pull.
> > 
> >> The qmp command returns like this:
> >> {"return":{"status":"measured","dirty-rate":374,"start-time":3718293,"calc-time":1},"id":"libvirt-15"}
> >>
> >> Further test dirtyrate by libvirt api like this:
> >> virsh getdirtyrate [vmname] [sleep-time]
> > 
> > So do you have some libvirt patches you're going to post?
> > 
> > Dave
> > 
> 
> Hi, Dave.
> 
> Yes, libvirt(including libvirt-python) interface will come after this qemu series patch:)

OK, great; please cc in jdenemar@redhat.com when you do.

Dave

> >> Chuan Zheng (12):
> >>   migration/dirtyrate: setup up query-dirtyrate framwork
> >>   migration/dirtyrate: add DirtyRateStatus to denote calculation status
> >>   migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info
> >>   migration/dirtyrate: Add dirtyrate statistics series functions
> >>   migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h
> >>   migration/dirtyrate: Record hash results for each sampled page
> >>   migration/dirtyrate: Compare page hash results for recorded sampled
> >>     page
> >>   migration/dirtyrate: skip sampling ramblock with size below
> >>     MIN_RAMBLOCK_SIZE
> >>   migration/dirtyrate: Implement set_sample_page_period() and
> >>     is_sample_period_valid()
> >>   migration/dirtyrate: Implement calculate_dirtyrate() function
> >>   migration/dirtyrate: Implement
> >>     qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
> >>   migration/dirtyrate: Add trace_calls to make it easier to debug
> >>
> >>  migration/dirtyrate.c  | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  migration/dirtyrate.h  |  70 ++++++++
> >>  migration/meson.build  |   2 +-
> >>  migration/ram.c        |  11 +-
> >>  migration/ram.h        |  10 ++
> >>  migration/trace-events |   8 +
> >>  qapi/migration.json    |  67 ++++++++
> >>  7 files changed, 583 insertions(+), 11 deletions(-)
> >>  create mode 100644 migration/dirtyrate.c
> >>  create mode 100644 migration/dirtyrate.h
> >>
> >> -- 
> >> 1.8.3.1
> >>
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[Zheng Chuan]

On 2020/9/17 17:38, Dr. David Alan Gilbert wrote:
> * Zheng Chuan (zhengchuan@huawei.com) wrote:
>>
>>
>> On 2020/9/17 17:15, Dr. David Alan Gilbert wrote:
>>> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>>>> v9 -> v10:
>>>>     rename find_page_matched as find_block_matched
>>>>     fix wrong termination condition in find_block_matched
>>>>     add review-by for patches
>>>>
>>>> v8 -> v9:
>>>>     fix wrong index return of record_ramblock_hash_info
>>>>     optimize variable name according to review
>>>>     reset dirty_rate as -1
>>>>     change returns of compare_page_hash_info to bool
>>>>
>>>> v7 -> v8:
>>>>     add atomic_read for dirtyrate status
>>>>     add error_report if set dirtyrate state failed
>>>>     change returns of save_ramblock_hash and record_ramblock_hash_info to bool
>>>>     alloc ramblock dirtyinfo array at one time
>>>>     add review-by for patches
>>>>
>>>> v6 -> v7:
>>>>     fix minior comments and coding style by review
>>>>     add review-by for patches
>>>>
>>>> v5 -> v6:
>>>>     fix coding style according to review
>>>>     use TARGET_PAGE_SIZE and TARGET_PAGE_BITS instead of self-defined macros
>>>>     return start-time and calc-time by qmp command
>>>>
>>>> v4 -> v5:
>>>>     fix git apply failed due to meson-build
>>>>     add review-by for patches in v3
>>>>
>>>> v3 -> v4:
>>>>     use crc32 to get hash result instead of md5
>>>>     add DirtyRateStatus to denote calculation status
>>>>     add some trace_calls to make it easier to debug
>>>>     fix some comments accroding to review
>>>>
>>>> v2 -> v3:
>>>>     fix size_t compile warning
>>>>     fix codestyle checked by checkpatch.pl
>>>>
>>>> v1 -> v2:
>>>>     use g_rand_new() to generate rand_buf
>>>>     move RAMBLOCK_FOREACH_MIGRATABLE into migration/ram.h
>>>>     add skip_sample_ramblock to filter sampled ramblock
>>>>     fix multi-numa vm coredump when query dirtyrate
>>>>     rename qapi interface and rename some structures and functions
>>>>     succeed to compile by appling each patch
>>>>     add test for migrating vm
>>>>
>>>> Sometimes it is neccessary to evaluate dirty page rate before migration.
>>>> Users could decide whether to proceed migration based on the evaluation
>>>> in case of vm performance loss due to heavy workload.
>>>> Unlikey simulating dirtylog sync which could do harm on runnning vm,
>>>> we provide a sample-hash method to compare hash results for samping page.
>>>> In this way, it would have hardly no impact on vm performance.
>>>>
>>>> Evaluate the dirtypage rate both on running and migration vm.
>>>> The VM specifications for migration are as follows:
>>>> - VM use 4-K page;
>>>> - the number of VCPU is 32;
>>>> - the total memory is 32Gigabit;
>>>> - use 'mempress' tool to pressurize VM(mempress 4096 1024);
>>>> - migration bandwidth is 1GB/s
>>>>
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> |                      |  running  |                  migrating                           |
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> | no mempress          |   4MB/s   |          8MB/s      (migrated success)               |
>>>> -------------------------------------------------------------------------------------------
>>>> | mempress 4096 1024   |  1060MB/s |     456MB/s ~ 1142MB/s (cpu throttle triggered)      |
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> | mempress 4096 4096   |  4114MB/s |     688MB/s ~ 4132MB/s (cpu throttle triggered)      |
>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>
>>>> Test dirtyrate by qmp command like this:
>>>> 1.  virsh qemu-monitor-command [vmname] '{"execute":"calc-dirty-rate", "arguments": {"calc-time": [sleep-time]}}'; 
>>>> 2.  sleep specific time which is a bit larger than sleep-time
>>>> 3.  virsh qemu-monitor-command [vmname] '{"execute":"query-dirty-rate"}'
>>>
>>> Thanks; it looks like we have a full set of reviews; I'll try and add
>>> this for the next migration pull.
>>>
>>>> The qmp command returns like this:
>>>> {"return":{"status":"measured","dirty-rate":374,"start-time":3718293,"calc-time":1},"id":"libvirt-15"}
>>>>
>>>> Further test dirtyrate by libvirt api like this:
>>>> virsh getdirtyrate [vmname] [sleep-time]
>>>
>>> So do you have some libvirt patches you're going to post?
>>>
>>> Dave
>>>
>>
>> Hi, Dave.
>>
>> Yes, libvirt(including libvirt-python) interface will come after this qemu series patch:)
> 
> OK, great; please cc in jdenemar@redhat.com when you do.
> 
> Dave
> 
Sure, will post it asap:)

>>>> Chuan Zheng (12):
>>>>   migration/dirtyrate: setup up query-dirtyrate framwork
>>>>   migration/dirtyrate: add DirtyRateStatus to denote calculation status
>>>>   migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info
>>>>   migration/dirtyrate: Add dirtyrate statistics series functions
>>>>   migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h
>>>>   migration/dirtyrate: Record hash results for each sampled page
>>>>   migration/dirtyrate: Compare page hash results for recorded sampled
>>>>     page
>>>>   migration/dirtyrate: skip sampling ramblock with size below
>>>>     MIN_RAMBLOCK_SIZE
>>>>   migration/dirtyrate: Implement set_sample_page_period() and
>>>>     is_sample_period_valid()
>>>>   migration/dirtyrate: Implement calculate_dirtyrate() function
>>>>   migration/dirtyrate: Implement
>>>>     qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
>>>>   migration/dirtyrate: Add trace_calls to make it easier to debug
>>>>
>>>>  migration/dirtyrate.c  | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  migration/dirtyrate.h  |  70 ++++++++
>>>>  migration/meson.build  |   2 +-
>>>>  migration/ram.c        |  11 +-
>>>>  migration/ram.h        |  10 ++
>>>>  migration/trace-events |   8 +
>>>>  qapi/migration.json    |  67 ++++++++
>>>>  7 files changed, 583 insertions(+), 11 deletions(-)
>>>>  create mode 100644 migration/dirtyrate.c
>>>>  create mode 100644 migration/dirtyrate.h
>>>>
>>>> -- 
>>>> 1.8.3.1
>>>>
>>

Re: [PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[Dr. David Alan Gilbert]

Queued

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> v9 -> v10:
>     rename find_page_matched as find_block_matched
>     fix wrong termination condition in find_block_matched
>     add review-by for patches
> 
> v8 -> v9:
>     fix wrong index return of record_ramblock_hash_info
>     optimize variable name according to review
>     reset dirty_rate as -1
>     change returns of compare_page_hash_info to bool
> 
> v7 -> v8:
>     add atomic_read for dirtyrate status
>     add error_report if set dirtyrate state failed
>     change returns of save_ramblock_hash and record_ramblock_hash_info to bool
>     alloc ramblock dirtyinfo array at one time
>     add review-by for patches
> 
> v6 -> v7:
>     fix minior comments and coding style by review
>     add review-by for patches
> 
> v5 -> v6:
>     fix coding style according to review
>     use TARGET_PAGE_SIZE and TARGET_PAGE_BITS instead of self-defined macros
>     return start-time and calc-time by qmp command
> 
> v4 -> v5:
>     fix git apply failed due to meson-build
>     add review-by for patches in v3
> 
> v3 -> v4:
>     use crc32 to get hash result instead of md5
>     add DirtyRateStatus to denote calculation status
>     add some trace_calls to make it easier to debug
>     fix some comments accroding to review
> 
> v2 -> v3:
>     fix size_t compile warning
>     fix codestyle checked by checkpatch.pl
> 
> v1 -> v2:
>     use g_rand_new() to generate rand_buf
>     move RAMBLOCK_FOREACH_MIGRATABLE into migration/ram.h
>     add skip_sample_ramblock to filter sampled ramblock
>     fix multi-numa vm coredump when query dirtyrate
>     rename qapi interface and rename some structures and functions
>     succeed to compile by appling each patch
>     add test for migrating vm
> 
> Sometimes it is neccessary to evaluate dirty page rate before migration.
> Users could decide whether to proceed migration based on the evaluation
> in case of vm performance loss due to heavy workload.
> Unlikey simulating dirtylog sync which could do harm on runnning vm,
> we provide a sample-hash method to compare hash results for samping page.
> In this way, it would have hardly no impact on vm performance.
> 
> Evaluate the dirtypage rate both on running and migration vm.
> The VM specifications for migration are as follows:
> - VM use 4-K page;
> - the number of VCPU is 32;
> - the total memory is 32Gigabit;
> - use 'mempress' tool to pressurize VM(mempress 4096 1024);
> - migration bandwidth is 1GB/s
> 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> |                      |  running  |                  migrating                           |
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> | no mempress          |   4MB/s   |          8MB/s      (migrated success)               |
> -------------------------------------------------------------------------------------------
> | mempress 4096 1024   |  1060MB/s |     456MB/s ~ 1142MB/s (cpu throttle triggered)      |
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> | mempress 4096 4096   |  4114MB/s |     688MB/s ~ 4132MB/s (cpu throttle triggered)      |
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 
> Test dirtyrate by qmp command like this:
> 1.  virsh qemu-monitor-command [vmname] '{"execute":"calc-dirty-rate", "arguments": {"calc-time": [sleep-time]}}'; 
> 2.  sleep specific time which is a bit larger than sleep-time
> 3.  virsh qemu-monitor-command [vmname] '{"execute":"query-dirty-rate"}'
> 
> The qmp command returns like this:
> {"return":{"status":"measured","dirty-rate":374,"start-time":3718293,"calc-time":1},"id":"libvirt-15"}
> 
> Further test dirtyrate by libvirt api like this:
> virsh getdirtyrate [vmname] [sleep-time]
> 
> Chuan Zheng (12):
>   migration/dirtyrate: setup up query-dirtyrate framwork
>   migration/dirtyrate: add DirtyRateStatus to denote calculation status
>   migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info
>   migration/dirtyrate: Add dirtyrate statistics series functions
>   migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h
>   migration/dirtyrate: Record hash results for each sampled page
>   migration/dirtyrate: Compare page hash results for recorded sampled
>     page
>   migration/dirtyrate: skip sampling ramblock with size below
>     MIN_RAMBLOCK_SIZE
>   migration/dirtyrate: Implement set_sample_page_period() and
>     is_sample_period_valid()
>   migration/dirtyrate: Implement calculate_dirtyrate() function
>   migration/dirtyrate: Implement
>     qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
>   migration/dirtyrate: Add trace_calls to make it easier to debug
> 
>  migration/dirtyrate.c  | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
>  migration/dirtyrate.h  |  70 ++++++++
>  migration/meson.build  |   2 +-
>  migration/ram.c        |  11 +-
>  migration/ram.h        |  10 ++
>  migration/trace-events |   8 +
>  qapi/migration.json    |  67 ++++++++
>  7 files changed, 583 insertions(+), 11 deletions(-)
>  create mode 100644 migration/dirtyrate.c
>  create mode 100644 migration/dirtyrate.h
> 
> -- 
> 1.8.3.1
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v10 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[Eric Blake]

On 9/16/20 1:22 AM, Chuan Zheng wrote:
> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---

> +++ b/qapi/migration.json
> @@ -1737,3 +1737,53 @@
>   ##
>   { 'enum': 'DirtyRateStatus',
>     'data': [ 'unstarted', 'measuring', 'measured'] }
> +
> +##
> +# @DirtyRateInfo:
> +#
> +# Information about current dirty page rate of vm.
> +#
> +# @dirty-rate: @dirtyrate describing the dirty page rate of vm
> +#          in units of MB/s.
> +#          If this field return '-1', it means querying is not
> +#          start or not complete.

Grammar:

it means querying has not yet started or completed.

Should this field instead be optional, and omitted for those cases?  In 
which case, I'd suggest:

...in units of MB/s, present only when querying the rate has completed.

> +#
> +# @status: status containing dirtyrate query status includes
> +#          'unstarted' or 'measuring' or 'measured'
> +#
> +# @start-time: start time in units of second for calculation
> +#
> +# @calc-time: time in units of second for sample dirty pages
> +#
> +# Since: 5.2
> +#
> +##
> +{ 'struct': 'DirtyRateInfo',
> +  'data': {'dirty-rate': 'int64',
> +           'status': 'DirtyRateStatus',
> +           'start-time': 'int64',
> +           'calc-time': 'int64'} }
> +
> +##
> +# @calc-dirty-rate:
> +#
> +# start calculating dirty page rate for vm
> +#
> +# @calc-time: time in units of second for sample dirty pages
> +#
> +# Since: 5.2
> +#
> +# Example:
> +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
> +#
> +##
> +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
> +
> +##
> +# @query-dirty-rate:
> +#
> +# query dirty page rate in units of MB/s for vm
> +#
> +# Since: 5.2
> +##
> +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v10 00/12] *** A Method for evaluating dirty page rate ***

[Eric Blake]

On 9/23/20 1:04 PM, Dr. David Alan Gilbert wrote:
> Queued
> 
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> v9 -> v10:
>>      rename find_page_matched as find_block_matched
>>      fix wrong termination condition in find_block_matched
>>      add review-by for patches

Sorry for a late review, but I suggested a QAPI tweak to 11/12.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v10 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 9/16/20 1:22 AM, Chuan Zheng wrote:
> > Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
> > 
> > Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> > ---
> 
> > +++ b/qapi/migration.json
> > @@ -1737,3 +1737,53 @@
> >   ##
> >   { 'enum': 'DirtyRateStatus',
> >     'data': [ 'unstarted', 'measuring', 'measured'] }
> > +
> > +##
> > +# @DirtyRateInfo:
> > +#
> > +# Information about current dirty page rate of vm.
> > +#
> > +# @dirty-rate: @dirtyrate describing the dirty page rate of vm
> > +#          in units of MB/s.
> > +#          If this field return '-1', it means querying is not
> > +#          start or not complete.
> 
> Grammar:
> 
> it means querying has not yet started or completed.
> 
> Should this field instead be optional, and omitted for those cases?  In
> which case, I'd suggest:
> 
> ...in units of MB/s, present only when querying the rate has completed.

I've already got it queued; I'll fix up the grammar; if someone wants to
send a change to make it optional before this version freezes that's OK.

Dave

> 
> > +#
> > +# @status: status containing dirtyrate query status includes
> > +#          'unstarted' or 'measuring' or 'measured'
> > +#
> > +# @start-time: start time in units of second for calculation
> > +#
> > +# @calc-time: time in units of second for sample dirty pages
> > +#
> > +# Since: 5.2
> > +#
> > +##
> > +{ 'struct': 'DirtyRateInfo',
> > +  'data': {'dirty-rate': 'int64',
> > +           'status': 'DirtyRateStatus',
> > +           'start-time': 'int64',
> > +           'calc-time': 'int64'} }
> > +
> > +##
> > +# @calc-dirty-rate:
> > +#
> > +# start calculating dirty page rate for vm
> > +#
> > +# @calc-time: time in units of second for sample dirty pages
> > +#
> > +# Since: 5.2
> > +#
> > +# Example:
> > +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
> > +#
> > +##
> > +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
> > +
> > +##
> > +# @query-dirty-rate:
> > +#
> > +# query dirty page rate in units of MB/s for vm
> > +#
> > +# Since: 5.2
> > +##
> > +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v10 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[Zheng Chuan]

On 2020/9/24 3:03, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
>> On 9/16/20 1:22 AM, Chuan Zheng wrote:
>>> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
>>>
>>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>>> ---
>>
>>> +++ b/qapi/migration.json
>>> @@ -1737,3 +1737,53 @@
>>>   ##
>>>   { 'enum': 'DirtyRateStatus',
>>>     'data': [ 'unstarted', 'measuring', 'measured'] }
>>> +
>>> +##
>>> +# @DirtyRateInfo:
>>> +#
>>> +# Information about current dirty page rate of vm.
>>> +#
>>> +# @dirty-rate: @dirtyrate describing the dirty page rate of vm
>>> +#          in units of MB/s.
>>> +#          If this field return '-1', it means querying is not
>>> +#          start or not complete.
>>
>> Grammar:
>>
>> it means querying has not yet started or completed.
>>
>> Should this field instead be optional, and omitted for those cases?  In
>> which case, I'd suggest:
>>
>> ...in units of MB/s, present only when querying the rate has completed.
> 
Hi, Eric.
Thanks for your review.
Yeah, it could be optional.
and should it need keep start-time and calc-time when omit dirtyrate?
like:
{"return":{"status":"measuring","start-time":3718293,"calc-time":1},"id":"libvirt-15"}
or
{"return":{"status":"unstarted","start-time":3718293,"calc-time":1},"id":"libvirt-15"}

> I've already got it queued; I'll fix up the grammar; if someone wants to
> send a change to make it optional before this version freezes that's OK.
> 
> Dave
> 
>>
>>> +#
>>> +# @status: status containing dirtyrate query status includes
>>> +#          'unstarted' or 'measuring' or 'measured'
>>> +#
>>> +# @start-time: start time in units of second for calculation
>>> +#
>>> +# @calc-time: time in units of second for sample dirty pages
>>> +#
>>> +# Since: 5.2
>>> +#
>>> +##
>>> +{ 'struct': 'DirtyRateInfo',
>>> +  'data': {'dirty-rate': 'int64',
>>> +           'status': 'DirtyRateStatus',
>>> +           'start-time': 'int64',
>>> +           'calc-time': 'int64'} }
>>> +
>>> +##
>>> +# @calc-dirty-rate:
>>> +#
>>> +# start calculating dirty page rate for vm
>>> +#
>>> +# @calc-time: time in units of second for sample dirty pages
>>> +#
>>> +# Since: 5.2
>>> +#
>>> +# Example:
>>> +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
>>> +#
>>> +##
>>> +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
>>> +
>>> +##
>>> +# @query-dirty-rate:
>>> +#
>>> +# query dirty page rate in units of MB/s for vm
>>> +#
>>> +# Since: 5.2
>>> +##
>>> +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
>>>
>>
>> -- 
>> Eric Blake, Principal Software Engineer
>> Red Hat, Inc.           +1-919-301-3226
>> Virtualization:  qemu.org | libvirt.org