All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu
@ 2021-05-31 17:02 ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Peter Xu, Hyman

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Since the Dirty Ring on QEMU part has been merged recently, how to use
this feature is under consideration.

In the scene of migration, it is valuable to provide a more accurante
interface to track dirty memory than existing one, so that the upper
layer application can make a wise decision, or whatever. More importantly,
dirtyrate info at the granualrity of vcpu could provide a possibility to
make migration convergent by imposing restriction on vcpu. With Dirty
Ring, we can calculate dirtyrate efficiently and cheaply.

The old interface implemented by sampling pages, it consumes cpu 
resource, and the larger guest memory size become, the more cpu resource
it consumes, namely, hard to scale. New interface has no such drawback.

Please review, thanks !

Best Regards !

Hyman Huang(黄勇) (6):
  KVM: add kvm_dirty_ring_enabled function
  KVM: introduce dirty_pages into CPUState
  migration/dirtyrate: add vcpu option for qmp calc-dirty-rate
  migration/dirtyrate: adjust struct DirtyRateStat
  migration/dirtyrate: check support of calculation for vcpu
  migration/dirtyrate: implement dirty-ring dirtyrate calculation

 accel/kvm/kvm-all.c    |  11 +++
 include/hw/core/cpu.h  |   2 +
 include/sysemu/kvm.h   |   1 +
 migration/dirtyrate.c  | 179 +++++++++++++++++++++++++++++++++++++----
 migration/dirtyrate.h  |  19 ++++-
 migration/trace-events |   1 +
 qapi/migration.json    |  28 ++++++-
 7 files changed, 222 insertions(+), 19 deletions(-)

-- 
2.24.3


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

* [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu
@ 2021-05-31 17:02 ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Juan Quintela, Hyman, Dr. David Alan Gilbert, Peter Xu,
	Paolo Bonzini

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Since the Dirty Ring on QEMU part has been merged recently, how to use
this feature is under consideration.

In the scene of migration, it is valuable to provide a more accurante
interface to track dirty memory than existing one, so that the upper
layer application can make a wise decision, or whatever. More importantly,
dirtyrate info at the granualrity of vcpu could provide a possibility to
make migration convergent by imposing restriction on vcpu. With Dirty
Ring, we can calculate dirtyrate efficiently and cheaply.

The old interface implemented by sampling pages, it consumes cpu 
resource, and the larger guest memory size become, the more cpu resource
it consumes, namely, hard to scale. New interface has no such drawback.

Please review, thanks !

Best Regards !

Hyman Huang(黄勇) (6):
  KVM: add kvm_dirty_ring_enabled function
  KVM: introduce dirty_pages into CPUState
  migration/dirtyrate: add vcpu option for qmp calc-dirty-rate
  migration/dirtyrate: adjust struct DirtyRateStat
  migration/dirtyrate: check support of calculation for vcpu
  migration/dirtyrate: implement dirty-ring dirtyrate calculation

 accel/kvm/kvm-all.c    |  11 +++
 include/hw/core/cpu.h  |   2 +
 include/sysemu/kvm.h   |   1 +
 migration/dirtyrate.c  | 179 +++++++++++++++++++++++++++++++++++++----
 migration/dirtyrate.h  |  19 ++++-
 migration/trace-events |   1 +
 qapi/migration.json    |  28 ++++++-
 7 files changed, 222 insertions(+), 19 deletions(-)

-- 
2.24.3



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

* [PATCH v1 1/6] KVM: add kvm_dirty_ring_enabled function
  2021-05-31 17:02 ` huangy81
@ 2021-05-31 17:03   ` huangy81
  -1 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Peter Xu, Hyman

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

introduce kvm_dirty_ring_enabled to show if kvm-reaper is working.
dirtyrate thread could use it to check if calculation can base on
dirty ring feature.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 accel/kvm/kvm-all.c  | 5 +++++
 include/sysemu/kvm.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c7ec538850..2e96b77b31 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2293,6 +2293,11 @@ bool kvm_vcpu_id_is_valid(int vcpu_id)
     return vcpu_id >= 0 && vcpu_id < kvm_max_vcpu_id(s);
 }
 
+bool kvm_dirty_ring_enabled(void)
+{
+    return kvm_state->kvm_dirty_ring_size ? true : false;
+}
+
 static int kvm_init(MachineState *ms)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d..7b22aeb6ae 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -547,4 +547,5 @@ bool kvm_cpu_check_are_resettable(void);
 
 bool kvm_arch_cpu_check_are_resettable(void);
 
+bool kvm_dirty_ring_enabled(void);
 #endif
-- 
2.24.3


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

* [PATCH v1 1/6] KVM: add kvm_dirty_ring_enabled function
@ 2021-05-31 17:03   ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Juan Quintela, Hyman, Dr. David Alan Gilbert, Peter Xu,
	Paolo Bonzini

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

introduce kvm_dirty_ring_enabled to show if kvm-reaper is working.
dirtyrate thread could use it to check if calculation can base on
dirty ring feature.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 accel/kvm/kvm-all.c  | 5 +++++
 include/sysemu/kvm.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c7ec538850..2e96b77b31 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2293,6 +2293,11 @@ bool kvm_vcpu_id_is_valid(int vcpu_id)
     return vcpu_id >= 0 && vcpu_id < kvm_max_vcpu_id(s);
 }
 
+bool kvm_dirty_ring_enabled(void)
+{
+    return kvm_state->kvm_dirty_ring_size ? true : false;
+}
+
 static int kvm_init(MachineState *ms)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d..7b22aeb6ae 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -547,4 +547,5 @@ bool kvm_cpu_check_are_resettable(void);
 
 bool kvm_arch_cpu_check_are_resettable(void);
 
+bool kvm_dirty_ring_enabled(void);
 #endif
-- 
2.24.3



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

* [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
  2021-05-31 17:02 ` huangy81
@ 2021-05-31 17:04   ` huangy81
  -1 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Peter Xu, Hyman

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

dirty_pages is used to calculate dirtyrate via dirty ring, when enabled,
kvm-reaper will increase the dirty pages after gfns being dirtied.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 accel/kvm/kvm-all.c   | 6 ++++++
 include/hw/core/cpu.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 2e96b77b31..52cba1b094 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -506,6 +506,9 @@ int kvm_init_vcpu(CPUState *cpu, Error **errp)
         }
     }
 
+    cpu->dirty_pages = 0;
+    cpu->stat_dirty_pages = false;
+
     ret = kvm_arch_init_vcpu(cpu);
     if (ret < 0) {
         error_setg_errno(errp, -ret,
@@ -739,6 +742,9 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu)
                                  cur->offset);
         dirty_gfn_set_collected(cur);
         trace_kvm_dirty_ring_page(cpu->cpu_index, fetch, cur->offset);
+        if (cpu->stat_dirty_pages) {
+            cpu->dirty_pages++;
+        }
         fetch++;
         count++;
     }
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 044f668a6e..973c193501 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -375,6 +375,8 @@ struct CPUState {
     struct kvm_run *kvm_run;
     struct kvm_dirty_gfn *kvm_dirty_gfns;
     uint32_t kvm_fetch_index;
+    uint64_t dirty_pages;
+    bool stat_dirty_pages;
 
     /* Used for events with 'vcpu' and *without* the 'disabled' properties */
     DECLARE_BITMAP(trace_dstate_delayed, CPU_TRACE_DSTATE_MAX_EVENTS);
-- 
2.24.3


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

* [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
@ 2021-05-31 17:04   ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Juan Quintela, Hyman, Dr. David Alan Gilbert, Peter Xu,
	Paolo Bonzini

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

dirty_pages is used to calculate dirtyrate via dirty ring, when enabled,
kvm-reaper will increase the dirty pages after gfns being dirtied.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 accel/kvm/kvm-all.c   | 6 ++++++
 include/hw/core/cpu.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 2e96b77b31..52cba1b094 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -506,6 +506,9 @@ int kvm_init_vcpu(CPUState *cpu, Error **errp)
         }
     }
 
+    cpu->dirty_pages = 0;
+    cpu->stat_dirty_pages = false;
+
     ret = kvm_arch_init_vcpu(cpu);
     if (ret < 0) {
         error_setg_errno(errp, -ret,
@@ -739,6 +742,9 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu)
                                  cur->offset);
         dirty_gfn_set_collected(cur);
         trace_kvm_dirty_ring_page(cpu->cpu_index, fetch, cur->offset);
+        if (cpu->stat_dirty_pages) {
+            cpu->dirty_pages++;
+        }
         fetch++;
         count++;
     }
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 044f668a6e..973c193501 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -375,6 +375,8 @@ struct CPUState {
     struct kvm_run *kvm_run;
     struct kvm_dirty_gfn *kvm_dirty_gfns;
     uint32_t kvm_fetch_index;
+    uint64_t dirty_pages;
+    bool stat_dirty_pages;
 
     /* Used for events with 'vcpu' and *without* the 'disabled' properties */
     DECLARE_BITMAP(trace_dstate_delayed, CPU_TRACE_DSTATE_MAX_EVENTS);
-- 
2.24.3



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

* [PATCH v1 3/6] migration/dirtyrate: add vcpu option for qmp calc-dirty-rate
  2021-05-31 17:02 ` huangy81
@ 2021-05-31 17:05   ` huangy81
  -1 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Peter Xu, Hyman

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

calculate dirtyrate for each vcpu if vcpu is true, add the
dirtyrate of each vcpu to the return value also.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c |  5 ++++-
 migration/dirtyrate.h |  1 +
 qapi/migration.json   | 28 ++++++++++++++++++++++++++--
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ccb98147e8..3c1a824a41 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -383,7 +383,10 @@ void *get_dirtyrate_thread(void *arg)
     return NULL;
 }
 
-void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
+void qmp_calc_dirty_rate(int64_t calc_time,
+                         bool has_vcpu,
+                         bool vcpu,
+                         Error **errp)
 {
     static struct DirtyRateConfig config;
     QemuThread thread;
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 6ec429534d..f20dd52d77 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -38,6 +38,7 @@
 struct DirtyRateConfig {
     uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
     int64_t sample_period_seconds; /* time duration between two sampling */
+    bool vcpu; /* calculate dirtyrate for each vcpu using dirty ring */
 };
 
 /*
diff --git a/qapi/migration.json b/qapi/migration.json
index 7a5bdf9a0d..896ebcb93b 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1708,6 +1708,21 @@
 { 'event': 'UNPLUG_PRIMARY',
   'data': { 'device-id': 'str' } }
 
+##
+# @DirtyRateVcpu:
+#
+# Dirty rate of vcpu.
+#
+# @id: vcpu index.
+#
+# @dirty-rate: dirty rate.
+#
+# Since: 6.1
+#
+##
+{ 'struct': 'DirtyRateVcpu',
+  'data': { 'id': 'int', 'dirty-rate': 'int64' } }
+
 ##
 # @DirtyRateStatus:
 #
@@ -1740,6 +1755,10 @@
 #
 # @calc-time: time in units of second for sample dirty pages
 #
+# @vcpu: calculate dirtyrate for each vcpu (Since 6.1)
+#
+# @vcpu-dirty-rate: dirtyrate for each vcpu (Since 6.1)
+#
 # Since: 5.2
 #
 ##
@@ -1747,7 +1766,9 @@
   'data': {'*dirty-rate': 'int64',
            'status': 'DirtyRateStatus',
            'start-time': 'int64',
-           'calc-time': 'int64'} }
+           'calc-time': 'int64',
+           '*vcpu': 'bool',
+           '*vcpu-dirty-rate': [ 'DirtyRateVcpu' ] } }
 
 ##
 # @calc-dirty-rate:
@@ -1756,13 +1777,16 @@
 #
 # @calc-time: time in units of second for sample dirty pages
 #
+# @vcpu: calculate vcpu dirty rate if true, the default value is
+#        false (since 6.1)
+#
 # Since: 5.2
 #
 # Example:
 #   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
 #
 ##
-{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
+{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64', '*vcpu': 'bool'} }
 
 ##
 # @query-dirty-rate:
-- 
2.24.3


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

* [PATCH v1 3/6] migration/dirtyrate: add vcpu option for qmp calc-dirty-rate
@ 2021-05-31 17:05   ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Juan Quintela, Hyman, Dr. David Alan Gilbert, Peter Xu,
	Paolo Bonzini

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

calculate dirtyrate for each vcpu if vcpu is true, add the
dirtyrate of each vcpu to the return value also.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c |  5 ++++-
 migration/dirtyrate.h |  1 +
 qapi/migration.json   | 28 ++++++++++++++++++++++++++--
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ccb98147e8..3c1a824a41 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -383,7 +383,10 @@ void *get_dirtyrate_thread(void *arg)
     return NULL;
 }
 
-void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
+void qmp_calc_dirty_rate(int64_t calc_time,
+                         bool has_vcpu,
+                         bool vcpu,
+                         Error **errp)
 {
     static struct DirtyRateConfig config;
     QemuThread thread;
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 6ec429534d..f20dd52d77 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -38,6 +38,7 @@
 struct DirtyRateConfig {
     uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
     int64_t sample_period_seconds; /* time duration between two sampling */
+    bool vcpu; /* calculate dirtyrate for each vcpu using dirty ring */
 };
 
 /*
diff --git a/qapi/migration.json b/qapi/migration.json
index 7a5bdf9a0d..896ebcb93b 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1708,6 +1708,21 @@
 { 'event': 'UNPLUG_PRIMARY',
   'data': { 'device-id': 'str' } }
 
+##
+# @DirtyRateVcpu:
+#
+# Dirty rate of vcpu.
+#
+# @id: vcpu index.
+#
+# @dirty-rate: dirty rate.
+#
+# Since: 6.1
+#
+##
+{ 'struct': 'DirtyRateVcpu',
+  'data': { 'id': 'int', 'dirty-rate': 'int64' } }
+
 ##
 # @DirtyRateStatus:
 #
@@ -1740,6 +1755,10 @@
 #
 # @calc-time: time in units of second for sample dirty pages
 #
+# @vcpu: calculate dirtyrate for each vcpu (Since 6.1)
+#
+# @vcpu-dirty-rate: dirtyrate for each vcpu (Since 6.1)
+#
 # Since: 5.2
 #
 ##
@@ -1747,7 +1766,9 @@
   'data': {'*dirty-rate': 'int64',
            'status': 'DirtyRateStatus',
            'start-time': 'int64',
-           'calc-time': 'int64'} }
+           'calc-time': 'int64',
+           '*vcpu': 'bool',
+           '*vcpu-dirty-rate': [ 'DirtyRateVcpu' ] } }
 
 ##
 # @calc-dirty-rate:
@@ -1756,13 +1777,16 @@
 #
 # @calc-time: time in units of second for sample dirty pages
 #
+# @vcpu: calculate vcpu dirty rate if true, the default value is
+#        false (since 6.1)
+#
 # Since: 5.2
 #
 # Example:
 #   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
 #
 ##
-{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
+{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64', '*vcpu': 'bool'} }
 
 ##
 # @query-dirty-rate:
-- 
2.24.3



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

* [PATCH v1 4/6] migration/dirtyrate: adjust struct DirtyRateStat
  2021-05-31 17:02 ` huangy81
@ 2021-05-31 17:05   ` huangy81
  -1 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Peter Xu, Hyman

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

use union to store stat data of two mutual exclusive methods.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c | 32 ++++++++++++++++++++------------
 migration/dirtyrate.h | 18 +++++++++++++++---
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 3c1a824a41..7952eb6117 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -78,31 +78,39 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
     return info;
 }
 
-static void init_dirtyrate_stat(int64_t start_time, int64_t calc_time)
+static void init_dirtyrate_stat(int64_t start_time,
+                                int64_t calc_time,
+                                struct DirtyRateConfig config)
 {
-    DirtyStat.total_dirty_samples = 0;
-    DirtyStat.total_sample_count = 0;
-    DirtyStat.total_block_mem_MB = 0;
     DirtyStat.dirty_rate = -1;
     DirtyStat.start_time = start_time;
     DirtyStat.calc_time = calc_time;
+
+    if (config.vcpu) {
+        DirtyStat.method.vcpu.nvcpu = -1;
+        DirtyStat.method.vcpu.rates = NULL;
+    } else {
+        DirtyStat.method.vm.total_dirty_samples = 0;
+        DirtyStat.method.vm.total_sample_count = 0;
+        DirtyStat.method.vm.total_block_mem_MB = 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;
+    DirtyStat.method.vm.total_dirty_samples += info->sample_dirty_count;
+    DirtyStat.method.vm.total_sample_count += info->sample_pages_count;
     /* size of total pages in MB */
-    DirtyStat.total_block_mem_MB += (info->ramblock_pages *
+    DirtyStat.method.vm.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;
+    uint64_t total_dirty_samples = DirtyStat.method.vm.total_dirty_samples;
+    uint64_t total_sample_count = DirtyStat.method.vm.total_sample_count;
+    uint64_t total_block_mem_MB = DirtyStat.method.vm.total_block_mem_MB;
 
     dirtyrate = total_dirty_samples * total_block_mem_MB *
                 1000 / (total_sample_count * msec);
@@ -315,7 +323,7 @@ static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
         update_dirtyrate_stat(block_dinfo);
     }
 
-    if (DirtyStat.total_sample_count == 0) {
+    if (DirtyStat.method.vm.total_sample_count == 0) {
         return false;
     }
 
@@ -371,7 +379,7 @@ void *get_dirtyrate_thread(void *arg)
 
     start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
     calc_time = config.sample_period_seconds;
-    init_dirtyrate_stat(start_time, calc_time);
+    init_dirtyrate_stat(start_time, calc_time, config);
 
     calculate_dirtyrate(config);
 
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index f20dd52d77..3ab8e81f42 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -54,16 +54,28 @@ struct RamblockDirtyInfo {
     uint32_t *hash_result; /* array of hash result for sampled pages */
 };
 
+typedef struct SampleVMStat {
+    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 */
+} SampleVMStat;
+
+typedef struct VcpuStat {
+    int nvcpu; /* number of vcpu */
+    DirtyRateVcpu *rates; /* array of dirty rate for each vcpu */
+} VcpuStat;
+
 /*
  * 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 */
+    union {
+        SampleVMStat vm;
+        VcpuStat vcpu;
+    } method;
 };
 
 void *get_dirtyrate_thread(void *arg);
-- 
2.24.3


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

* [PATCH v1 4/6] migration/dirtyrate: adjust struct DirtyRateStat
@ 2021-05-31 17:05   ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Juan Quintela, Hyman, Dr. David Alan Gilbert, Peter Xu,
	Paolo Bonzini

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

use union to store stat data of two mutual exclusive methods.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c | 32 ++++++++++++++++++++------------
 migration/dirtyrate.h | 18 +++++++++++++++---
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 3c1a824a41..7952eb6117 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -78,31 +78,39 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
     return info;
 }
 
-static void init_dirtyrate_stat(int64_t start_time, int64_t calc_time)
+static void init_dirtyrate_stat(int64_t start_time,
+                                int64_t calc_time,
+                                struct DirtyRateConfig config)
 {
-    DirtyStat.total_dirty_samples = 0;
-    DirtyStat.total_sample_count = 0;
-    DirtyStat.total_block_mem_MB = 0;
     DirtyStat.dirty_rate = -1;
     DirtyStat.start_time = start_time;
     DirtyStat.calc_time = calc_time;
+
+    if (config.vcpu) {
+        DirtyStat.method.vcpu.nvcpu = -1;
+        DirtyStat.method.vcpu.rates = NULL;
+    } else {
+        DirtyStat.method.vm.total_dirty_samples = 0;
+        DirtyStat.method.vm.total_sample_count = 0;
+        DirtyStat.method.vm.total_block_mem_MB = 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;
+    DirtyStat.method.vm.total_dirty_samples += info->sample_dirty_count;
+    DirtyStat.method.vm.total_sample_count += info->sample_pages_count;
     /* size of total pages in MB */
-    DirtyStat.total_block_mem_MB += (info->ramblock_pages *
+    DirtyStat.method.vm.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;
+    uint64_t total_dirty_samples = DirtyStat.method.vm.total_dirty_samples;
+    uint64_t total_sample_count = DirtyStat.method.vm.total_sample_count;
+    uint64_t total_block_mem_MB = DirtyStat.method.vm.total_block_mem_MB;
 
     dirtyrate = total_dirty_samples * total_block_mem_MB *
                 1000 / (total_sample_count * msec);
@@ -315,7 +323,7 @@ static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
         update_dirtyrate_stat(block_dinfo);
     }
 
-    if (DirtyStat.total_sample_count == 0) {
+    if (DirtyStat.method.vm.total_sample_count == 0) {
         return false;
     }
 
@@ -371,7 +379,7 @@ void *get_dirtyrate_thread(void *arg)
 
     start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
     calc_time = config.sample_period_seconds;
-    init_dirtyrate_stat(start_time, calc_time);
+    init_dirtyrate_stat(start_time, calc_time, config);
 
     calculate_dirtyrate(config);
 
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index f20dd52d77..3ab8e81f42 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -54,16 +54,28 @@ struct RamblockDirtyInfo {
     uint32_t *hash_result; /* array of hash result for sampled pages */
 };
 
+typedef struct SampleVMStat {
+    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 */
+} SampleVMStat;
+
+typedef struct VcpuStat {
+    int nvcpu; /* number of vcpu */
+    DirtyRateVcpu *rates; /* array of dirty rate for each vcpu */
+} VcpuStat;
+
 /*
  * 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 */
+    union {
+        SampleVMStat vm;
+        VcpuStat vcpu;
+    } method;
 };
 
 void *get_dirtyrate_thread(void *arg);
-- 
2.24.3



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

* [PATCH v1 5/6] migration/dirtyrate: check support of calculation for vcpu
  2021-05-31 17:02 ` huangy81
@ 2021-05-31 17:05   ` huangy81
  -1 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Peter Xu, Hyman

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

vcpu method only works when kvm dirty ring is enabled, use
kvm_dirty_ring_enabled to probe if dirty ring is enabled.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 7952eb6117..da6500c8ec 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -16,6 +16,7 @@
 #include "cpu.h"
 #include "exec/ramblock.h"
 #include "qemu/rcu_queue.h"
+#include "sysemu/kvm.h"
 #include "qapi/qapi-commands-migration.h"
 #include "ram.h"
 #include "trace.h"
@@ -415,6 +416,14 @@ void qmp_calc_dirty_rate(int64_t calc_time,
         return;
     }
 
+    /*
+     * Vcpu method only works when kvm dirty ring is enabled.
+     */
+    if (has_vcpu && vcpu && !kvm_dirty_ring_enabled()) {
+        error_setg(errp, "kvm dirty ring is disabled, use sample method.");
+        return;
+    }
+
     /*
      * Init calculation state as unstarted.
      */
@@ -427,6 +436,7 @@ void qmp_calc_dirty_rate(int64_t calc_time,
 
     config.sample_period_seconds = calc_time;
     config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+    config.vcpu = has_vcpu ? vcpu : false;
     qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
                        (void *)&config, QEMU_THREAD_DETACHED);
 }
-- 
2.24.3


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

* [PATCH v1 5/6] migration/dirtyrate: check support of calculation for vcpu
@ 2021-05-31 17:05   ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Juan Quintela, Hyman, Dr. David Alan Gilbert, Peter Xu,
	Paolo Bonzini

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

vcpu method only works when kvm dirty ring is enabled, use
kvm_dirty_ring_enabled to probe if dirty ring is enabled.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 7952eb6117..da6500c8ec 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -16,6 +16,7 @@
 #include "cpu.h"
 #include "exec/ramblock.h"
 #include "qemu/rcu_queue.h"
+#include "sysemu/kvm.h"
 #include "qapi/qapi-commands-migration.h"
 #include "ram.h"
 #include "trace.h"
@@ -415,6 +416,14 @@ void qmp_calc_dirty_rate(int64_t calc_time,
         return;
     }
 
+    /*
+     * Vcpu method only works when kvm dirty ring is enabled.
+     */
+    if (has_vcpu && vcpu && !kvm_dirty_ring_enabled()) {
+        error_setg(errp, "kvm dirty ring is disabled, use sample method.");
+        return;
+    }
+
     /*
      * Init calculation state as unstarted.
      */
@@ -427,6 +436,7 @@ void qmp_calc_dirty_rate(int64_t calc_time,
 
     config.sample_period_seconds = calc_time;
     config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+    config.vcpu = has_vcpu ? vcpu : false;
     qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
                        (void *)&config, QEMU_THREAD_DETACHED);
 }
-- 
2.24.3



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

* [PATCH v1 6/6] migration/dirtyrate: implement dirty-ring dirtyrate calculation
  2021-05-31 17:02 ` huangy81
@ 2021-05-31 17:06   ` huangy81
  -1 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Peter Xu, Hyman

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

use dirty ring feature to implement dirtyrate calculation.
to enable it, set vcpu option as true in qmp calc-dirty-rate.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c  | 146 ++++++++++++++++++++++++++++++++++++++---
 migration/trace-events |   1 +
 2 files changed, 139 insertions(+), 8 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index da6500c8ec..028c11d117 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -16,14 +16,22 @@
 #include "cpu.h"
 #include "exec/ramblock.h"
 #include "qemu/rcu_queue.h"
+#include "qemu/main-loop.h"
 #include "sysemu/kvm.h"
 #include "qapi/qapi-commands-migration.h"
 #include "ram.h"
 #include "trace.h"
 #include "dirtyrate.h"
 
+typedef enum {
+    CALC_NONE = 0,
+    CALC_DIRTY_RING,
+    CALC_SAMPLE_PAGES,
+} CalcMethod;
+
 static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
 static struct DirtyRateStat DirtyStat;
+static CalcMethod last_method = CALC_NONE;
 
 static int64_t set_sample_page_period(int64_t msec, int64_t initial_time)
 {
@@ -64,6 +72,7 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
 {
     int64_t dirty_rate = DirtyStat.dirty_rate;
     struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
+    DirtyRateVcpuList *head = NULL, **tail = &head;
 
     if (qatomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) {
         info->has_dirty_rate = true;
@@ -73,6 +82,22 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
     info->status = CalculatingState;
     info->start_time = DirtyStat.start_time;
     info->calc_time = DirtyStat.calc_time;
+    info->has_vcpu = true;
+
+    if (last_method == CALC_DIRTY_RING) {
+        int i = 0;
+        info->vcpu = true;
+        info->has_vcpu_dirty_rate = true;
+        for (i = 0; i < DirtyStat.method.vcpu.nvcpu; i++) {
+            DirtyRateVcpu *rate = g_malloc0(sizeof(DirtyRateVcpu));
+            rate->id = DirtyStat.method.vcpu.rates[i].id;
+            rate->dirty_rate = DirtyStat.method.vcpu.rates[i].dirty_rate;
+            QAPI_LIST_APPEND(tail, rate);
+        }
+        info->vcpu_dirty_rate = head;
+    } else {
+        info->vcpu = false;
+    }
 
     trace_query_dirty_rate_info(DirtyRateStatus_str(CalculatingState));
 
@@ -87,13 +112,29 @@ static void init_dirtyrate_stat(int64_t start_time,
     DirtyStat.start_time = start_time;
     DirtyStat.calc_time = calc_time;
 
-    if (config.vcpu) {
-        DirtyStat.method.vcpu.nvcpu = -1;
-        DirtyStat.method.vcpu.rates = NULL;
-    } else {
-        DirtyStat.method.vm.total_dirty_samples = 0;
-        DirtyStat.method.vm.total_sample_count = 0;
-        DirtyStat.method.vm.total_block_mem_MB = 0;
+    switch (last_method) {
+    case CALC_NONE:
+    case CALC_SAMPLE_PAGES:
+        if (config.vcpu) {
+            DirtyStat.method.vcpu.nvcpu = -1;
+            DirtyStat.method.vcpu.rates = NULL;
+        } else {
+            DirtyStat.method.vm.total_dirty_samples = 0;
+            DirtyStat.method.vm.total_sample_count = 0;
+            DirtyStat.method.vm.total_block_mem_MB = 0;
+        }
+        break;
+    case CALC_DIRTY_RING:
+        if (!config.vcpu) {
+            g_free(DirtyStat.method.vcpu.rates);
+            DirtyStat.method.vcpu.rates = NULL;
+            DirtyStat.method.vm.total_dirty_samples = 0;
+            DirtyStat.method.vm.total_sample_count = 0;
+            DirtyStat.method.vm.total_block_mem_MB = 0;
+        }
+        break;
+    default:
+        break;
     }
 }
 
@@ -331,7 +372,84 @@ static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
     return true;
 }
 
-static void calculate_dirtyrate(struct DirtyRateConfig config)
+static void stat_dirtypages(CPUState *cpu, bool start)
+{
+    cpu->stat_dirty_pages = start;
+}
+
+static void start_kvm_dirty_log(void)
+{
+    qemu_mutex_lock_iothread();
+    memory_global_dirty_log_start();
+    qemu_mutex_unlock_iothread();
+}
+
+static void stop_kvm_dirty_log(void)
+{
+    qemu_mutex_lock_iothread();
+    memory_global_dirty_log_stop();
+    qemu_mutex_unlock_iothread();
+}
+
+static int64_t do_calculate_dirtyrate_vcpu(CPUState *cpu)
+{
+    uint64_t memory_size_MB;
+    int64_t time_s;
+
+    memory_size_MB = (cpu->dirty_pages * TARGET_PAGE_SIZE) >> 20;
+    time_s = DirtyStat.calc_time;
+
+    return memory_size_MB / time_s;
+}
+
+static void calculate_dirtyrate_vcpu(struct DirtyRateConfig config)
+{
+    CPUState *cpu;
+    int64_t msec = 0;
+    int64_t start_time;
+    uint64_t dirtyrate = 0;
+    uint64_t dirtyrate_sum = 0;
+    int nvcpu, i = 0;
+
+    CPU_FOREACH(cpu) {
+        stat_dirtypages(cpu, true);
+        nvcpu++;
+    }
+
+    DirtyStat.method.vcpu.nvcpu = nvcpu;
+
+    if (last_method != CALC_DIRTY_RING) {
+        DirtyStat.method.vcpu.rates =
+            g_malloc0(sizeof(DirtyRateVcpu) * nvcpu);
+    }
+
+    start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    DirtyStat.start_time = start_time / 1000;
+
+    start_kvm_dirty_log();
+
+    msec = config.sample_period_seconds * 1000;
+    msec = set_sample_page_period(msec, start_time);
+    DirtyStat.calc_time = msec / 1000;
+
+    CPU_FOREACH(cpu) {
+        stat_dirtypages(cpu, false);
+    }
+
+    stop_kvm_dirty_log();
+
+    CPU_FOREACH(cpu) {
+        dirtyrate = do_calculate_dirtyrate_vcpu(cpu);
+        DirtyStat.method.vcpu.rates[i].id = cpu->cpu_index;
+        DirtyStat.method.vcpu.rates[i].dirty_rate = dirtyrate;
+        dirtyrate_sum += dirtyrate;
+        i++;
+    }
+
+    DirtyStat.dirty_rate = dirtyrate_sum / nvcpu;
+}
+
+static void calculate_dirtyrate_sample_vm(struct DirtyRateConfig config)
 {
     struct RamblockDirtyInfo *block_dinfo = NULL;
     int block_count = 0;
@@ -364,6 +482,18 @@ out:
     rcu_unregister_thread();
 }
 
+static void calculate_dirtyrate(struct DirtyRateConfig config)
+{
+    if (config.vcpu) {
+        calculate_dirtyrate_vcpu(config);
+        last_method = CALC_DIRTY_RING;
+    } else {
+        calculate_dirtyrate_sample_vm(config);
+        last_method = CALC_SAMPLE_PAGES;
+    }
+    trace_calculate_dirtyrate(DirtyStat.dirty_rate);
+}
+
 void *get_dirtyrate_thread(void *arg)
 {
     struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
diff --git a/migration/trace-events b/migration/trace-events
index 668c562fed..5a80b39a62 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -330,6 +330,7 @@ get_ramblock_vfn_hash(const char *idstr, uint64_t vfn, uint32_t crc) "ramblock n
 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"
+calculate_dirtyrate(int64_t dirtyrate) "dirty rate: %" PRIi64
 
 # block.c
 migration_block_init_shared(const char *blk_device_name) "Start migration for %s with shared base image"
-- 
2.24.3


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

* [PATCH v1 6/6] migration/dirtyrate: implement dirty-ring dirtyrate calculation
@ 2021-05-31 17:06   ` huangy81
  0 siblings, 0 replies; 24+ messages in thread
From: huangy81 @ 2021-05-31 17:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: kvm, Juan Quintela, Hyman, Dr. David Alan Gilbert, Peter Xu,
	Paolo Bonzini

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

use dirty ring feature to implement dirtyrate calculation.
to enable it, set vcpu option as true in qmp calc-dirty-rate.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 migration/dirtyrate.c  | 146 ++++++++++++++++++++++++++++++++++++++---
 migration/trace-events |   1 +
 2 files changed, 139 insertions(+), 8 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index da6500c8ec..028c11d117 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -16,14 +16,22 @@
 #include "cpu.h"
 #include "exec/ramblock.h"
 #include "qemu/rcu_queue.h"
+#include "qemu/main-loop.h"
 #include "sysemu/kvm.h"
 #include "qapi/qapi-commands-migration.h"
 #include "ram.h"
 #include "trace.h"
 #include "dirtyrate.h"
 
+typedef enum {
+    CALC_NONE = 0,
+    CALC_DIRTY_RING,
+    CALC_SAMPLE_PAGES,
+} CalcMethod;
+
 static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
 static struct DirtyRateStat DirtyStat;
+static CalcMethod last_method = CALC_NONE;
 
 static int64_t set_sample_page_period(int64_t msec, int64_t initial_time)
 {
@@ -64,6 +72,7 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
 {
     int64_t dirty_rate = DirtyStat.dirty_rate;
     struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
+    DirtyRateVcpuList *head = NULL, **tail = &head;
 
     if (qatomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) {
         info->has_dirty_rate = true;
@@ -73,6 +82,22 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
     info->status = CalculatingState;
     info->start_time = DirtyStat.start_time;
     info->calc_time = DirtyStat.calc_time;
+    info->has_vcpu = true;
+
+    if (last_method == CALC_DIRTY_RING) {
+        int i = 0;
+        info->vcpu = true;
+        info->has_vcpu_dirty_rate = true;
+        for (i = 0; i < DirtyStat.method.vcpu.nvcpu; i++) {
+            DirtyRateVcpu *rate = g_malloc0(sizeof(DirtyRateVcpu));
+            rate->id = DirtyStat.method.vcpu.rates[i].id;
+            rate->dirty_rate = DirtyStat.method.vcpu.rates[i].dirty_rate;
+            QAPI_LIST_APPEND(tail, rate);
+        }
+        info->vcpu_dirty_rate = head;
+    } else {
+        info->vcpu = false;
+    }
 
     trace_query_dirty_rate_info(DirtyRateStatus_str(CalculatingState));
 
@@ -87,13 +112,29 @@ static void init_dirtyrate_stat(int64_t start_time,
     DirtyStat.start_time = start_time;
     DirtyStat.calc_time = calc_time;
 
-    if (config.vcpu) {
-        DirtyStat.method.vcpu.nvcpu = -1;
-        DirtyStat.method.vcpu.rates = NULL;
-    } else {
-        DirtyStat.method.vm.total_dirty_samples = 0;
-        DirtyStat.method.vm.total_sample_count = 0;
-        DirtyStat.method.vm.total_block_mem_MB = 0;
+    switch (last_method) {
+    case CALC_NONE:
+    case CALC_SAMPLE_PAGES:
+        if (config.vcpu) {
+            DirtyStat.method.vcpu.nvcpu = -1;
+            DirtyStat.method.vcpu.rates = NULL;
+        } else {
+            DirtyStat.method.vm.total_dirty_samples = 0;
+            DirtyStat.method.vm.total_sample_count = 0;
+            DirtyStat.method.vm.total_block_mem_MB = 0;
+        }
+        break;
+    case CALC_DIRTY_RING:
+        if (!config.vcpu) {
+            g_free(DirtyStat.method.vcpu.rates);
+            DirtyStat.method.vcpu.rates = NULL;
+            DirtyStat.method.vm.total_dirty_samples = 0;
+            DirtyStat.method.vm.total_sample_count = 0;
+            DirtyStat.method.vm.total_block_mem_MB = 0;
+        }
+        break;
+    default:
+        break;
     }
 }
 
@@ -331,7 +372,84 @@ static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
     return true;
 }
 
-static void calculate_dirtyrate(struct DirtyRateConfig config)
+static void stat_dirtypages(CPUState *cpu, bool start)
+{
+    cpu->stat_dirty_pages = start;
+}
+
+static void start_kvm_dirty_log(void)
+{
+    qemu_mutex_lock_iothread();
+    memory_global_dirty_log_start();
+    qemu_mutex_unlock_iothread();
+}
+
+static void stop_kvm_dirty_log(void)
+{
+    qemu_mutex_lock_iothread();
+    memory_global_dirty_log_stop();
+    qemu_mutex_unlock_iothread();
+}
+
+static int64_t do_calculate_dirtyrate_vcpu(CPUState *cpu)
+{
+    uint64_t memory_size_MB;
+    int64_t time_s;
+
+    memory_size_MB = (cpu->dirty_pages * TARGET_PAGE_SIZE) >> 20;
+    time_s = DirtyStat.calc_time;
+
+    return memory_size_MB / time_s;
+}
+
+static void calculate_dirtyrate_vcpu(struct DirtyRateConfig config)
+{
+    CPUState *cpu;
+    int64_t msec = 0;
+    int64_t start_time;
+    uint64_t dirtyrate = 0;
+    uint64_t dirtyrate_sum = 0;
+    int nvcpu, i = 0;
+
+    CPU_FOREACH(cpu) {
+        stat_dirtypages(cpu, true);
+        nvcpu++;
+    }
+
+    DirtyStat.method.vcpu.nvcpu = nvcpu;
+
+    if (last_method != CALC_DIRTY_RING) {
+        DirtyStat.method.vcpu.rates =
+            g_malloc0(sizeof(DirtyRateVcpu) * nvcpu);
+    }
+
+    start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    DirtyStat.start_time = start_time / 1000;
+
+    start_kvm_dirty_log();
+
+    msec = config.sample_period_seconds * 1000;
+    msec = set_sample_page_period(msec, start_time);
+    DirtyStat.calc_time = msec / 1000;
+
+    CPU_FOREACH(cpu) {
+        stat_dirtypages(cpu, false);
+    }
+
+    stop_kvm_dirty_log();
+
+    CPU_FOREACH(cpu) {
+        dirtyrate = do_calculate_dirtyrate_vcpu(cpu);
+        DirtyStat.method.vcpu.rates[i].id = cpu->cpu_index;
+        DirtyStat.method.vcpu.rates[i].dirty_rate = dirtyrate;
+        dirtyrate_sum += dirtyrate;
+        i++;
+    }
+
+    DirtyStat.dirty_rate = dirtyrate_sum / nvcpu;
+}
+
+static void calculate_dirtyrate_sample_vm(struct DirtyRateConfig config)
 {
     struct RamblockDirtyInfo *block_dinfo = NULL;
     int block_count = 0;
@@ -364,6 +482,18 @@ out:
     rcu_unregister_thread();
 }
 
+static void calculate_dirtyrate(struct DirtyRateConfig config)
+{
+    if (config.vcpu) {
+        calculate_dirtyrate_vcpu(config);
+        last_method = CALC_DIRTY_RING;
+    } else {
+        calculate_dirtyrate_sample_vm(config);
+        last_method = CALC_SAMPLE_PAGES;
+    }
+    trace_calculate_dirtyrate(DirtyStat.dirty_rate);
+}
+
 void *get_dirtyrate_thread(void *arg)
 {
     struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
diff --git a/migration/trace-events b/migration/trace-events
index 668c562fed..5a80b39a62 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -330,6 +330,7 @@ get_ramblock_vfn_hash(const char *idstr, uint64_t vfn, uint32_t crc) "ramblock n
 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"
+calculate_dirtyrate(int64_t dirtyrate) "dirty rate: %" PRIi64
 
 # block.c
 migration_block_init_shared(const char *blk_device_name) "Start migration for %s with shared base image"
-- 
2.24.3



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

* Re: [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu
  2021-05-31 17:02 ` huangy81
@ 2021-06-01 21:54   ` Peter Xu
  -1 siblings, 0 replies; 24+ messages in thread
From: Peter Xu @ 2021-06-01 21:54 UTC (permalink / raw)
  To: huangy81
  Cc: qemu-devel, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Paolo Bonzini, Chuan Zheng

On Tue, Jun 01, 2021 at 01:02:45AM +0800, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> Since the Dirty Ring on QEMU part has been merged recently, how to use
> this feature is under consideration.
> 
> In the scene of migration, it is valuable to provide a more accurante
> interface to track dirty memory than existing one, so that the upper
> layer application can make a wise decision, or whatever. More importantly,
> dirtyrate info at the granualrity of vcpu could provide a possibility to
> make migration convergent by imposing restriction on vcpu. With Dirty
> Ring, we can calculate dirtyrate efficiently and cheaply.
> 
> The old interface implemented by sampling pages, it consumes cpu 
> resource, and the larger guest memory size become, the more cpu resource
> it consumes, namely, hard to scale. New interface has no such drawback.

Yong,

Thanks for working on this!

Some high-level comments:

- The layout of the patch looks a bit odd.  E.g., you introduced the new "vcpu"
  qmp parameter in patch 3, however it's not yet implemented, meanwhile I feel
  like you squashed mostly all the rest into patch 6.  It's okay to use a
  single big patch, but IMHO better to not declare that flag in QMP before it's
  working, so ideally that should be the last patch to do that.

  From that POV: patch 1/2/4 look ok to be separated; perhaps squash patch
  3/5/6 into one single patch to enable the new method as the last one?

- You used "vcpu" across the patchset to show the per-vcpu new method.  Shall
  we rename it globally to "per_vcpu" or "vcpu_based"?  A raw "vcpu" looks more
  like a struct pointer not a boolean.

- Using memory_global_dirty_log_start|stop() may not be wise too IMHO, at least
  we need to make sure it's not during migration, otherwise we could call the
  stop() before migration ends then that'll be a problem..

  Maybe we can start to make global_dirty_log a bitmask? Then we define:

    GLOBAL_DIRTY_MIGRATION
    GLOBAL_DIRTY_DIRTY_RATE

  All references to global_dirty_log should mostly be untouched because any bit
  set there should justify that global dirty logging is enabled (either for
  migration or for dirty rate measurement).

  Migration starting half-way of dirty rate measurement seems okay too even
  taking things like init-all-set into account, afaict.. as long as dirty rate
  code never touches the qemu dirty bitmap, but only do the accounting when
  collecting the pages...

  Feel free to think more about it on any other potential conflict with
  migration, but in general seems working to me.

- Would you consider picking up my HMP patch and let HMP work from the 1st day?

- Please Cc the author of dirty rate too (Chuan Zheng <zhengchuan@huawei.com>),
  while I already started to do so in this email.

Thanks,

-- 
Peter Xu


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

* Re: [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu
@ 2021-06-01 21:54   ` Peter Xu
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Xu @ 2021-06-01 21:54 UTC (permalink / raw)
  To: huangy81
  Cc: kvm, Juan Quintela, Dr. David Alan Gilbert, qemu-devel,
	Chuan Zheng, Paolo Bonzini

On Tue, Jun 01, 2021 at 01:02:45AM +0800, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> Since the Dirty Ring on QEMU part has been merged recently, how to use
> this feature is under consideration.
> 
> In the scene of migration, it is valuable to provide a more accurante
> interface to track dirty memory than existing one, so that the upper
> layer application can make a wise decision, or whatever. More importantly,
> dirtyrate info at the granualrity of vcpu could provide a possibility to
> make migration convergent by imposing restriction on vcpu. With Dirty
> Ring, we can calculate dirtyrate efficiently and cheaply.
> 
> The old interface implemented by sampling pages, it consumes cpu 
> resource, and the larger guest memory size become, the more cpu resource
> it consumes, namely, hard to scale. New interface has no such drawback.

Yong,

Thanks for working on this!

Some high-level comments:

- The layout of the patch looks a bit odd.  E.g., you introduced the new "vcpu"
  qmp parameter in patch 3, however it's not yet implemented, meanwhile I feel
  like you squashed mostly all the rest into patch 6.  It's okay to use a
  single big patch, but IMHO better to not declare that flag in QMP before it's
  working, so ideally that should be the last patch to do that.

  From that POV: patch 1/2/4 look ok to be separated; perhaps squash patch
  3/5/6 into one single patch to enable the new method as the last one?

- You used "vcpu" across the patchset to show the per-vcpu new method.  Shall
  we rename it globally to "per_vcpu" or "vcpu_based"?  A raw "vcpu" looks more
  like a struct pointer not a boolean.

- Using memory_global_dirty_log_start|stop() may not be wise too IMHO, at least
  we need to make sure it's not during migration, otherwise we could call the
  stop() before migration ends then that'll be a problem..

  Maybe we can start to make global_dirty_log a bitmask? Then we define:

    GLOBAL_DIRTY_MIGRATION
    GLOBAL_DIRTY_DIRTY_RATE

  All references to global_dirty_log should mostly be untouched because any bit
  set there should justify that global dirty logging is enabled (either for
  migration or for dirty rate measurement).

  Migration starting half-way of dirty rate measurement seems okay too even
  taking things like init-all-set into account, afaict.. as long as dirty rate
  code never touches the qemu dirty bitmap, but only do the accounting when
  collecting the pages...

  Feel free to think more about it on any other potential conflict with
  migration, but in general seems working to me.

- Would you consider picking up my HMP patch and let HMP work from the 1st day?

- Please Cc the author of dirty rate too (Chuan Zheng <zhengchuan@huawei.com>),
  while I already started to do so in this email.

Thanks,

-- 
Peter Xu



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

* Re: [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
  2021-05-31 17:04   ` huangy81
@ 2021-06-01 23:20     ` Peter Xu
  -1 siblings, 0 replies; 24+ messages in thread
From: Peter Xu @ 2021-06-01 23:20 UTC (permalink / raw)
  To: huangy81
  Cc: qemu-devel, Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert

On Tue, Jun 01, 2021 at 01:04:06AM +0800, huangy81@chinatelecom.cn wrote:
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 044f668a6e..973c193501 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -375,6 +375,8 @@ struct CPUState {
>      struct kvm_run *kvm_run;
>      struct kvm_dirty_gfn *kvm_dirty_gfns;
>      uint32_t kvm_fetch_index;
> +    uint64_t dirty_pages;
> +    bool stat_dirty_pages;

Shall we make this bool a global one?  As I don't think we'll be able to only
enable it on a subset of cpus?

-- 
Peter Xu


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

* Re: [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
@ 2021-06-01 23:20     ` Peter Xu
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Xu @ 2021-06-01 23:20 UTC (permalink / raw)
  To: huangy81
  Cc: Paolo Bonzini, Dr. David Alan Gilbert, qemu-devel, kvm, Juan Quintela

On Tue, Jun 01, 2021 at 01:04:06AM +0800, huangy81@chinatelecom.cn wrote:
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 044f668a6e..973c193501 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -375,6 +375,8 @@ struct CPUState {
>      struct kvm_run *kvm_run;
>      struct kvm_dirty_gfn *kvm_dirty_gfns;
>      uint32_t kvm_fetch_index;
> +    uint64_t dirty_pages;
> +    bool stat_dirty_pages;

Shall we make this bool a global one?  As I don't think we'll be able to only
enable it on a subset of cpus?

-- 
Peter Xu



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

* Re: [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
  2021-06-01 23:20     ` Peter Xu
@ 2021-06-02  0:27       ` Hyman Huang
  -1 siblings, 0 replies; 24+ messages in thread
From: Hyman Huang @ 2021-06-02  0:27 UTC (permalink / raw)
  To: Peter Xu
  Cc: qemu-devel, Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert



在 2021/6/2 7:20, Peter Xu 写道:
> On Tue, Jun 01, 2021 at 01:04:06AM +0800, huangy81@chinatelecom.cn wrote:
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index 044f668a6e..973c193501 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -375,6 +375,8 @@ struct CPUState {
>>       struct kvm_run *kvm_run;
>>       struct kvm_dirty_gfn *kvm_dirty_gfns;
>>       uint32_t kvm_fetch_index;
>> +    uint64_t dirty_pages;
>> +    bool stat_dirty_pages;
> 
> Shall we make this bool a global one?  As I don't think we'll be able to only
> enable it on a subset of cpus?
Yes, it's a reasonable advice, i'll apply this on the next version
> 

-- 
Best regard

Hyman Huang(黄勇)

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

* Re: [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
@ 2021-06-02  0:27       ` Hyman Huang
  0 siblings, 0 replies; 24+ messages in thread
From: Hyman Huang @ 2021-06-02  0:27 UTC (permalink / raw)
  To: Peter Xu
  Cc: Paolo Bonzini, Dr. David Alan Gilbert, qemu-devel, kvm, Juan Quintela



在 2021/6/2 7:20, Peter Xu 写道:
> On Tue, Jun 01, 2021 at 01:04:06AM +0800, huangy81@chinatelecom.cn wrote:
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index 044f668a6e..973c193501 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -375,6 +375,8 @@ struct CPUState {
>>       struct kvm_run *kvm_run;
>>       struct kvm_dirty_gfn *kvm_dirty_gfns;
>>       uint32_t kvm_fetch_index;
>> +    uint64_t dirty_pages;
>> +    bool stat_dirty_pages;
> 
> Shall we make this bool a global one?  As I don't think we'll be able to only
> enable it on a subset of cpus?
Yes, it's a reasonable advice, i'll apply this on the next version
> 

-- 
Best regard

Hyman Huang(黄勇)


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

* Re: [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu
  2021-06-01 21:54   ` Peter Xu
@ 2021-06-02  0:51     ` Hyman Huang
  -1 siblings, 0 replies; 24+ messages in thread
From: Hyman Huang @ 2021-06-02  0:51 UTC (permalink / raw)
  To: Peter Xu
  Cc: qemu-devel, kvm, Juan Quintela, Dr. David Alan Gilbert,
	Paolo Bonzini, Chuan Zheng, 张子健



在 2021/6/2 5:54, Peter Xu 写道:
> On Tue, Jun 01, 2021 at 01:02:45AM +0800, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> Since the Dirty Ring on QEMU part has been merged recently, how to use
>> this feature is under consideration.
>>
>> In the scene of migration, it is valuable to provide a more accurante
>> interface to track dirty memory than existing one, so that the upper
>> layer application can make a wise decision, or whatever. More importantly,
>> dirtyrate info at the granualrity of vcpu could provide a possibility to
>> make migration convergent by imposing restriction on vcpu. With Dirty
>> Ring, we can calculate dirtyrate efficiently and cheaply.
>>
>> The old interface implemented by sampling pages, it consumes cpu
>> resource, and the larger guest memory size become, the more cpu resource
>> it consumes, namely, hard to scale. New interface has no such drawback.
> 
> Yong,
> 
> Thanks for working on this!
> 
> Some high-level comments:
> 
> - The layout of the patch looks a bit odd.  E.g., you introduced the new "vcpu"
>    qmp parameter in patch 3, however it's not yet implemented, meanwhile I feel
>    like you squashed mostly all the rest into patch 6.  It's okay to use a
>    single big patch, but IMHO better to not declare that flag in QMP before it's
>    working, so ideally that should be the last patch to do that.
> 
>    From that POV: patch 1/2/4 look ok to be separated; perhaps squash patch
>    3/5/6 into one single patch to enable the new method as the last one?
> 
Yeah previously the concern is make the patch clear and small, however 
with the comment of each commit, it seems ok. As you said, it's okay to 
use a single big patch, i'll adjust the patch set style base on your advice.
> - You used "vcpu" across the patchset to show the per-vcpu new method.  Shall
>    we rename it globally to "per_vcpu" or "vcpu_based"?  A raw "vcpu" looks more
>    like a struct pointer not a boolean.
> 
Indeed, actually the initial name of the option is "per_vcpu". : ). i'll 
fix this.
> - Using memory_global_dirty_log_start|stop() may not be wise too IMHO, at least
>    we need to make sure it's not during migration, otherwise we could call the
>    stop() before migration ends then that'll be a problem..
Yeah, this may be a serious problem, thanks for your timely advice.
> 
>    Maybe we can start to make global_dirty_log a bitmask? Then we define:
> 
>      GLOBAL_DIRTY_MIGRATION
>      GLOBAL_DIRTY_DIRTY_RATE
> 
>    All references to global_dirty_log should mostly be untouched because any bit
>    set there should justify that global dirty logging is enabled (either for
>    migration or for dirty rate measurement).
> 
>    Migration starting half-way of dirty rate measurement seems okay too even
>    taking things like init-all-set into account, afaict.. as long as dirty rate
>    code never touches the qemu dirty bitmap, but only do the accounting when
>    collecting the pages...
> 
>    Feel free to think more about it on any other potential conflict with
>    migration, but in general seems working to me.
> 
I'll apply this on the next version.
> - Would you consider picking up my HMP patch and let HMP work from the 1st day?
> 
> - Please Cc the author of dirty rate too (Chuan Zheng <zhengchuan@huawei.com>),
>    while I already started to do so in this email.
> 
I'd be glad to do this above two.
> Thanks,
> 

Thanks Peter!

-- 
Best regard

Hyman Huang(黄勇)

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

* Re: [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu
@ 2021-06-02  0:51     ` Hyman Huang
  0 siblings, 0 replies; 24+ messages in thread
From: Hyman Huang @ 2021-06-02  0:51 UTC (permalink / raw)
  To: Peter Xu
  Cc: kvm, Juan Quintela, Dr. David Alan Gilbert, qemu-devel,
	Chuan Zheng, 张子健,
	Paolo Bonzini



在 2021/6/2 5:54, Peter Xu 写道:
> On Tue, Jun 01, 2021 at 01:02:45AM +0800, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> Since the Dirty Ring on QEMU part has been merged recently, how to use
>> this feature is under consideration.
>>
>> In the scene of migration, it is valuable to provide a more accurante
>> interface to track dirty memory than existing one, so that the upper
>> layer application can make a wise decision, or whatever. More importantly,
>> dirtyrate info at the granualrity of vcpu could provide a possibility to
>> make migration convergent by imposing restriction on vcpu. With Dirty
>> Ring, we can calculate dirtyrate efficiently and cheaply.
>>
>> The old interface implemented by sampling pages, it consumes cpu
>> resource, and the larger guest memory size become, the more cpu resource
>> it consumes, namely, hard to scale. New interface has no such drawback.
> 
> Yong,
> 
> Thanks for working on this!
> 
> Some high-level comments:
> 
> - The layout of the patch looks a bit odd.  E.g., you introduced the new "vcpu"
>    qmp parameter in patch 3, however it's not yet implemented, meanwhile I feel
>    like you squashed mostly all the rest into patch 6.  It's okay to use a
>    single big patch, but IMHO better to not declare that flag in QMP before it's
>    working, so ideally that should be the last patch to do that.
> 
>    From that POV: patch 1/2/4 look ok to be separated; perhaps squash patch
>    3/5/6 into one single patch to enable the new method as the last one?
> 
Yeah previously the concern is make the patch clear and small, however 
with the comment of each commit, it seems ok. As you said, it's okay to 
use a single big patch, i'll adjust the patch set style base on your advice.
> - You used "vcpu" across the patchset to show the per-vcpu new method.  Shall
>    we rename it globally to "per_vcpu" or "vcpu_based"?  A raw "vcpu" looks more
>    like a struct pointer not a boolean.
> 
Indeed, actually the initial name of the option is "per_vcpu". : ). i'll 
fix this.
> - Using memory_global_dirty_log_start|stop() may not be wise too IMHO, at least
>    we need to make sure it's not during migration, otherwise we could call the
>    stop() before migration ends then that'll be a problem..
Yeah, this may be a serious problem, thanks for your timely advice.
> 
>    Maybe we can start to make global_dirty_log a bitmask? Then we define:
> 
>      GLOBAL_DIRTY_MIGRATION
>      GLOBAL_DIRTY_DIRTY_RATE
> 
>    All references to global_dirty_log should mostly be untouched because any bit
>    set there should justify that global dirty logging is enabled (either for
>    migration or for dirty rate measurement).
> 
>    Migration starting half-way of dirty rate measurement seems okay too even
>    taking things like init-all-set into account, afaict.. as long as dirty rate
>    code never touches the qemu dirty bitmap, but only do the accounting when
>    collecting the pages...
> 
>    Feel free to think more about it on any other potential conflict with
>    migration, but in general seems working to me.
> 
I'll apply this on the next version.
> - Would you consider picking up my HMP patch and let HMP work from the 1st day?
> 
> - Please Cc the author of dirty rate too (Chuan Zheng <zhengchuan@huawei.com>),
>    while I already started to do so in this email.
> 
I'd be glad to do this above two.
> Thanks,
> 

Thanks Peter!

-- 
Best regard

Hyman Huang(黄勇)


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

* Re: [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
  2021-06-02  0:27       ` Hyman Huang
@ 2021-06-02  1:26         ` Peter Xu
  -1 siblings, 0 replies; 24+ messages in thread
From: Peter Xu @ 2021-06-02  1:26 UTC (permalink / raw)
  To: Hyman Huang
  Cc: qemu-devel, Paolo Bonzini, kvm, Juan Quintela, Dr. David Alan Gilbert

On Wed, Jun 02, 2021 at 08:27:19AM +0800, Hyman Huang wrote:
> 在 2021/6/2 7:20, Peter Xu 写道:
> > On Tue, Jun 01, 2021 at 01:04:06AM +0800, huangy81@chinatelecom.cn wrote:
> > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > > index 044f668a6e..973c193501 100644
> > > --- a/include/hw/core/cpu.h
> > > +++ b/include/hw/core/cpu.h
> > > @@ -375,6 +375,8 @@ struct CPUState {
> > >       struct kvm_run *kvm_run;
> > >       struct kvm_dirty_gfn *kvm_dirty_gfns;
> > >       uint32_t kvm_fetch_index;
> > > +    uint64_t dirty_pages;
> > > +    bool stat_dirty_pages;
> > 
> > Shall we make this bool a global one?  As I don't think we'll be able to only
> > enable it on a subset of cpus?
> Yes, it's a reasonable advice, i'll apply this on the next version

Or even drop the bool and do the accounting unconditionally?  No need to do +1
for each pfn, but do it once before returning from kvm_dirty_ring_reap_one().

-- 
Peter Xu


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

* Re: [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState
@ 2021-06-02  1:26         ` Peter Xu
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Xu @ 2021-06-02  1:26 UTC (permalink / raw)
  To: Hyman Huang
  Cc: Paolo Bonzini, Dr. David Alan Gilbert, qemu-devel, kvm, Juan Quintela

On Wed, Jun 02, 2021 at 08:27:19AM +0800, Hyman Huang wrote:
> 在 2021/6/2 7:20, Peter Xu 写道:
> > On Tue, Jun 01, 2021 at 01:04:06AM +0800, huangy81@chinatelecom.cn wrote:
> > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > > index 044f668a6e..973c193501 100644
> > > --- a/include/hw/core/cpu.h
> > > +++ b/include/hw/core/cpu.h
> > > @@ -375,6 +375,8 @@ struct CPUState {
> > >       struct kvm_run *kvm_run;
> > >       struct kvm_dirty_gfn *kvm_dirty_gfns;
> > >       uint32_t kvm_fetch_index;
> > > +    uint64_t dirty_pages;
> > > +    bool stat_dirty_pages;
> > 
> > Shall we make this bool a global one?  As I don't think we'll be able to only
> > enable it on a subset of cpus?
> Yes, it's a reasonable advice, i'll apply this on the next version

Or even drop the bool and do the accounting unconditionally?  No need to do +1
for each pfn, but do it once before returning from kvm_dirty_ring_reap_one().

-- 
Peter Xu



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

end of thread, other threads:[~2021-06-02  1:27 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31 17:02 [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu huangy81
2021-05-31 17:02 ` huangy81
2021-05-31 17:03 ` [PATCH v1 1/6] KVM: add kvm_dirty_ring_enabled function huangy81
2021-05-31 17:03   ` huangy81
2021-05-31 17:04 ` [PATCH v1 2/6] KVM: introduce dirty_pages into CPUState huangy81
2021-05-31 17:04   ` huangy81
2021-06-01 23:20   ` Peter Xu
2021-06-01 23:20     ` Peter Xu
2021-06-02  0:27     ` Hyman Huang
2021-06-02  0:27       ` Hyman Huang
2021-06-02  1:26       ` Peter Xu
2021-06-02  1:26         ` Peter Xu
2021-05-31 17:05 ` [PATCH v1 3/6] migration/dirtyrate: add vcpu option for qmp calc-dirty-rate huangy81
2021-05-31 17:05   ` huangy81
2021-05-31 17:05 ` [PATCH v1 4/6] migration/dirtyrate: adjust struct DirtyRateStat huangy81
2021-05-31 17:05   ` huangy81
2021-05-31 17:05 ` [PATCH v1 5/6] migration/dirtyrate: check support of calculation for vcpu huangy81
2021-05-31 17:05   ` huangy81
2021-05-31 17:06 ` [PATCH v1 6/6] migration/dirtyrate: implement dirty-ring dirtyrate calculation huangy81
2021-05-31 17:06   ` huangy81
2021-06-01 21:54 ` [PATCH v1 0/6] support dirtyrate at the granualrity of vcpu Peter Xu
2021-06-01 21:54   ` Peter Xu
2021-06-02  0:51   ` Hyman Huang
2021-06-02  0:51     ` Hyman Huang

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.