[PATCH] migration/throttle: Make throttle slower at tail stage

[PATCH] migration/throttle: Make throttle slower at tail stage

[Keqian Zhu]

At the tail stage of throttle, VM is very sensitive to
CPU percentage. We just throttle 30% of remaining CPU
when throttle is more than 80 percentage.

This doesn't conflict with cpu_throttle_increment.

This may make migration time longer, and is disabled
by default.

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 | 13 +++++++++++++
 migration/ram.c       | 18 ++++++++++++++++--
 monitor/hmp-cmds.c    |  4 ++++
 qapi/migration.json   | 21 +++++++++++++++++++++
 4 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 3a21a4686c..37b569cee9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -782,6 +782,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
     params->has_cpu_throttle_increment = true;
     params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
+    params->has_cpu_throttle_tailslow = true;
+    params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
     params->has_tls_creds = true;
     params->tls_creds = g_strdup(s->parameters.tls_creds);
     params->has_tls_hostname = true;
@@ -1287,6 +1289,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
         dest->cpu_throttle_increment = params->cpu_throttle_increment;
     }
 
+    if (params->has_cpu_throttle_tailslow) {
+        dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+    }
+
     if (params->has_tls_creds) {
         assert(params->tls_creds->type == QTYPE_QSTRING);
         dest->tls_creds = g_strdup(params->tls_creds->u.s);
@@ -1368,6 +1374,10 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
         s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
     }
 
+    if (params->has_cpu_throttle_tailslow) {
+        s->parameters.cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+    }
+
     if (params->has_tls_creds) {
         g_free(s->parameters.tls_creds);
         assert(params->tls_creds->type == QTYPE_QSTRING);
@@ -3504,6 +3514,8 @@ static Property migration_properties[] = {
     DEFINE_PROP_UINT8("x-cpu-throttle-increment", MigrationState,
                       parameters.cpu_throttle_increment,
                       DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT),
+    DEFINE_PROP_BOOL("x-cpu-throttle-tailslow", MigrationState,
+                      parameters.cpu_throttle_tailslow, false),
     DEFINE_PROP_SIZE("x-max-bandwidth", MigrationState,
                       parameters.max_bandwidth, MAX_THROTTLE),
     DEFINE_PROP_UINT64("x-downtime-limit", MigrationState,
@@ -3600,6 +3612,7 @@ static void migration_instance_init(Object *obj)
     params->has_decompress_threads = true;
     params->has_cpu_throttle_initial = true;
     params->has_cpu_throttle_increment = true;
+    params->has_cpu_throttle_tailslow = true;
     params->has_max_bandwidth = true;
     params->has_downtime_limit = true;
     params->has_x_checkpoint_delay = true;
diff --git a/migration/ram.c b/migration/ram.c
index ed23ed1c7c..d86413a5d1 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -29,6 +29,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include <zlib.h>
+#include <math.h>
 #include "qemu/cutils.h"
 #include "qemu/bitops.h"
 #include "qemu/bitmap.h"
@@ -620,15 +621,28 @@ static void mig_throttle_guest_down(void)
 {
     MigrationState *s = migrate_get_current();
     uint64_t pct_initial = s->parameters.cpu_throttle_initial;
-    uint64_t pct_icrement = s->parameters.cpu_throttle_increment;
+    uint64_t pct_increment = s->parameters.cpu_throttle_increment;
+    bool pct_tailslow = s->parameters.cpu_throttle_tailslow;
     int pct_max = s->parameters.max_cpu_throttle;
 
+    const uint64_t cpu_throttle_now = cpu_throttle_get_percentage();
+    uint64_t cpu_throttle_inc;
+
     /* We have not started throttling yet. Let's start it. */
     if (!cpu_throttle_active()) {
         cpu_throttle_set(pct_initial);
     } else {
         /* Throttling already on, just increase the rate */
-        cpu_throttle_set(MIN(cpu_throttle_get_percentage() + pct_icrement,
+        if (cpu_throttle_now >= 80 && pct_tailslow) {
+            /* Eat some scale of CPU from remaining */
+            cpu_throttle_inc = ceil((100 - cpu_throttle_now) * 0.3);
+            if (cpu_throttle_inc > pct_increment) {
+                cpu_throttle_inc = pct_increment;
+            }
+        } else {
+            cpu_throttle_inc = pct_increment;
+        }
+        cpu_throttle_set(MIN(cpu_throttle_now + cpu_throttle_inc,
                          pct_max));
     }
 }
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 558fe06b8f..b5f5c0b382 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -416,6 +416,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT),
             params->cpu_throttle_increment);
+        assert(params->has_cpu_throttle_tailslow);
+        monitor_printf(mon, "%s: %s\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW),
+            params->cpu_throttle_tailslow ? "on" : "off");
         assert(params->has_max_cpu_throttle);
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_THROTTLE),
diff --git a/qapi/migration.json b/qapi/migration.json
index b7348d0c8b..0ac94e00f2 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -532,6 +532,12 @@
 #                          auto-converge detects that migration is not making
 #                          progress. The default value is 10. (Since 2.7)
 #
+# @cpu-throttle-tailslow: Make throttle slower at tail stage
+#                         At the tail stage of throttle, VM is very sensitive to
+#                         CPU percentage. We just throttle 30% of remaining CPU
+#                         when throttle is more than 80 percentage. The default
+#                         value is false. (Since 4.1)
+#
 # @tls-creds: ID of the 'tls-creds' object that provides credentials for
 #             establishing a TLS connection over the migration data channel.
 #             On the outgoing side of the migration, the credentials must
@@ -594,6 +600,7 @@
            'compress-level', 'compress-threads', 'decompress-threads',
            'compress-wait-thread',
            'cpu-throttle-initial', 'cpu-throttle-increment',
+           'cpu-throttle-tailslow',
            'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
            'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
            'multifd-channels',
@@ -634,6 +641,12 @@
 #                          auto-converge detects that migration is not making
 #                          progress. The default value is 10. (Since 2.7)
 #
+# @cpu-throttle-tailslow: Make throttle slower at tail stage
+#                         At the tail stage of throttle, VM is very sensitive to
+#                         CPU percentage. We just throttle 30% of remaining CPU
+#                         when throttle is more than 80 percentage. The default
+#                         value is false. (Since 4.1)
+#
 # @tls-creds: ID of the 'tls-creds' object that provides credentials
 #             for establishing a TLS connection over the migration data
 #             channel. On the outgoing side of the migration, the credentials
@@ -703,6 +716,7 @@
             '*decompress-threads': 'int',
             '*cpu-throttle-initial': 'int',
             '*cpu-throttle-increment': 'int',
+            '*cpu-throttle-tailslow': 'bool',
             '*tls-creds': 'StrOrNull',
             '*tls-hostname': 'StrOrNull',
             '*tls-authz': 'StrOrNull',
@@ -767,6 +781,12 @@
 #                          auto-converge detects that migration is not making
 #                          progress. (Since 2.7)
 #
+# @cpu-throttle-tailslow: Make throttle slower at tail stage
+#                         At the tail stage of throttle, VM is very sensitive to
+#                         CPU percentage. We just throttle 30% of remaining CPU
+#                         when throttle is more than 80 percentage. The default
+#                         value is false. (Since 4.1)
+#
 # @tls-creds: ID of the 'tls-creds' object that provides credentials
 #             for establishing a TLS connection over the migration data
 #             channel. On the outgoing side of the migration, the credentials
@@ -836,6 +856,7 @@
             '*decompress-threads': 'uint8',
             '*cpu-throttle-initial': 'uint8',
             '*cpu-throttle-increment': 'uint8',
+            '*cpu-throttle-tailslow': 'bool',
             '*tls-creds': 'str',
             '*tls-hostname': 'str',
             '*tls-authz': 'str',
-- 
2.19.1

Re: [PATCH] migration/throttle: Make throttle slower at tail stage

[Eric Blake]

On 2/13/20 9:27 PM, Keqian Zhu wrote:
> At the tail stage of throttle, VM is very sensitive to
> CPU percentage. We just throttle 30% of remaining CPU
> when throttle is more than 80 percentage.
> 
> This doesn't conflict with cpu_throttle_increment.
> 
> This may make migration time longer, and is disabled
> by default.
> 
> 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
> @@ -532,6 +532,12 @@
>   #                          auto-converge detects that migration is not making
>   #                          progress. The default value is 10. (Since 2.7)
>   #
> +# @cpu-throttle-tailslow: Make throttle slower at tail stage
> +#                         At the tail stage of throttle, VM is very sensitive to
> +#                         CPU percentage. We just throttle 30% of remaining CPU
> +#                         when throttle is more than 80 percentage. The default
> +#                         value is false. (Since 4.1)

The next release is 5.0, not 4.1.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] migration/throttle: Make throttle slower at tail stage

[Dr. David Alan Gilbert]

* Keqian Zhu (zhukeqian1@huawei.com) wrote:
> At the tail stage of throttle, VM is very sensitive to
> CPU percentage. We just throttle 30% of remaining CPU
> when throttle is more than 80 percentage.

This is a bit unusual;  all of the rest of the throttling has no
fixed constants; all values are set by parameters.

You've got the two, the '80' and the '0.3'

I thinkt he easy solution is to replace your parameter 'tailslow' by two
new parameters; 'tailstart' and 'tailrate';  both defaulting to 100%.

Then you make it:

        if (cpu_throttle_now >= pct_tailstart) {
            /* Eat some scale of CPU from remaining */
            cpu_throttle_inc = ceil((100 - cpu_throttle_now) * pct_tailrate);

(with percentage scaling added).

Then setting 'tailstart' to 80 and 'tailrate' to 30 is equivalent to
what you have, but means we have no magical constants in the code.

Dave


> This doesn't conflict with cpu_throttle_increment.
> 
> This may make migration time longer, and is disabled
> by default.
> 
> 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 | 13 +++++++++++++
>  migration/ram.c       | 18 ++++++++++++++++--
>  monitor/hmp-cmds.c    |  4 ++++
>  qapi/migration.json   | 21 +++++++++++++++++++++
>  4 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 3a21a4686c..37b569cee9 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -782,6 +782,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
>      params->has_cpu_throttle_increment = true;
>      params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
> +    params->has_cpu_throttle_tailslow = true;
> +    params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
>      params->has_tls_creds = true;
>      params->tls_creds = g_strdup(s->parameters.tls_creds);
>      params->has_tls_hostname = true;
> @@ -1287,6 +1289,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>          dest->cpu_throttle_increment = params->cpu_throttle_increment;
>      }
>  
> +    if (params->has_cpu_throttle_tailslow) {
> +        dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
> +    }
> +
>      if (params->has_tls_creds) {
>          assert(params->tls_creds->type == QTYPE_QSTRING);
>          dest->tls_creds = g_strdup(params->tls_creds->u.s);
> @@ -1368,6 +1374,10 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>          s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
>      }
>  
> +    if (params->has_cpu_throttle_tailslow) {
> +        s->parameters.cpu_throttle_tailslow = params->cpu_throttle_tailslow;
> +    }
> +
>      if (params->has_tls_creds) {
>          g_free(s->parameters.tls_creds);
>          assert(params->tls_creds->type == QTYPE_QSTRING);
> @@ -3504,6 +3514,8 @@ static Property migration_properties[] = {
>      DEFINE_PROP_UINT8("x-cpu-throttle-increment", MigrationState,
>                        parameters.cpu_throttle_increment,
>                        DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT),
> +    DEFINE_PROP_BOOL("x-cpu-throttle-tailslow", MigrationState,
> +                      parameters.cpu_throttle_tailslow, false),
>      DEFINE_PROP_SIZE("x-max-bandwidth", MigrationState,
>                        parameters.max_bandwidth, MAX_THROTTLE),
>      DEFINE_PROP_UINT64("x-downtime-limit", MigrationState,
> @@ -3600,6 +3612,7 @@ static void migration_instance_init(Object *obj)
>      params->has_decompress_threads = true;
>      params->has_cpu_throttle_initial = true;
>      params->has_cpu_throttle_increment = true;
> +    params->has_cpu_throttle_tailslow = true;
>      params->has_max_bandwidth = true;
>      params->has_downtime_limit = true;
>      params->has_x_checkpoint_delay = true;
> diff --git a/migration/ram.c b/migration/ram.c
> index ed23ed1c7c..d86413a5d1 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -29,6 +29,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include <zlib.h>
> +#include <math.h>
>  #include "qemu/cutils.h"
>  #include "qemu/bitops.h"
>  #include "qemu/bitmap.h"
> @@ -620,15 +621,28 @@ static void mig_throttle_guest_down(void)
>  {
>      MigrationState *s = migrate_get_current();
>      uint64_t pct_initial = s->parameters.cpu_throttle_initial;
> -    uint64_t pct_icrement = s->parameters.cpu_throttle_increment;
> +    uint64_t pct_increment = s->parameters.cpu_throttle_increment;
> +    bool pct_tailslow = s->parameters.cpu_throttle_tailslow;
>      int pct_max = s->parameters.max_cpu_throttle;
>  
> +    const uint64_t cpu_throttle_now = cpu_throttle_get_percentage();
> +    uint64_t cpu_throttle_inc;
> +
>      /* We have not started throttling yet. Let's start it. */
>      if (!cpu_throttle_active()) {
>          cpu_throttle_set(pct_initial);
>      } else {
>          /* Throttling already on, just increase the rate */
> -        cpu_throttle_set(MIN(cpu_throttle_get_percentage() + pct_icrement,
> +        if (cpu_throttle_now >= 80 && pct_tailslow) {
> +            /* Eat some scale of CPU from remaining */
> +            cpu_throttle_inc = ceil((100 - cpu_throttle_now) * 0.3);
> +            if (cpu_throttle_inc > pct_increment) {
> +                cpu_throttle_inc = pct_increment;
> +            }
> +        } else {
> +            cpu_throttle_inc = pct_increment;
> +        }
> +        cpu_throttle_set(MIN(cpu_throttle_now + cpu_throttle_inc,
>                           pct_max));
>      }
>  }
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 558fe06b8f..b5f5c0b382 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -416,6 +416,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
>          monitor_printf(mon, "%s: %u\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_INCREMENT),
>              params->cpu_throttle_increment);
> +        assert(params->has_cpu_throttle_tailslow);
> +        monitor_printf(mon, "%s: %s\n",
> +            MigrationParameter_str(MIGRATION_PARAMETER_CPU_THROTTLE_TAILSLOW),
> +            params->cpu_throttle_tailslow ? "on" : "off");
>          assert(params->has_max_cpu_throttle);
>          monitor_printf(mon, "%s: %u\n",
>              MigrationParameter_str(MIGRATION_PARAMETER_MAX_CPU_THROTTLE),
> diff --git a/qapi/migration.json b/qapi/migration.json
> index b7348d0c8b..0ac94e00f2 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -532,6 +532,12 @@
>  #                          auto-converge detects that migration is not making
>  #                          progress. The default value is 10. (Since 2.7)
>  #
> +# @cpu-throttle-tailslow: Make throttle slower at tail stage
> +#                         At the tail stage of throttle, VM is very sensitive to
> +#                         CPU percentage. We just throttle 30% of remaining CPU
> +#                         when throttle is more than 80 percentage. The default
> +#                         value is false. (Since 4.1)
> +#
>  # @tls-creds: ID of the 'tls-creds' object that provides credentials for
>  #             establishing a TLS connection over the migration data channel.
>  #             On the outgoing side of the migration, the credentials must
> @@ -594,6 +600,7 @@
>             'compress-level', 'compress-threads', 'decompress-threads',
>             'compress-wait-thread',
>             'cpu-throttle-initial', 'cpu-throttle-increment',
> +           'cpu-throttle-tailslow',
>             'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>             'multifd-channels',
> @@ -634,6 +641,12 @@
>  #                          auto-converge detects that migration is not making
>  #                          progress. The default value is 10. (Since 2.7)
>  #
> +# @cpu-throttle-tailslow: Make throttle slower at tail stage
> +#                         At the tail stage of throttle, VM is very sensitive to
> +#                         CPU percentage. We just throttle 30% of remaining CPU
> +#                         when throttle is more than 80 percentage. The default
> +#                         value is false. (Since 4.1)
> +#
>  # @tls-creds: ID of the 'tls-creds' object that provides credentials
>  #             for establishing a TLS connection over the migration data
>  #             channel. On the outgoing side of the migration, the credentials
> @@ -703,6 +716,7 @@
>              '*decompress-threads': 'int',
>              '*cpu-throttle-initial': 'int',
>              '*cpu-throttle-increment': 'int',
> +            '*cpu-throttle-tailslow': 'bool',
>              '*tls-creds': 'StrOrNull',
>              '*tls-hostname': 'StrOrNull',
>              '*tls-authz': 'StrOrNull',
> @@ -767,6 +781,12 @@
>  #                          auto-converge detects that migration is not making
>  #                          progress. (Since 2.7)
>  #
> +# @cpu-throttle-tailslow: Make throttle slower at tail stage
> +#                         At the tail stage of throttle, VM is very sensitive to
> +#                         CPU percentage. We just throttle 30% of remaining CPU
> +#                         when throttle is more than 80 percentage. The default
> +#                         value is false. (Since 4.1)
> +#
>  # @tls-creds: ID of the 'tls-creds' object that provides credentials
>  #             for establishing a TLS connection over the migration data
>  #             channel. On the outgoing side of the migration, the credentials
> @@ -836,6 +856,7 @@
>              '*decompress-threads': 'uint8',
>              '*cpu-throttle-initial': 'uint8',
>              '*cpu-throttle-increment': 'uint8',
> +            '*cpu-throttle-tailslow': 'bool',
>              '*tls-creds': 'str',
>              '*tls-hostname': 'str',
>              '*tls-authz': 'str',
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] migration/throttle: Make throttle slower at tail stage

[Juan Quintela]

Keqian Zhu <zhukeqian1@huawei.com> wrote:
> At the tail stage of throttle, VM is very sensitive to
> CPU percentage. We just throttle 30% of remaining CPU
> when throttle is more than 80 percentage.

Why?

If we really think that this is better that current approarch, just do
this _always_.  And throothre 30% of remaining CPU.  So we go:

30%
30% + 0.3(70%)
...

Or anything else.

My experience is:
- you really need to go to very high throothle to make migration happens
  (more than 70% or so usually).
- The way that we throotle is not completely exact.

> This doesn't conflict with cpu_throttle_increment.
>
> This may make migration time longer, and is disabled
> by default.


What do you think?
I think that it is better to change method and improve documentation
that yet adding another parameter.

Later, Juan.

Re: [PATCH] migration/throttle: Make throttle slower at tail stage

[zhukeqian]

Hi, Juan

On 2020/2/14 20:37, Juan Quintela wrote:
> Keqian Zhu <zhukeqian1@huawei.com> wrote:
>> At the tail stage of throttle, VM is very sensitive to
>> CPU percentage. We just throttle 30% of remaining CPU
>> when throttle is more than 80 percentage.
> 
> Why?
> 
My original idea is that if we throttle a fixed percentage of CPU every time,
then the VM is more and more sensitive to performance decrease.

For example, if the initial throttle is 10% and we throttle 10% every time. At the
beginning, the performance changes from 100% to 90%, which makes little effect on VM.
However, if the dirty rate is very high and it is not enough even throttle 80%, then
the performance changes from 20% to 10%, which half the performance and makes heavy
effect on VM.

In the example above, if throttle 85% is enough, then throttle 90% makes unnecessary
performance loss on VM. So this is the reason for slowdown throttling when we are about
to reach the best throttle.

> If we really think that this is better that current approarch, just do
> this _always_.  And throothre 30% of remaining CPU.  So we go:
> 
> 30%
> 30% + 0.3(70%)
> ...
> 
> Or anything else.
> 

This should not be a new approach, instead it is an optional enhancement to

current approach. However, after my deeper thinking, the way that throttle
30% of remaining CPU is unusual and not suitable. We should use another way
to slowdown the tail stage.

When dirty bytes is is 50% more than the approx. bytes xfer, we start or increase
throttling. My idea is that we can calculate the throttle increment expected.
When dirty rate is about to reach the 50% of bandwidth, the throttle increment
expected will smaller than "cpu_throttle_increment" at tail stage.

Maybe the core code likes this:

-static void mig_throttle_guest_down(void)
+static void mig_throttle_guest_down(uint64_t bytes_dirty, uint64_t bytes_xfer)
 {
     MigrationState *s = migrate_get_current();
     uint64_t pct_initial = s->parameters.cpu_throttle_initial;
-    uint64_t pct_icrement = s->parameters.cpu_throttle_increment;
+    uint64_t pct_increment = s->parameters.cpu_throttle_increment;
+    bool pct_tailslow = s->parameters.cpu_throttle_tailslow;
     int pct_max = s->parameters.max_cpu_throttle;

+    uint64_t cpu_throttle_now = cpu_throttle_get_percentage();
+    uint64_t cpu_now, cpu_target, cpu_throttle_expect;
+    uint64_t cpu_throttle_inc;
+
     /* We have not started throttling yet. Let's start it. */
     if (!cpu_throttle_active()) {
         cpu_throttle_set(pct_initial);
     } else {
         /* Throttling already on, just increase the rate */
-        cpu_throttle_set(MIN(cpu_throttle_get_percentage() + pct_icrement,
+        cpu_throttle_inc = pct_increment;
+        if (pct_tailslow) {
+            cpu_now = 100 - cpu_throttle_now;
+            cpu_target = ((bytes_xfer / 2.0) / bytes_dirty) * cpu_now;
+            cpu_throttle_expect = cpu_now - cpu_target;
+            if (cpu_throttle_expect < pct_increment) {
+                cpu_throttle_inc = cpu_throttle_expect;
+            }
+        }
+        cpu_throttle_set(MIN(cpu_throttle_now + cpu_throttle_inc,
                          pct_max));
     }
 }
__________________________________________________________________________

-            if ((rs->num_dirty_pages_period * TARGET_PAGE_SIZE >
-                   (bytes_xfer_now - rs->bytes_xfer_prev) / 2) &&
+            bytes_dirty_period = rs->num_dirty_pages_period * TARGET_PAGE_SIZE;
+            bytes_xfer_period = bytes_xfer_now - rs->bytes_xfer_prev;
+            if ((bytes_dirty_period > bytes_xfer_period / 2) &&
                 (++rs->dirty_rate_high_cnt >= 2)) {
                     trace_migration_throttle();
                     rs->dirty_rate_high_cnt = 0;
-                    mig_throttle_guest_down();
+                    mig_throttle_guest_down(bytes_dirty_period,
+                                            bytes_xfer_period);
             }
> My experience is:
> - you really need to go to very high throothle to make migration happens
>   (more than 70% or so usually).
> - The way that we throotle is not completely exact.
> 
>> This doesn't conflict with cpu_throttle_increment.
>>
>> This may make migration time longer, and is disabled
>> by default.
> 
> 
> What do you think?
> I think that it is better to change method and improve documentation
> that yet adding another parameter.
> 
> Later, Juan.
> 
> 
> .
> 
Thanks,
Keqian

Re: [PATCH] migration/throttle: Make throttle slower at tail stage

[zhukeqian]

On 2020/2/14 19:46, Eric Blake wrote:
> On 2/13/20 9:27 PM, Keqian Zhu wrote:
>> At the tail stage of throttle, VM is very sensitive to
>> CPU percentage. We just throttle 30% of remaining CPU
>> when throttle is more than 80 percentage.
>>
>> This doesn't conflict with cpu_throttle_increment.
>>
>> This may make migration time longer, and is disabled
>> by default.
>>
>> 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
>> @@ -532,6 +532,12 @@
>>   #                          auto-converge detects that migration is not making
>>   #                          progress. The default value is 10. (Since 2.7)
>>   #
>> +# @cpu-throttle-tailslow: Make throttle slower at tail stage
>> +#                         At the tail stage of throttle, VM is very sensitive to
>> +#                         CPU percentage. We just throttle 30% of remaining CPU
>> +#                         when throttle is more than 80 percentage. The default
>> +#                         value is false. (Since 4.1)
> 
> The next release is 5.0, not 4.1.
Thanks for reminding me.
> 
Thanks,
Keqian

Re: [PATCH] migration/throttle: Make throttle slower at tail stage

[zhukeqian]

On 2020/2/14 20:28, Dr. David Alan Gilbert wrote:
> * Keqian Zhu (zhukeqian1@huawei.com) wrote:
>> At the tail stage of throttle, VM is very sensitive to
>> CPU percentage. We just throttle 30% of remaining CPU
>> when throttle is more than 80 percentage.
> 
> This is a bit unusual;  all of the rest of the throttling has no
> fixed constants; all values are set by parameters.
> 
> You've got the two, the '80' and the '0.3'
> 
> I thinkt he easy solution is to replace your parameter 'tailslow' by two
> new parameters; 'tailstart' and 'tailrate';  both defaulting to 100%.
> 
> Then you make it:
> 
>         if (cpu_throttle_now >= pct_tailstart) {
>             /* Eat some scale of CPU from remaining */
>             cpu_throttle_inc = ceil((100 - cpu_throttle_now) * pct_tailrate);
> 
> (with percentage scaling added).
> 
> Then setting 'tailstart' to 80 and 'tailrate' to 30 is equivalent to
> what you have, but means we have no magical constants in the code.
> 
Yes, this is a good suggestion. Though this patch is not the final idea,
I will apply it when throttle approach is decided.
> Dave
> 
> 
[...]
>> -- 
>> 2.19.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 
> .
> 
Thanks,
Keqian