All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 00/12] *** A Method for evaluating dirty page rate ***
@ 2020-09-09 14:18 Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 01/12] migration/dirtyrate: setup up query-dirtyrate framwork Chuan Zheng
                   ` (11 more replies)
  0 siblings, 12 replies; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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
    get_sample_page_period()
  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  | 435 +++++++++++++++++++++++++++++++++++++++++++++++++
 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, 592 insertions(+), 11 deletions(-)
 create mode 100644 migration/dirtyrate.c
 create mode 100644 migration/dirtyrate.h

-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v7 01/12] migration/dirtyrate: setup up query-dirtyrate framwork
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status Chuan Zheng
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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>
---
 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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 01/12] migration/dirtyrate: setup up query-dirtyrate framwork Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-10 14:27   ` Li Qiang
  2020-09-09 14:18 ` [PATCH v7 03/12] migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info Chuan Zheng
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

add DirtyRateStatus to denote calculating status.

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

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index bf7fd24..0a3350a 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,17 @@ 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) {
+        return NULL;
+    }
 
     calculate_dirtyrate(config);
 
+    ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_MEASURING,
+                              DIRTY_RATE_STATUS_MEASURED);
     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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 03/12] migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 01/12] migration/dirtyrate: setup up query-dirtyrate framwork Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 04/12] migration/dirtyrate: Add dirtyrate statistics series functions Chuan Zheng
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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>
---
 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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 04/12] migration/dirtyrate: Add dirtyrate statistics series functions
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (2 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 03/12] migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-10 16:27   ` Dr. David Alan Gilbert
  2020-09-09 14:18 ` [PATCH v7 05/12] migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h Chuan Zheng
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

Add dirtyrate statistics functions to record/update dirtyrate info.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.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 0a3350a..5911ff0 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 = 0;
+    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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 05/12] migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (3 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 04/12] migration/dirtyrate: Add dirtyrate statistics series functions Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page Chuan Zheng
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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>
---
 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 5911ff0..d56cd93 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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (4 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 05/12] migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-10 13:51   ` Li Qiang
  2020-09-09 14:18 ` [PATCH v7 07/12] migration/dirtyrate: Compare page hash results for recorded " Chuan Zheng
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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>
---
 migration/dirtyrate.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index d56cd93..bc87269 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,130 @@ 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 int 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 0;
+    }
+
+    info->hash_result = g_try_malloc0_n(sample_pages_count,
+                                        sizeof(uint32_t));
+    if (!info->hash_result) {
+        return -1;
+    }
+
+    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 -1;
+    }
+
+    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 0;
+}
+
+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 struct RamblockDirtyInfo *
+alloc_ramblock_dirty_info(int *block_index,
+                          struct RamblockDirtyInfo *block_dinfo)
+{
+    struct RamblockDirtyInfo *info = NULL;
+    int index = *block_index;
+
+    if (!block_dinfo) {
+        index = 0;
+        block_dinfo = g_try_new(struct RamblockDirtyInfo, 1);
+    } else {
+        index++;
+        block_dinfo = g_try_realloc(block_dinfo, (index + 1) *
+                                    sizeof(struct RamblockDirtyInfo));
+    }
+    if (!block_dinfo) {
+        return NULL;
+    }
+
+    info = &block_dinfo[index];
+    *block_index = index;
+    memset(info, 0, sizeof(struct RamblockDirtyInfo));
+
+    return block_dinfo;
+}
+
+static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
+                                     struct DirtyRateConfig config,
+                                     int *block_index)
+{
+    struct RamblockDirtyInfo *info = NULL;
+    struct RamblockDirtyInfo *dinfo = NULL;
+    RAMBlock *block = NULL;
+    int index = 0;
+
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        dinfo = alloc_ramblock_dirty_info(&index, dinfo);
+        if (dinfo == NULL) {
+            return -1;
+        }
+        info = &dinfo[index];
+        get_ramblock_dirty_info(block, info, &config);
+        if (save_ramblock_hash(info) < 0) {
+            *block_dinfo = dinfo;
+            *block_index = index;
+            return -1;
+        }
+    }
+
+    *block_dinfo = dinfo;
+    *block_index = index;
+
+    return 0;
+}
+
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
     /* todo */
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 07/12] migration/dirtyrate: Compare page hash results for recorded sampled page
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (5 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-10 16:45   ` Dr. David Alan Gilbert
  2020-09-09 14:18 ` [PATCH v7 08/12] migration/dirtyrate: skip sampling ramblock with size below MIN_RAMBLOCK_SIZE Chuan Zheng
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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>
---
 migration/dirtyrate.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index bc87269..f5987d8 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -193,6 +193,69 @@ static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
     return 0;
 }
 
+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_page_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 int compare_page_hash_info(struct RamblockDirtyInfo *info,
+                                  int block_index)
+{
+    struct RamblockDirtyInfo *block_dinfo = NULL;
+    RAMBlock *block = NULL;
+
+    RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        block_dinfo = find_page_matched(block, block_index + 1, info);
+        if (block_dinfo == NULL) {
+            continue;
+        }
+        calc_page_dirty_rate(block_dinfo);
+        update_dirtyrate_stat(block_dinfo);
+    }
+
+    if (DirtyStat.total_sample_count == 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
     /* todo */
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 08/12] migration/dirtyrate: skip sampling ramblock with size below MIN_RAMBLOCK_SIZE
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (6 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 07/12] migration/dirtyrate: Compare page hash results for recorded " Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period() Chuan Zheng
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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>
---
 migration/dirtyrate.c | 18 ++++++++++++++++++
 migration/dirtyrate.h |  5 +++++
 2 files changed, 23 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index f5987d8..ae1959b 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -164,6 +164,18 @@ alloc_ramblock_dirty_info(int *block_index,
     return block_dinfo;
 }
 
+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 int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
                                      struct DirtyRateConfig config,
                                      int *block_index)
@@ -174,6 +186,9 @@ static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
     int index = 0;
 
     RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        if (skip_sample_ramblock(block)) {
+            continue;
+        }
         dinfo = alloc_ramblock_dirty_info(&index, dinfo);
         if (dinfo == NULL) {
             return -1;
@@ -241,6 +256,9 @@ static int compare_page_hash_info(struct RamblockDirtyInfo *info,
     RAMBlock *block = NULL;
 
     RAMBLOCK_FOREACH_MIGRATABLE(block) {
+        if (skip_sample_ramblock(block)) {
+            continue;
+        }
         block_dinfo = find_page_matched(block, block_index + 1, 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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period()
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (7 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 08/12] migration/dirtyrate: skip sampling ramblock with size below MIN_RAMBLOCK_SIZE Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-10 13:59   ` Li Qiang
  2020-09-09 14:18 ` [PATCH v7 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function Chuan Zheng
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

Implement set_sample_page_period()/get_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>
---
 migration/dirtyrate.c | 24 ++++++++++++++++++++++++
 migration/dirtyrate.h |  6 ++++++
 2 files changed, 30 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ae1959b..8a30261 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 get_sample_page_period(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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (8 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period() Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-10 17:26   ` Dr. David Alan Gilbert
  2020-09-09 14:18 ` [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function Chuan Zheng
  2020-09-09 14:18 ` [PATCH v7 12/12] migration/dirtyrate: Add trace_calls to make it easier to debug Chuan Zheng
  11 siblings, 1 reply; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

Implement calculate_dirtyrate() function.

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhuang <ann.zhuangyanying@huawei.com>
---
 migration/dirtyrate.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 8a30261..2f9ac34 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 struct RamblockDirtyInfo *
 alloc_ramblock_dirty_info(int *block_index,
                           struct RamblockDirtyInfo *block_dinfo)
@@ -300,8 +315,34 @@ static int compare_page_hash_info(struct RamblockDirtyInfo *info,
 
 static void calculate_dirtyrate(struct DirtyRateConfig config)
 {
-    /* todo */
-    return;
+    struct RamblockDirtyInfo *block_dinfo = NULL;
+    int block_index = 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_index) < 0) {
+        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_index) < 0) {
+        goto out;
+    }
+
+    update_dirtyrate(msec);
+
+out:
+    rcu_read_unlock();
+    free_ramblock_dirty_info(block_dinfo, block_index + 1);
+    rcu_unregister_thread();
 }
 
 void *get_dirtyrate_thread(void *arg)
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (9 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  2020-09-10 14:27   ` Li Qiang
  2020-09-10 17:36   ` Dr. David Alan Gilbert
  2020-09-09 14:18 ` [PATCH v7 12/12] migration/dirtyrate: Add trace_calls to make it easier to debug Chuan Zheng
  11 siblings, 2 replies; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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 2f9ac34..e9e9e35 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 (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;
@@ -331,6 +349,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_index) < 0) {
@@ -362,3 +382,45 @@ void *get_dirtyrate_thread(void *arg)
                               DIRTY_RATE_STATUS_MEASURED);
     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 (CalculatingState == DIRTY_RATE_STATUS_MEASURING) {
+        error_setg(errp, "the dirty rate is already being measured.");
+        return;
+    }
+
+    if (!get_sample_page_period(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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v7 12/12] migration/dirtyrate: Add trace_calls to make it easier to debug
  2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
                   ` (10 preceding siblings ...)
  2020-09-09 14:18 ` [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function Chuan Zheng
@ 2020-09-09 14:18 ` Chuan Zheng
  11 siblings, 0 replies; 25+ messages in thread
From: Chuan Zheng @ 2020-09-09 14:18 UTC (permalink / raw)
  To: quintela, eblake, dgilbert, berrange, dme
  Cc: alex.chen, ann.zhuangyanying, zhang.zhanghailiang, xiexiangyou,
	qemu-devel

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 e9e9e35..0286ff4 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 get_sample_page_period(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;
 }
 
@@ -227,6 +232,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;
     }
 
@@ -273,6 +280,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++;
         }
     }
@@ -298,6 +306,7 @@ find_page_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



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page
  2020-09-09 14:18 ` [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page Chuan Zheng
@ 2020-09-10 13:51   ` Li Qiang
  2020-09-13  2:59     ` Zheng Chuan
  0 siblings, 1 reply; 25+ messages in thread
From: Li Qiang @ 2020-09-10 13:51 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: Daniel P. Berrange, zhanghailiang, dme, Juan Quintela,
	Dr. David Alan Gilbert, xiexiangyou, Qemu Developers, AlexChen,
	ann.zhuangyanying

Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:14写道:
>
> 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>
> ---
>  migration/dirtyrate.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 125 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index d56cd93..bc87269 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,130 @@ 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 int 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 0;
> +    }
> +
> +    info->hash_result = g_try_malloc0_n(sample_pages_count,
> +                                        sizeof(uint32_t));
> +    if (!info->hash_result) {
> +        return -1;
> +    }
> +
> +    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 -1;
> +    }
> +
> +    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 0;
> +}
> +
> +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 struct RamblockDirtyInfo *
> +alloc_ramblock_dirty_info(int *block_index,
> +                          struct RamblockDirtyInfo *block_dinfo)
> +{
> +    struct RamblockDirtyInfo *info = NULL;
> +    int index = *block_index;
> +
> +    if (!block_dinfo) {
> +        index = 0;
> +        block_dinfo = g_try_new(struct RamblockDirtyInfo, 1);
> +    } else {
> +        index++;
> +        block_dinfo = g_try_realloc(block_dinfo, (index + 1) *
> +                                    sizeof(struct RamblockDirtyInfo));
> +    }
> +    if (!block_dinfo) {
> +        return NULL;

What if this case happens the 'index' has been increased?  but the
allocation is failed.

> +    }
> +
> +    info = &block_dinfo[index];
> +    *block_index = index;
> +    memset(info, 0, sizeof(struct RamblockDirtyInfo));
> +
> +    return block_dinfo;
> +}
> +
> +static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
> +                                     struct DirtyRateConfig config,
> +                                     int *block_index)
> +{
> +    struct RamblockDirtyInfo *info = NULL;
> +    struct RamblockDirtyInfo *dinfo = NULL;
> +    RAMBlock *block = NULL;
> +    int index = 0;
> +
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +        dinfo = alloc_ramblock_dirty_info(&index, dinfo);

Here for every migratable block, you call 'alloc_ramblock_dirty_info'.
This also complicates the 'alloc_ramblock_dirty_info' itself as:
1. you need to differentiate the first and other element.
2. you need to use two out parameter which seems can make confusion.

Could we allocates this array at onetime.  This maybe two iteration
the ram block list.
But I think may make the code more simple and clean.

Thank,s
Li Qiang

> +        if (dinfo == NULL) {
> +            return -1;
> +        }
> +        info = &dinfo[index];
> +        get_ramblock_dirty_info(block, info, &config);
> +        if (save_ramblock_hash(info) < 0) {
> +            *block_dinfo = dinfo;
> +            *block_index = index;

As the first comment, here 'index' seems not right?


Thanks,
Li Qiang
> +            return -1;
> +        }
> +    }
> +
> +    *block_dinfo = dinfo;
> +    *block_index = index;
> +
> +    return 0;
> +}
> +
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
>      /* todo */
> --
> 1.8.3.1
>
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period()
  2020-09-09 14:18 ` [PATCH v7 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period() Chuan Zheng
@ 2020-09-10 13:59   ` Li Qiang
  2020-09-13  3:00     ` Zheng Chuan
  0 siblings, 1 reply; 25+ messages in thread
From: Li Qiang @ 2020-09-10 13:59 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: Daniel P. Berrange, zhanghailiang, dme, Juan Quintela,
	Dr. David Alan Gilbert, xiexiangyou, Qemu Developers, AlexChen,
	ann.zhuangyanying

Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:15写道:
>
> Implement set_sample_page_period()/get_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>
> ---
>  migration/dirtyrate.c | 24 ++++++++++++++++++++++++
>  migration/dirtyrate.h |  6 ++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index ae1959b..8a30261 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 get_sample_page_period(int64_t sec)


This function name may confuse people the this will get the period.
But in fact you just check whether the 'period' is valid.
I think it is better to name it to be 'is_sample_period_valid' or
something meaningful.

Thanks,
Li Qiang

> +{
> +    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
>
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
  2020-09-09 14:18 ` [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function Chuan Zheng
@ 2020-09-10 14:27   ` Li Qiang
  2020-09-13  3:00     ` Zheng Chuan
  2020-09-10 17:36   ` Dr. David Alan Gilbert
  1 sibling, 1 reply; 25+ messages in thread
From: Li Qiang @ 2020-09-10 14:27 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: Daniel P. Berrange, zhanghailiang, dme, Juan Quintela,
	Dr. David Alan Gilbert, xiexiangyou, Qemu Developers, AlexChen,
	ann.zhuangyanying

Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:14写道:
>
> 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 2f9ac34..e9e9e35 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 (CalculatingState == DIRTY_RATE_STATUS_MEASURED) {

Should we use atomic read 'CalculatingState'? The qmp thread maybe run
with the sampled thread.

> +        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;
> @@ -331,6 +349,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_index) < 0) {
> @@ -362,3 +382,45 @@ void *get_dirtyrate_thread(void *arg)
>                                DIRTY_RATE_STATUS_MEASURED);
>      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 (CalculatingState == DIRTY_RATE_STATUS_MEASURING) {

atomic read?

> +        error_setg(errp, "the dirty rate is already being measured.");
> +        return;
> +    }
> +
> +    if (!get_sample_page_period(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
>
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status
  2020-09-09 14:18 ` [PATCH v7 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status Chuan Zheng
@ 2020-09-10 14:27   ` Li Qiang
  0 siblings, 0 replies; 25+ messages in thread
From: Li Qiang @ 2020-09-10 14:27 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: Daniel P. Berrange, zhanghailiang, dme, Juan Quintela,
	Dr. David Alan Gilbert, xiexiangyou, Qemu Developers, AlexChen,
	ann.zhuangyanying

Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:08写道:
>
> add DirtyRateStatus to denote calculating status.
>
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration/dirtyrate.c | 22 ++++++++++++++++++++++
>  qapi/migration.json   | 17 +++++++++++++++++
>  2 files changed, 39 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index bf7fd24..0a3350a 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,17 @@ 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) {
> +        return NULL;
> +    }
>
>      calculate_dirtyrate(config);
>
> +    ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_MEASURING,
> +                              DIRTY_RATE_STATUS_MEASURED);

Doesn't need to check the 'ret'?
Just curious you checked other places but not here.

Thanks,
Li Qiang

>      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
>
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 04/12] migration/dirtyrate: Add dirtyrate statistics series functions
  2020-09-09 14:18 ` [PATCH v7 04/12] migration/dirtyrate: Add dirtyrate statistics series functions Chuan Zheng
@ 2020-09-10 16:27   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 25+ messages in thread
From: Dr. David Alan Gilbert @ 2020-09-10 16:27 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: berrange, zhang.zhanghailiang, dme, quintela, qemu-devel,
	xiexiangyou, alex.chen, ann.zhuangyanying

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> 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>

> ---
>  migration/dirtyrate.c | 32 ++++++++++++++++++++++++++++++++
>  migration/dirtyrate.h | 12 ++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 0a3350a..5911ff0 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 = 0;
> +    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
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 07/12] migration/dirtyrate: Compare page hash results for recorded sampled page
  2020-09-09 14:18 ` [PATCH v7 07/12] migration/dirtyrate: Compare page hash results for recorded " Chuan Zheng
@ 2020-09-10 16:45   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 25+ messages in thread
From: Dr. David Alan Gilbert @ 2020-09-10 16:45 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: berrange, zhang.zhanghailiang, dme, quintela, qemu-devel,
	xiexiangyou, alex.chen, ann.zhuangyanying

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> 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 bc87269..f5987d8 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -193,6 +193,69 @@ static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
>      return 0;
>  }
>  
> +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_page_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 int compare_page_hash_info(struct RamblockDirtyInfo *info,
> +                                  int block_index)
> +{
> +    struct RamblockDirtyInfo *block_dinfo = NULL;
> +    RAMBlock *block = NULL;
> +
> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +        block_dinfo = find_page_matched(block, block_index + 1, info);
> +        if (block_dinfo == NULL) {
> +            continue;
> +        }
> +        calc_page_dirty_rate(block_dinfo);
> +        update_dirtyrate_stat(block_dinfo);
> +    }
> +
> +    if (DirtyStat.total_sample_count == 0) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
>      /* todo */
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function
  2020-09-09 14:18 ` [PATCH v7 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function Chuan Zheng
@ 2020-09-10 17:26   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 25+ messages in thread
From: Dr. David Alan Gilbert @ 2020-09-10 17:26 UTC (permalink / raw)
  To: Chuan Zheng
  Cc: berrange, zhang.zhanghailiang, dme, quintela, qemu-devel,
	xiexiangyou, alex.chen, ann.zhuangyanying

* Chuan Zheng (zhengchuan@huawei.com) wrote:
> 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>

> ---
>  migration/dirtyrate.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 8a30261..2f9ac34 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 struct RamblockDirtyInfo *
>  alloc_ramblock_dirty_info(int *block_index,
>                            struct RamblockDirtyInfo *block_dinfo)
> @@ -300,8 +315,34 @@ static int compare_page_hash_info(struct RamblockDirtyInfo *info,
>  
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
> -    /* todo */
> -    return;
> +    struct RamblockDirtyInfo *block_dinfo = NULL;
> +    int block_index = 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_index) < 0) {
> +        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_index) < 0) {
> +        goto out;
> +    }
> +
> +    update_dirtyrate(msec);
> +
> +out:
> +    rcu_read_unlock();
> +    free_ramblock_dirty_info(block_dinfo, block_index + 1);
> +    rcu_unregister_thread();
>  }
>  
>  void *get_dirtyrate_thread(void *arg)
> -- 
> 1.8.3.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
  2020-09-09 14:18 ` [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function Chuan Zheng
  2020-09-10 14:27   ` Li Qiang
@ 2020-09-10 17:36   ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 25+ messages in thread
From: Dr. David Alan Gilbert @ 2020-09-10 17:36 UTC (permalink / raw)
  To: Chuan Zheng, eblake
  Cc: berrange, zhang.zhanghailiang, dme, quintela, qemu-devel,
	xiexiangyou, alex.chen, ann.zhuangyanying

* 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>

THis is looking OK by me,
Eric: are you happy with the qapi stuff now?

Dave

> ---
>  migration/dirtyrate.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/migration.json   | 50 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 112 insertions(+)
> 
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 2f9ac34..e9e9e35 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 (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;
> @@ -331,6 +349,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_index) < 0) {
> @@ -362,3 +382,45 @@ void *get_dirtyrate_thread(void *arg)
>                                DIRTY_RATE_STATUS_MEASURED);
>      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 (CalculatingState == DIRTY_RATE_STATUS_MEASURING) {
> +        error_setg(errp, "the dirty rate is already being measured.");
> +        return;
> +    }
> +
> +    if (!get_sample_page_period(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



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page
  2020-09-10 13:51   ` Li Qiang
@ 2020-09-13  2:59     ` Zheng Chuan
  2020-09-14 11:25       ` Li Qiang
  0 siblings, 1 reply; 25+ messages in thread
From: Zheng Chuan @ 2020-09-13  2:59 UTC (permalink / raw)
  To: Li Qiang
  Cc: Daniel P. Berrange, zhanghailiang, dme, Juan Quintela,
	Dr. David Alan Gilbert, xiexiangyou, Qemu Developers, AlexChen,
	ann.zhuangyanying



On 2020/9/10 21:51, Li Qiang wrote:
> Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:14写道:
>>
>> 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>
>> ---
>>  migration/dirtyrate.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 125 insertions(+)
>>
>> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
>> index d56cd93..bc87269 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,130 @@ 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 int 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 0;
>> +    }
>> +
>> +    info->hash_result = g_try_malloc0_n(sample_pages_count,
>> +                                        sizeof(uint32_t));
>> +    if (!info->hash_result) {
>> +        return -1;
>> +    }
>> +
>> +    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 -1;
>> +    }
>> +
>> +    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 0;
>> +}
>> +
>> +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 struct RamblockDirtyInfo *
>> +alloc_ramblock_dirty_info(int *block_index,
>> +                          struct RamblockDirtyInfo *block_dinfo)
>> +{
>> +    struct RamblockDirtyInfo *info = NULL;
>> +    int index = *block_index;
>> +
>> +    if (!block_dinfo) {
>> +        index = 0;
>> +        block_dinfo = g_try_new(struct RamblockDirtyInfo, 1);
>> +    } else {
>> +        index++;
>> +        block_dinfo = g_try_realloc(block_dinfo, (index + 1) *
>> +                                    sizeof(struct RamblockDirtyInfo));
>> +    }
>> +    if (!block_dinfo) {
>> +        return NULL;
> 
> What if this case happens the 'index' has been increased?  but the
> allocation is failed.
> 
>> +    }
>> +
>> +    info = &block_dinfo[index];
>> +    *block_index = index;
>> +    memset(info, 0, sizeof(struct RamblockDirtyInfo));
>> +
>> +    return block_dinfo;
>> +}
>> +
>> +static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
>> +                                     struct DirtyRateConfig config,
>> +                                     int *block_index)
>> +{
>> +    struct RamblockDirtyInfo *info = NULL;
>> +    struct RamblockDirtyInfo *dinfo = NULL;
>> +    RAMBlock *block = NULL;
>> +    int index = 0;
>> +
>> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
>> +        dinfo = alloc_ramblock_dirty_info(&index, dinfo);
> 
> Here for every migratable block, you call 'alloc_ramblock_dirty_info'.
> This also complicates the 'alloc_ramblock_dirty_info' itself as:
> 1. you need to differentiate the first and other element.
> 2. you need to use two out parameter which seems can make confusion.
> 
> Could we allocates this array at onetime.  This maybe two iteration
> the ram block list.
> But I think may make the code more simple and clean.
> 
> Thank,s
> Li Qiang
> 
Hi, Qiang.
Thank you for your review.
I am not sure if i fully understand what's you mean:)
You mean we first record total index by first iteration
the ram block list and allocate array at onetime?

>> +        if (dinfo == NULL) {
>> +            return -1;
>> +        }
>> +        info = &dinfo[index];
>> +        get_ramblock_dirty_info(block, info, &config);
>> +        if (save_ramblock_hash(info) < 0) {
>> +            *block_dinfo = dinfo;
>> +            *block_index = index;
> 
> As the first comment, here 'index' seems not right?
> 
> 
> Thanks,
> Li Qiang
>> +            return -1;
>> +        }
>> +    }
>> +
>> +    *block_dinfo = dinfo;
>> +    *block_index = index;
>> +
>> +    return 0;
>> +}
>> +
>>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>>  {
>>      /* todo */
>> --
>> 1.8.3.1
>>
>>
> 
> .
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period()
  2020-09-10 13:59   ` Li Qiang
@ 2020-09-13  3:00     ` Zheng Chuan
  0 siblings, 0 replies; 25+ messages in thread
From: Zheng Chuan @ 2020-09-13  3:00 UTC (permalink / raw)
  To: Li Qiang
  Cc: Daniel P. Berrange, zhanghailiang, dme, Juan Quintela,
	Dr. David Alan Gilbert, xiexiangyou, Qemu Developers, AlexChen,
	ann.zhuangyanying



On 2020/9/10 21:59, Li Qiang wrote:
> Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:15写道:
>>
>> Implement set_sample_page_period()/get_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>
>> ---
>>  migration/dirtyrate.c | 24 ++++++++++++++++++++++++
>>  migration/dirtyrate.h |  6 ++++++
>>  2 files changed, 30 insertions(+)
>>
>> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
>> index ae1959b..8a30261 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 get_sample_page_period(int64_t sec)
> 
> 
> This function name may confuse people the this will get the period.
> But in fact you just check whether the 'period' is valid.
> I think it is better to name it to be 'is_sample_period_valid' or
> something meaningful.
> 
> Thanks,
> Li Qiang
> 
Sure, will optimize it in later patch update.

>> +{
>> +    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
>>
>>
> 
> .
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function
  2020-09-10 14:27   ` Li Qiang
@ 2020-09-13  3:00     ` Zheng Chuan
  0 siblings, 0 replies; 25+ messages in thread
From: Zheng Chuan @ 2020-09-13  3:00 UTC (permalink / raw)
  To: Li Qiang
  Cc: Daniel P. Berrange, zhanghailiang, dme, Juan Quintela,
	Dr. David Alan Gilbert, xiexiangyou, Qemu Developers, AlexChen,
	ann.zhuangyanying



On 2020/9/10 22:27, Li Qiang wrote:
> Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:14写道:
>>
>> 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 2f9ac34..e9e9e35 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 (CalculatingState == DIRTY_RATE_STATUS_MEASURED) {
> 
> Should we use atomic read 'CalculatingState'? The qmp thread maybe run
> with the sampled thread.
> 
OK, will optimize it in V8
>> +        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;
>> @@ -331,6 +349,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_index) < 0) {
>> @@ -362,3 +382,45 @@ void *get_dirtyrate_thread(void *arg)
>>                                DIRTY_RATE_STATUS_MEASURED);
>>      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 (CalculatingState == DIRTY_RATE_STATUS_MEASURING) {
> 
> atomic read?
> 
>> +        error_setg(errp, "the dirty rate is already being measured.");
>> +        return;
>> +    }
>> +
>> +    if (!get_sample_page_period(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
>>
>>
> 
> .
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page
  2020-09-13  2:59     ` Zheng Chuan
@ 2020-09-14 11:25       ` Li Qiang
  0 siblings, 0 replies; 25+ messages in thread
From: Li Qiang @ 2020-09-14 11:25 UTC (permalink / raw)
  To: Zheng Chuan
  Cc: Daniel P. Berrange, zhanghailiang, David Edmondson,
	Juan Quintela, Dr. David Alan Gilbert, xiexiangyou,
	Qemu Developers, AlexChen, ann.zhuangyanying

Zheng Chuan <zhengchuan@huawei.com> 于2020年9月13日周日 上午10:59写道:
>
>
>
> On 2020/9/10 21:51, Li Qiang wrote:
> > Chuan Zheng <zhengchuan@huawei.com> 于2020年9月9日周三 下午10:14写道:
> >>
> >> 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>
> >> ---
> >>  migration/dirtyrate.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 125 insertions(+)
> >>
> >> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> >> index d56cd93..bc87269 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,130 @@ 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 int 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 0;
> >> +    }
> >> +
> >> +    info->hash_result = g_try_malloc0_n(sample_pages_count,
> >> +                                        sizeof(uint32_t));
> >> +    if (!info->hash_result) {
> >> +        return -1;
> >> +    }
> >> +
> >> +    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 -1;
> >> +    }
> >> +
> >> +    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 0;
> >> +}
> >> +
> >> +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 struct RamblockDirtyInfo *
> >> +alloc_ramblock_dirty_info(int *block_index,
> >> +                          struct RamblockDirtyInfo *block_dinfo)
> >> +{
> >> +    struct RamblockDirtyInfo *info = NULL;
> >> +    int index = *block_index;
> >> +
> >> +    if (!block_dinfo) {
> >> +        index = 0;
> >> +        block_dinfo = g_try_new(struct RamblockDirtyInfo, 1);
> >> +    } else {
> >> +        index++;
> >> +        block_dinfo = g_try_realloc(block_dinfo, (index + 1) *
> >> +                                    sizeof(struct RamblockDirtyInfo));
> >> +    }
> >> +    if (!block_dinfo) {
> >> +        return NULL;
> >
> > What if this case happens the 'index' has been increased?  but the
> > allocation is failed.
> >
> >> +    }
> >> +
> >> +    info = &block_dinfo[index];
> >> +    *block_index = index;
> >> +    memset(info, 0, sizeof(struct RamblockDirtyInfo));
> >> +
> >> +    return block_dinfo;
> >> +}
> >> +
> >> +static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
> >> +                                     struct DirtyRateConfig config,
> >> +                                     int *block_index)
> >> +{
> >> +    struct RamblockDirtyInfo *info = NULL;
> >> +    struct RamblockDirtyInfo *dinfo = NULL;
> >> +    RAMBlock *block = NULL;
> >> +    int index = 0;
> >> +
> >> +    RAMBLOCK_FOREACH_MIGRATABLE(block) {
> >> +        dinfo = alloc_ramblock_dirty_info(&index, dinfo);
> >
> > Here for every migratable block, you call 'alloc_ramblock_dirty_info'.
> > This also complicates the 'alloc_ramblock_dirty_info' itself as:
> > 1. you need to differentiate the first and other element.
> > 2. you need to use two out parameter which seems can make confusion.
> >
> > Could we allocates this array at onetime.  This maybe two iteration
> > the ram block list.
> > But I think may make the code more simple and clean.
> >
> > Thank,s
> > Li Qiang
> >
> Hi, Qiang.
> Thank you for your review.
> I am not sure if i fully understand what's you mean:)
> You mean we first record total index by first iteration
> the ram block list and allocate array at onetime?

Hi Chuan,

Yes, this is what I mean.
I have just see your new patches, will review asap.

Thanks,
Li Qiang

>
> >> +        if (dinfo == NULL) {
> >> +            return -1;
> >> +        }
> >> +        info = &dinfo[index];
> >> +        get_ramblock_dirty_info(block, info, &config);
> >> +        if (save_ramblock_hash(info) < 0) {
> >> +            *block_dinfo = dinfo;
> >> +            *block_index = index;
> >
> > As the first comment, here 'index' seems not right?
> >
> >
> > Thanks,
> > Li Qiang
> >> +            return -1;
> >> +        }
> >> +    }
> >> +
> >> +    *block_dinfo = dinfo;
> >> +    *block_index = index;
> >> +
> >> +    return 0;
> >> +}
> >> +
> >>  static void calculate_dirtyrate(struct DirtyRateConfig config)
> >>  {
> >>      /* todo */
> >> --
> >> 1.8.3.1
> >>
> >>
> >
> > .
> >


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2020-09-14 11:26 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09 14:18 [PATCH v7 00/12] *** A Method for evaluating dirty page rate *** Chuan Zheng
2020-09-09 14:18 ` [PATCH v7 01/12] migration/dirtyrate: setup up query-dirtyrate framwork Chuan Zheng
2020-09-09 14:18 ` [PATCH v7 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status Chuan Zheng
2020-09-10 14:27   ` Li Qiang
2020-09-09 14:18 ` [PATCH v7 03/12] migration/dirtyrate: Add RamblockDirtyInfo to store sampled page info Chuan Zheng
2020-09-09 14:18 ` [PATCH v7 04/12] migration/dirtyrate: Add dirtyrate statistics series functions Chuan Zheng
2020-09-10 16:27   ` Dr. David Alan Gilbert
2020-09-09 14:18 ` [PATCH v7 05/12] migration/dirtyrate: move RAMBLOCK_FOREACH_MIGRATABLE into ram.h Chuan Zheng
2020-09-09 14:18 ` [PATCH v7 06/12] migration/dirtyrate: Record hash results for each sampled page Chuan Zheng
2020-09-10 13:51   ` Li Qiang
2020-09-13  2:59     ` Zheng Chuan
2020-09-14 11:25       ` Li Qiang
2020-09-09 14:18 ` [PATCH v7 07/12] migration/dirtyrate: Compare page hash results for recorded " Chuan Zheng
2020-09-10 16:45   ` Dr. David Alan Gilbert
2020-09-09 14:18 ` [PATCH v7 08/12] migration/dirtyrate: skip sampling ramblock with size below MIN_RAMBLOCK_SIZE Chuan Zheng
2020-09-09 14:18 ` [PATCH v7 09/12] migration/dirtyrate: Implement set_sample_page_period() and get_sample_page_period() Chuan Zheng
2020-09-10 13:59   ` Li Qiang
2020-09-13  3:00     ` Zheng Chuan
2020-09-09 14:18 ` [PATCH v7 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function Chuan Zheng
2020-09-10 17:26   ` Dr. David Alan Gilbert
2020-09-09 14:18 ` [PATCH v7 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function Chuan Zheng
2020-09-10 14:27   ` Li Qiang
2020-09-13  3:00     ` Zheng Chuan
2020-09-10 17:36   ` Dr. David Alan Gilbert
2020-09-09 14:18 ` [PATCH v7 12/12] migration/dirtyrate: Add trace_calls to make it easier to debug Chuan Zheng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.