Re: [PATCH] migration/throttle: Make throttle slower at tail stage
On 2020/2/14 20:28, Dr. David Alan Gilbert wrote:
> * Keqian Zhu (zhukeqi...@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 / dgilb...@redhat.com / Manchester, UK
>
>
> .
>
Thanks,
Keqian
Re: [PATCH] migration/throttle: Make throttle slower at tail stage
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 >> --- >> Cc: Juan Quintela >> Cc: "Dr. David Alan Gilbert" >> Cc: Eric Blake >> Cc: Markus Armbruster > >> +++ 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
Hi, Juan
On 2020/2/14 20:37, Juan Quintela wrote:
> 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.
>
> 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
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. 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
* Keqian Zhu (zhukeqi...@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
> ---
> Cc: Juan Quintela
> Cc: "Dr. David Alan Gilbert"
> Cc: Eric Blake
> Cc: Markus Armbruster
> ---
> 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
> +#include
> #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
Re: [PATCH] migration/throttle: Make throttle slower at tail stage
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 --- Cc: Juan Quintela Cc: "Dr. David Alan Gilbert" Cc: Eric Blake Cc: Markus Armbruster +++ 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
[PATCH] migration/throttle: 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.
This doesn't conflict with cpu_throttle_increment.
This may make migration time longer, and is disabled
by default.
Signed-off-by: Keqian Zhu
---
Cc: Juan Quintela
Cc: "Dr. David Alan Gilbert"
Cc: Eric Blake
Cc: Markus Armbruster
---
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
+#include
#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)
