diff mbox series

[v1,1/3] migration/dirtyrate: implement vCPU dirtyrate calculation periodically

Message ID 499bdfeea4b19ef44a36e0fbb5be3e0d51765430.1637214721.git.huangy81@chinatelecom.cn
State New
Headers show
Series support dirty restraint on vCPU | expand

Commit Message

Hyman Huang Nov. 18, 2021, 6:07 a.m. UTC
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

introduce the third method GLOBAL_DIRTY_RESTRAINT of dirty
tracking for calculate dirtyrate periodly for dirty restraint.

implement thread for calculate dirtyrate periodly, which will
be used for dirty restraint.

add dirtyrestraint.h to introduce the util function for dirty
restrain.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 include/exec/memory.h           |   5 +-
 include/sysemu/dirtyrestraint.h |  20 +++++++
 migration/dirtyrate.c           | 118 ++++++++++++++++++++++++++++++++++++++++
 migration/dirtyrate.h           |   2 +
 4 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 include/sysemu/dirtyrestraint.h

Comments

Juan Quintela Nov. 18, 2021, 9:26 a.m. UTC | #1
huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>
> introduce the third method GLOBAL_DIRTY_RESTRAINT of dirty
> tracking for calculate dirtyrate periodly for dirty restraint.
>
> implement thread for calculate dirtyrate periodly, which will
> be used for dirty restraint.
>
> add dirtyrestraint.h to introduce the util function for dirty
> restrain.
>
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Some comentes:

> +void dirtyrestraint_calc_start(void);
> +
> +void dirtyrestraint_calc_state_init(int max_cpus);

dirtylimit_? instead of restraint.

We have a start function, but I can't see a finish/end/stop functions.

> +#define DIRTYRESTRAINT_CALC_TIME_MS         1000    /* 1000ms */
> +
> +struct {
> +    DirtyRatesData data;
> +    int64_t period;
> +    bool enable;

Related to previous comment.  I can't see where we set enable to 1, but
nowhere were we set it back to 0, so this never finish.

> +    QemuCond ready_cond;
> +    QemuMutex ready_mtx;

This is a question of style, but when you only have a mutex and a cond
in one struct, you can use the "cond" and "mutex" names.

But as said, it is a question of style, if you preffer do it this way.

> +static inline void record_dirtypages(DirtyPageRecord *dirty_pages,
> +                                     CPUState *cpu, bool start);

You have put the code at the beggining of the file, if you put it at the
end of it, I think you can avoid this forward declaration.

> +static void dirtyrestraint_calc_func(void)
> +{
> +    CPUState *cpu;
> +    DirtyPageRecord *dirty_pages;
> +    int64_t start_time, end_time, calc_time;
> +    DirtyRateVcpu rate;
> +    int i = 0;
> +
> +    dirty_pages = g_malloc0(sizeof(*dirty_pages) *
> +        dirtyrestraint_calc_state->data.nvcpu);
> +
> +    dirtyrestraint_global_dirty_log_start();
> +
> +    CPU_FOREACH(cpu) {
> +        record_dirtypages(dirty_pages, cpu, true);
> +    }
> +
> +    start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    g_usleep(DIRTYRESTRAINT_CALC_TIME_MS * 1000);
> +    end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +    calc_time = end_time - start_time;
> +
> +    dirtyrestraint_global_dirty_log_stop();
> +
> +    CPU_FOREACH(cpu) {
> +        record_dirtypages(dirty_pages, cpu, false);
> +    }
> +
> +    for (i = 0; i < dirtyrestraint_calc_state->data.nvcpu; i++) {
> +        uint64_t increased_dirty_pages =
> +            dirty_pages[i].end_pages - dirty_pages[i].start_pages;
> +        uint64_t memory_size_MB =
> +            (increased_dirty_pages * TARGET_PAGE_SIZE) >> 20;
> +        int64_t dirtyrate = (memory_size_MB * 1000) / calc_time;
> +
> +        rate.id = i;
> +        rate.dirty_rate  = dirtyrate;
> +        dirtyrestraint_calc_state->data.rates[i] = rate;
> +
> +        trace_dirtyrate_do_calculate_vcpu(i,
> +            dirtyrestraint_calc_state->data.rates[i].dirty_rate);
> +    }
> +
> +    return;

unnecesary return;

> +}
> +
> +static void *dirtyrestraint_calc_thread(void *opaque)
> +{
> +    rcu_register_thread();
> +
> +    while (qatomic_read(&dirtyrestraint_calc_state->enable)) {
> +        dirtyrestraint_calc_func();
> +        dirtyrestraint_calc_state->ready = true;

           You really need this to be a global variable?  You can pass
           it on the opaque, no?

Later, Juan.
diff mbox series

Patch

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 20f1b27..565d06b 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -69,7 +69,10 @@  static inline void fuzz_dma_read_cb(size_t addr,
 /* Dirty tracking enabled because measuring dirty rate */
 #define GLOBAL_DIRTY_DIRTY_RATE (1U << 1)
 
-#define GLOBAL_DIRTY_MASK  (0x3)
+/* Dirty tracking enabled because dirty restraint */
+#define GLOBAL_DIRTY_RESTRAINT  (1U << 2)
+
+#define GLOBAL_DIRTY_MASK  (0x7)
 
 extern unsigned int global_dirty_tracking;
 
diff --git a/include/sysemu/dirtyrestraint.h b/include/sysemu/dirtyrestraint.h
new file mode 100644
index 0000000..ca744af
--- /dev/null
+++ b/include/sysemu/dirtyrestraint.h
@@ -0,0 +1,20 @@ 
+/*
+ * dirty restraint helper functions
+ *
+ * Copyright (c) 2021 CHINA TELECOM CO.,LTD.
+ *
+ * Authors:
+ *  Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
+ *
+ * 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_DIRTYRESTRAINT_H
+#define QEMU_DIRTYRESTRAINT_H
+
+#define DIRTYRESTRAINT_CALC_PERIOD_TIME_S   15      /* 15s */
+
+void dirtyrestraint_calc_start(void);
+
+void dirtyrestraint_calc_state_init(int max_cpus);
+#endif
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index d65e744..b453b3a 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -27,6 +27,7 @@ 
 #include "qapi/qmp/qdict.h"
 #include "sysemu/kvm.h"
 #include "sysemu/runstate.h"
+#include "sysemu/dirtyrestraint.h"
 #include "exec/memory.h"
 
 /*
@@ -46,6 +47,123 @@  static struct DirtyRateStat DirtyStat;
 static DirtyRateMeasureMode dirtyrate_mode =
                 DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING;
 
+#define DIRTYRESTRAINT_CALC_TIME_MS         1000    /* 1000ms */
+
+struct {
+    DirtyRatesData data;
+    int64_t period;
+    bool enable;
+    QemuCond ready_cond;
+    QemuMutex ready_mtx;
+    bool ready;
+} *dirtyrestraint_calc_state;
+
+static inline void record_dirtypages(DirtyPageRecord *dirty_pages,
+                                     CPUState *cpu, bool start);
+
+static void dirtyrestraint_global_dirty_log_start(void)
+{
+    qemu_mutex_lock_iothread();
+    memory_global_dirty_log_start(GLOBAL_DIRTY_RESTRAINT);
+    qemu_mutex_unlock_iothread();
+}
+
+static void dirtyrestraint_global_dirty_log_stop(void)
+{
+    qemu_mutex_lock_iothread();
+    memory_global_dirty_log_sync();
+    memory_global_dirty_log_stop(GLOBAL_DIRTY_RESTRAINT);
+    qemu_mutex_unlock_iothread();
+}
+
+static void dirtyrestraint_calc_func(void)
+{
+    CPUState *cpu;
+    DirtyPageRecord *dirty_pages;
+    int64_t start_time, end_time, calc_time;
+    DirtyRateVcpu rate;
+    int i = 0;
+
+    dirty_pages = g_malloc0(sizeof(*dirty_pages) *
+        dirtyrestraint_calc_state->data.nvcpu);
+
+    dirtyrestraint_global_dirty_log_start();
+
+    CPU_FOREACH(cpu) {
+        record_dirtypages(dirty_pages, cpu, true);
+    }
+
+    start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    g_usleep(DIRTYRESTRAINT_CALC_TIME_MS * 1000);
+    end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+    calc_time = end_time - start_time;
+
+    dirtyrestraint_global_dirty_log_stop();
+
+    CPU_FOREACH(cpu) {
+        record_dirtypages(dirty_pages, cpu, false);
+    }
+
+    for (i = 0; i < dirtyrestraint_calc_state->data.nvcpu; i++) {
+        uint64_t increased_dirty_pages =
+            dirty_pages[i].end_pages - dirty_pages[i].start_pages;
+        uint64_t memory_size_MB =
+            (increased_dirty_pages * TARGET_PAGE_SIZE) >> 20;
+        int64_t dirtyrate = (memory_size_MB * 1000) / calc_time;
+
+        rate.id = i;
+        rate.dirty_rate  = dirtyrate;
+        dirtyrestraint_calc_state->data.rates[i] = rate;
+
+        trace_dirtyrate_do_calculate_vcpu(i,
+            dirtyrestraint_calc_state->data.rates[i].dirty_rate);
+    }
+
+    return;
+}
+
+static void *dirtyrestraint_calc_thread(void *opaque)
+{
+    rcu_register_thread();
+
+    while (qatomic_read(&dirtyrestraint_calc_state->enable)) {
+        dirtyrestraint_calc_func();
+        dirtyrestraint_calc_state->ready = true;
+        qemu_cond_signal(&dirtyrestraint_calc_state->ready_cond);
+        sleep(dirtyrestraint_calc_state->period);
+    }
+
+    rcu_unregister_thread();
+    return NULL;
+}
+
+void dirtyrestraint_calc_start(void)
+{
+    if (likely(!qatomic_read(&dirtyrestraint_calc_state->enable))) {
+        qatomic_set(&dirtyrestraint_calc_state->enable, 1);
+        QemuThread thread;
+        qemu_thread_create(&thread, "dirtyrestraint-calc",
+            dirtyrestraint_calc_thread,
+            NULL, QEMU_THREAD_DETACHED);
+    }
+}
+
+void dirtyrestraint_calc_state_init(int max_cpus)
+{
+    dirtyrestraint_calc_state =
+        g_malloc0(sizeof(*dirtyrestraint_calc_state));
+
+    dirtyrestraint_calc_state->data.nvcpu = max_cpus;
+    dirtyrestraint_calc_state->data.rates =
+        g_malloc0(sizeof(DirtyRateVcpu) * max_cpus);
+
+    dirtyrestraint_calc_state->period =
+        DIRTYRESTRAINT_CALC_PERIOD_TIME_S;
+
+    qemu_cond_init(&dirtyrestraint_calc_state->ready_cond);
+    qemu_mutex_init(&dirtyrestraint_calc_state->ready_mtx);
+}
+
 static int64_t set_sample_page_period(int64_t msec, int64_t initial_time)
 {
     int64_t current_time;
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 69d4c5b..e96acdc 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -70,6 +70,8 @@  typedef struct VcpuStat {
     DirtyRateVcpu *rates; /* array of dirty rate for each vcpu */
 } VcpuStat;
 
+typedef struct VcpuStat DirtyRatesData;
+
 /*
  * Store calculation statistics for each measure.
  */