diff mbox series

migration/throttle: Add throttle-trig-thres migration parameter

Message ID 20200221025727.63808-1-zhukeqian1@huawei.com (mailing list archive)
State New, archived
Headers show
Series migration/throttle: Add throttle-trig-thres migration parameter | expand

Commit Message

zhukeqian Feb. 21, 2020, 2:57 a.m. UTC
Currently, if the bytes_dirty_period is more than the 50% of
bytes_xfer_period, we start or increase throttling.

If we make this percentage higher, then we can tolerate higher
dirty rate during migration, which means less impact on guest.
The side effect of higher percentage is longer migration time.

We can configure this parameter to switch between migration time
firt or guest performance first. The default value is 50.

Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
---
Cc: Juan Quintela <quintela@redhat.com>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
---
 migration/migration.c | 24 ++++++++++++++++++++++++
 migration/ram.c       | 21 +++++++++++++++------
 monitor/hmp-cmds.c    |  7 +++++++
 qapi/migration.json   | 16 +++++++++++++++-
 4 files changed, 61 insertions(+), 7 deletions(-)

Comments

Eric Blake Feb. 21, 2020, 2:14 p.m. UTC | #1
On 2/20/20 8:57 PM, Keqian Zhu wrote:
> Currently, if the bytes_dirty_period is more than the 50% of
> bytes_xfer_period, we start or increase throttling.
> 
> If we make this percentage higher, then we can tolerate higher
> dirty rate during migration, which means less impact on guest.
> The side effect of higher percentage is longer migration time.
> 
> We can configure this parameter to switch between migration time
> firt or guest performance first. The default value is 50.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/qapi/migration.json
> @@ -524,6 +524,10 @@
>   #                      compression, so set the decompress-threads to the number about 1/4
>   #                      of compress-threads is adequate.
>   #
> +# @throttle-trig-thres: The ratio of bytes_dirty_period and bytes_xfer_period to
> +#                       trigger throttling. It is expressed as percentage. The
> +#                       default value is 50. (Since 5.0)
> +#

Abbreviating feels odd; can you please spell this out as 
throttle-trigger-threshold?

Can the threshold exceed 100%?
zhukeqian Feb. 24, 2020, 1:11 a.m. UTC | #2
Hi, Eric

On 2020/2/21 22:14, Eric Blake wrote:
> On 2/20/20 8:57 PM, Keqian Zhu wrote:
>> Currently, if the bytes_dirty_period is more than the 50% of
>> bytes_xfer_period, we start or increase throttling.
>>
>> If we make this percentage higher, then we can tolerate higher
>> dirty rate during migration, which means less impact on guest.
>> The side effect of higher percentage is longer migration time.
>>
>> We can configure this parameter to switch between migration time
>> firt or guest performance first. The default value is 50.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>> ---
>> Cc: Juan Quintela <quintela@redhat.com>
>> Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Markus Armbruster <armbru@redhat.com>
>> ---
> 
>> +++ b/qapi/migration.json
>> @@ -524,6 +524,10 @@
>>   #                      compression, so set the decompress-threads to the number about 1/4
>>   #                      of compress-threads is adequate.
>>   #
>> +# @throttle-trig-thres: The ratio of bytes_dirty_period and bytes_xfer_period to
>> +#                       trigger throttling. It is expressed as percentage. The
>> +#                       default value is 50. (Since 5.0)
>> +#
> 
> Abbreviating feels odd; can you please spell this out as throttle-trigger-threshold?
OK, I will use full name in v2.
> 
> Can the threshold exceed 100%?
If the threshold exceed 100% and the dirty rate is between 100% and threshold, then throttling
will not be started, so the migration will not converge and last an uncertain time until the workload
in guest is down by itself. So I think that the threshold exceed 100% maybe not suitable :).
> 

Thanks.
Keqian
Dr. David Alan Gilbert March 12, 2020, 5:02 p.m. UTC | #3
* Eric Blake (eblake@redhat.com) wrote:
> On 2/20/20 8:57 PM, Keqian Zhu wrote:
> > Currently, if the bytes_dirty_period is more than the 50% of
> > bytes_xfer_period, we start or increase throttling.
> > 
> > If we make this percentage higher, then we can tolerate higher
> > dirty rate during migration, which means less impact on guest.
> > The side effect of higher percentage is longer migration time.
> > 
> > We can configure this parameter to switch between migration time
> > firt or guest performance first. The default value is 50.
> > 
> > Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> > ---
> > Cc: Juan Quintela <quintela@redhat.com>
> > Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > Cc: Eric Blake <eblake@redhat.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > ---
> 
> > +++ b/qapi/migration.json
> > @@ -524,6 +524,10 @@
> >   #                      compression, so set the decompress-threads to the number about 1/4
> >   #                      of compress-threads is adequate.
> >   #
> > +# @throttle-trig-thres: The ratio of bytes_dirty_period and bytes_xfer_period to
> > +#                       trigger throttling. It is expressed as percentage. The
> > +#                       default value is 50. (Since 5.0)
> > +#
> 
> Abbreviating feels odd; can you please spell this out as
> throttle-trigger-threshold?
> 
> Can the threshold exceed 100%?

Note the code checks for that and disallows it, only allowing 1..99

Dave

> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/migration/migration.c b/migration/migration.c
index 8fb68795dc..e6c2451734 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -78,6 +78,7 @@ 
 /*0: means nocompress, 1: best speed, ... 9: best compress ratio */
 #define DEFAULT_MIGRATE_COMPRESS_LEVEL 1
 /* Define default autoconverge cpu throttle migration parameters */
+#define DEFAULT_MIGRATE_THROTTLE_TRIG_THRES 50
 #define DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL 20
 #define DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT 10
 #define DEFAULT_MIGRATE_MAX_CPU_THROTTLE 99
@@ -778,6 +779,8 @@  MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->compress_wait_thread = s->parameters.compress_wait_thread;
     params->has_decompress_threads = true;
     params->decompress_threads = s->parameters.decompress_threads;
+    params->has_throttle_trig_thres = true;
+    params->throttle_trig_thres = s->parameters.throttle_trig_thres;
     params->has_cpu_throttle_initial = true;
     params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
     params->has_cpu_throttle_increment = true;
@@ -1164,6 +1167,15 @@  static bool migrate_params_check(MigrationParameters *params, Error **errp)
         return false;
     }
 
+    if (params->has_throttle_trig_thres &&
+        (params->throttle_trig_thres < 1 ||
+         params->throttle_trig_thres > 99)) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "throttle_trig_thres",
+                   "an integer in the range of 1 to 99");
+        return false;
+    }
+
     if (params->has_cpu_throttle_initial &&
         (params->cpu_throttle_initial < 1 ||
          params->cpu_throttle_initial > 99)) {
@@ -1279,6 +1291,10 @@  static void migrate_params_test_apply(MigrateSetParameters *params,
         dest->decompress_threads = params->decompress_threads;
     }
 
+    if (params->has_throttle_trig_thres) {
+        dest->throttle_trig_thres = params->throttle_trig_thres;
+    }
+
     if (params->has_cpu_throttle_initial) {
         dest->cpu_throttle_initial = params->cpu_throttle_initial;
     }
@@ -1360,6 +1376,10 @@  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
         s->parameters.decompress_threads = params->decompress_threads;
     }
 
+    if (params->has_throttle_trig_thres) {
+        s->parameters.throttle_trig_thres = params->throttle_trig_thres;
+    }
+
     if (params->has_cpu_throttle_initial) {
         s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
     }
@@ -3506,6 +3526,9 @@  static Property migration_properties[] = {
     DEFINE_PROP_UINT8("x-decompress-threads", MigrationState,
                       parameters.decompress_threads,
                       DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT),
+    DEFINE_PROP_UINT8("x-throttle-trig-thres", MigrationState,
+                      parameters.throttle_trig_thres,
+                      DEFAULT_MIGRATE_THROTTLE_TRIG_THRES),
     DEFINE_PROP_UINT8("x-cpu-throttle-initial", MigrationState,
                       parameters.cpu_throttle_initial,
                       DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL),
@@ -3606,6 +3629,7 @@  static void migration_instance_init(Object *obj)
     params->has_compress_level = true;
     params->has_compress_threads = true;
     params->has_decompress_threads = true;
+    params->has_throttle_trig_thres = true;
     params->has_cpu_throttle_initial = true;
     params->has_cpu_throttle_increment = true;
     params->has_max_bandwidth = true;
diff --git a/migration/ram.c b/migration/ram.c
index ed23ed1c7c..28081cb1e1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -901,6 +901,11 @@  static void migration_bitmap_sync(RAMState *rs)
     RAMBlock *block;
     int64_t end_time;
     uint64_t bytes_xfer_now;
+    uint64_t bytes_dirty_period;
+    uint64_t bytes_xfer_period;
+    uint64_t bytes_dirty_thres;
+    uint64_t throttle_trig_thres;
+    MigrationState *s = migrate_get_current();
 
     ram_counters.dirty_sync_count++;
 
@@ -934,13 +939,17 @@  static void migration_bitmap_sync(RAMState *rs)
          * throttling logic during the bulk phase of block migration. */
         if (migrate_auto_converge() && !blk_mig_bulk_active()) {
             /* The following detection logic can be refined later. For now:
-               Check to see if the dirtied bytes is 50% more than the approx.
-               amount of bytes that just got transferred since the last time we
-               were in this routine. If that happens twice, start or increase
-               throttling */
+               Check to see if the ratio between dirtied bytes and the approx.
+               amount of bytes that just got transferred since the last time
+               we were in this routine reaches the threshold. If that happens
+               twice, start or increase throttling. */
+
+            bytes_dirty_period = rs->num_dirty_pages_period * TARGET_PAGE_SIZE;
+            bytes_xfer_period = bytes_xfer_now - rs->bytes_xfer_prev;
+            throttle_trig_thres = s->parameters.throttle_trig_thres;
+            bytes_dirty_thres = bytes_xfer_period * throttle_trig_thres / 100;
 
-            if ((rs->num_dirty_pages_period * TARGET_PAGE_SIZE >
-                   (bytes_xfer_now - rs->bytes_xfer_prev) / 2) &&
+            if ((bytes_dirty_period > bytes_dirty_thres) &&
                 (++rs->dirty_rate_high_cnt >= 2)) {
                     trace_migration_throttle();
                     rs->dirty_rate_high_cnt = 0;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index b237613e0d..8673f5eeb2 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -409,6 +409,10 @@  void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_DECOMPRESS_THREADS),
             params->decompress_threads);
+        assert(params->has_throttle_trig_thres);
+        monitor_printf(mon, "%s: %u\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_THROTTLE_TRIG_THRES),
+            params->throttle_trig_thres);
         assert(params->has_cpu_throttle_initial);
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL),
@@ -1764,6 +1768,9 @@  void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_decompress_threads = true;
         visit_type_int(v, param, &p->decompress_threads, &err);
         break;
+    case MIGRATION_PARAMETER_THROTTLE_TRIG_THRES:
+        p->has_throttle_trig_thres = true;
+        visit_type_int(v, param, &p->throttle_trig_thres, &err);
     case MIGRATION_PARAMETER_CPU_THROTTLE_INITIAL:
         p->has_cpu_throttle_initial = true;
         visit_type_int(v, param, &p->cpu_throttle_initial, &err);
diff --git a/qapi/migration.json b/qapi/migration.json
index 52f3429969..4da5385832 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -524,6 +524,10 @@ 
 #                      compression, so set the decompress-threads to the number about 1/4
 #                      of compress-threads is adequate.
 #
+# @throttle-trig-thres: The ratio of bytes_dirty_period and bytes_xfer_period to
+#                       trigger throttling. It is expressed as percentage. The
+#                       default value is 50. (Since 5.0)
+#
 # @cpu-throttle-initial: Initial percentage of time guest cpus are throttled
 #                        when migration auto-converge is activated. The
 #                        default value is 20. (Since 2.7)
@@ -592,7 +596,7 @@ 
   'data': ['announce-initial', 'announce-max',
            'announce-rounds', 'announce-step',
            'compress-level', 'compress-threads', 'decompress-threads',
-           'compress-wait-thread',
+           'compress-wait-thread', 'throttle-trig-thres',
            'cpu-throttle-initial', 'cpu-throttle-increment',
            'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
            'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
@@ -626,6 +630,10 @@ 
 #
 # @decompress-threads: decompression thread count
 #
+# @throttle-trig-thres: The ratio of bytes_dirty_period and bytes_xfer_period to
+#                       trigger throttling. It is expressed as percentage. The
+#                       default value is 50. (Since 5.0)
+#
 # @cpu-throttle-initial: Initial percentage of time guest cpus are
 #                        throttled when migration auto-converge is activated.
 #                        The default value is 20. (Since 2.7)
@@ -701,6 +709,7 @@ 
             '*compress-threads': 'int',
             '*compress-wait-thread': 'bool',
             '*decompress-threads': 'int',
+            '*throttle-trig-thres': 'int',
             '*cpu-throttle-initial': 'int',
             '*cpu-throttle-increment': 'int',
             '*tls-creds': 'StrOrNull',
@@ -759,6 +768,10 @@ 
 #
 # @decompress-threads: decompression thread count
 #
+# @throttle-trig-thres: The ratio of bytes_dirty_period and bytes_xfer_period to
+#                       trigger throttling. It is expressed as percentage. The
+#                       default value is 50. (Since 5.0)
+#
 # @cpu-throttle-initial: Initial percentage of time guest cpus are
 #                        throttled when migration auto-converge is activated.
 #                        (Since 2.7)
@@ -834,6 +847,7 @@ 
             '*compress-threads': 'uint8',
             '*compress-wait-thread': 'bool',
             '*decompress-threads': 'uint8',
+            '*throttle-trig-thres': 'uint8',
             '*cpu-throttle-initial': 'uint8',
             '*cpu-throttle-increment': 'uint8',
             '*tls-creds': 'str',