Re: [PATCH v7 6/7] migration/dirtyrate: move init step of calculation to main thread

[Hyman Huang]

在 2021/6/17 23:34, Peter Xu 写道:

On Thu, Jun 17, 2021 at 10:12:07PM +0800, huang...@chinatelecom.cn wrote:

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

since main thread may "query dirty rate" at any time, it's better
to move init step into main thead so that synchronization overhead
between "main" and "get_dirtyrate" can be reduced.

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

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index a9bdd60..8a9dcf7 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -26,6 +26,7 @@

static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;

  static struct DirtyRateStat DirtyStat;
+static DirtyRateMeasureMode dirtyrate_mode = DIRTY_RATE_MEASURE_MODE_NONE;

static int64_t set_sample_page_period(int64_t msec, int64_t initial_time)

  {
@@ -111,6 +112,11 @@ static void init_dirtyrate_stat(int64_t start_time,
      }
  }

+static void cleanup_dirtyrate_stat(struct DirtyRateConfig config)

+{
+    /* TODO */
+}
+
  static void update_dirtyrate_stat(struct RamblockDirtyInfo *info)
  {
      DirtyStat.page_sampling.total_dirty_samples += info->sample_dirty_count;
@@ -380,7 +386,6 @@ void *get_dirtyrate_thread(void *arg)
  {
      struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
      int ret;
-    int64_t start_time;
      rcu_register_thread();

ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_UNSTARTED,

@@ -390,9 +395,6 @@ void *get_dirtyrate_thread(void *arg)
          return NULL;
      }

- start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;

-    init_dirtyrate_stat(start_time, config);
-
      calculate_dirtyrate(config);

ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_MEASURING,

@@ -411,6 +413,7 @@ void qmp_calc_dirty_rate(int64_t calc_time, bool 
has_sample_pages,
      static struct DirtyRateConfig config;
      QemuThread thread;
      int ret;
+    int64_t start_time;

/*

       * If the dirty rate is already being measured, don't attempt to start.
@@ -451,6 +454,18 @@ void qmp_calc_dirty_rate(int64_t calc_time, bool 
has_sample_pages,
      config.sample_period_seconds = calc_time;
      config.sample_pages_per_gigabytes = sample_pages;
      config.mode = DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING;
+
+    cleanup_dirtyrate_stat(config);

This line should ideally be moved into the next patch, as sampling itself
doesn't need it. >

+
+    /*
+     * update dirty rate mode so that we can figure out what mode has
+     * been used in last calculation
+     **/
+    dirtyrate_mode = DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING;

This line is odd. Would page sampling broken if without this line?  We need to
make sure each commit keeps the old way working..

yes, i'll drop this to make sure each commit keeps the old way working

Thanks,

+
+    start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
+    init_dirtyrate_stat(start_time, config);
+
      qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
                         (void *)&config, QEMU_THREAD_DETACHED);
  }
--
1.8.3.1

--
Best regard

Hyman Huang(黄勇)