[v7] support dirtyrate at the granualrity of vcpu

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

Posted by huangy81@chinatelecom.cn 4 years, 7 months ago

From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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);
+
+    /*
+     * 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;
+
+    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

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

Posted by Peter Xu 4 years, 7 months ago

On Thu, Jun 17, 2021 at 10:12:07PM +0800, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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..

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
> 

-- 
Peter Xu

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

Posted by Hyman Huang 4 years, 7 months ago


在 2021/6/17 23:34, Peter Xu 写道:
> On Thu, Jun 17, 2021 at 10:12:07PM +0800, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@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(黄勇) <huangy81@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(黄勇)

[PATCH v7 1/7] KVM: introduce dirty_pages and kvm_dirty_ring_enabled
[PATCH v7 2/7] memory: rename global_dirty_log to global_dirty_tracking
[PATCH v7 3/7] memory: make global_dirty_tracking a bitmask
[PATCH v7 4/7] migration/dirtyrate: introduce struct and adjust DirtyRateStat
[PATCH v7 5/7] migration/dirtyrate: adjust order of registering thread
[PATCH v7 6/7] migration/dirtyrate: move init step of calculation to main thread
[PATCH v7 7/7] migration/dirtyrate: implement dirty-ring dirtyrate calculation