* [PATCH v4 00/10] migration: introduce dirtylimit capability
@ 2023-02-16 16:18 huangy81
2023-02-16 16:18 ` [PATCH v4 01/10] dirtylimit: Fix overflow when computing MB huangy81
` (10 more replies)
0 siblings, 11 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
v4:
1. Polish the docs and update the release version suggested by Markus
2. Rename the migrate exported info "dirty-limit-throttle-time-per-round"
to "dirty-limit-throttle-time-per-full".
The following 5 commits hasn't been acked or reviewed yet:
kvm: dirty-ring: Fix race with vcpu creation
qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
migration: Implement dirty-limit convergence algo
migration: Extend query-migrate to provide dirty page limit info
tests: Add migration dirty-limit capability test
Ping David and Juan.
Please review if you have time. Thanks.
Yong
v3(resend):
- fix the syntax error of the topic.
v3:
This version make some modifications inspired by Peter and Markus
as following:
1. Do the code clean up in [PATCH v2 02/11] suggested by Markus
2. Replace the [PATCH v2 03/11] with a much simpler patch posted by
Peter to fix the following bug:
https://bugzilla.redhat.com/show_bug.cgi?id=2124756
3. Fix the error path of migrate_params_check in [PATCH v2 04/11]
pointed out by Markus. Enrich the commit message to explain why
x-vcpu-dirty-limit-period an unstable parameter.
4. Refactor the dirty-limit convergence algo in [PATCH v2 07/11]
suggested by Peter:
a. apply blk_mig_bulk_active check before enable dirty-limit
b. drop the unhelpful check function before enable dirty-limit
c. change the migration_cancel logic, just cancel dirty-limit
only if dirty-limit capability turned on.
d. abstract a code clean commit [PATCH v3 07/10] to adjust
the check order before enable auto-converge
5. Change the name of observing indexes during dirty-limit live
migration to make them more easy-understanding. Use the
maximum throttle time of vpus as "dirty-limit-throttle-time-per-full"
6. Fix some grammatical and spelling errors pointed out by Markus
and enrich the document about the dirty-limit live migration
observing indexes "dirty-limit-ring-full-time"
and "dirty-limit-throttle-time-per-full"
7. Change the default value of x-vcpu-dirty-limit-period to 1000ms,
which is optimal value pointed out in cover letter in that
testing environment.
8. Drop the 2 guestperf test commits [PATCH v2 10/11],
[PATCH v2 11/11] and post them with a standalone series in the
future.
Thanks Peter and Markus sincerely for the passionate, efficient
and careful comments and suggestions.
Please review.
Yong
v2:
This version make a little bit modifications comparing with
version 1 as following:
1. fix the overflow issue reported by Peter Maydell
2. add parameter check for hmp "set_vcpu_dirty_limit" command
3. fix the racing issue between dirty ring reaper thread and
Qemu main thread.
4. add migrate parameter check for x-vcpu-dirty-limit-period
and vcpu-dirty-limit.
5. add the logic to forbid hmp/qmp commands set_vcpu_dirty_limit,
cancel_vcpu_dirty_limit during dirty-limit live migration when
implement dirty-limit convergence algo.
6. add capability check to ensure auto-converge and dirty-limit
are mutually exclusive.
7. pre-check if kvm dirty ring size is configured before setting
dirty-limit migrate parameter
A more comprehensive test was done comparing with version 1.
The following are test environment:
-------------------------------------------------------------
a. Host hardware info:
CPU:
Intel(R) Xeon(R) Gold 5218 CPU @ 2.30GHz
CPU(s): 64
On-line CPU(s) list: 0-63
Thread(s) per core: 2
Core(s) per socket: 16
Socket(s): 2
NUMA node(s): 2
NUMA node0 CPU(s): 0-15,32-47
NUMA node1 CPU(s): 16-31,48-63
Memory:
Hynix 503Gi
Interface:
Intel Corporation Ethernet Connection X722 for 1GbE (rev 09)
Speed: 1000Mb/s
b. Host software info:
OS: ctyunos release 2
Kernel: 4.19.90-2102.2.0.0066.ctl2.x86_64
Libvirt baseline version: libvirt-6.9.0
Qemu baseline version: qemu-5.0
c. vm scale
CPU: 4
Memory: 4G
-------------------------------------------------------------
All the supplementary test data shown as follows are basing on
above test environment.
In version 1, we post a test data from unixbench as follows:
$ taskset -c 8-15 ./Run -i 2 -c 8 {unixbench test item}
host cpu: Intel(R) Xeon(R) Platinum 8378A
host interface speed: 1000Mb/s
|---------------------+--------+------------+---------------|
| UnixBench test item | Normal | Dirtylimit | Auto-converge |
|---------------------+--------+------------+---------------|
| dhry2reg | 32800 | 32786 | 25292 |
| whetstone-double | 10326 | 10315 | 9847 |
| pipe | 15442 | 15271 | 14506 |
| context1 | 7260 | 6235 | 4514 |
| spawn | 3663 | 3317 | 3249 |
| syscall | 4669 | 4667 | 3841 |
|---------------------+--------+------------+---------------|
In version 2, we post a supplementary test data that do not use
taskset and make the scenario more general, see as follows:
$ ./Run
per-vcpu data:
|---------------------+--------+------------+---------------|
| UnixBench test item | Normal | Dirtylimit | Auto-converge |
|---------------------+--------+------------+---------------|
| dhry2reg | 2991 | 2902 | 1722 |
| whetstone-double | 1018 | 1006 | 627 |
| Execl Throughput | 955 | 320 | 660 |
| File Copy - 1 | 2362 | 805 | 1325 |
| File Copy - 2 | 1500 | 1406 | 643 |
| File Copy - 3 | 4778 | 2160 | 1047 |
| Pipe Throughput | 1181 | 1170 | 842 |
| Context Switching | 192 | 224 | 198 |
| Process Creation | 490 | 145 | 95 |
| Shell Scripts - 1 | 1284 | 565 | 610 |
| Shell Scripts - 2 | 2368 | 900 | 1040 |
| System Call Overhead| 983 | 948 | 698 |
| Index Score | 1263 | 815 | 600 |
|---------------------+--------+------------+---------------|
Note:
File Copy - 1: File Copy 1024 bufsize 2000 maxblocks
File Copy - 2: File Copy 256 bufsize 500 maxblocks
File Copy - 3: File Copy 4096 bufsize 8000 maxblocks
Shell Scripts - 1: Shell Scripts (1 concurrent)
Shell Scripts - 2: Shell Scripts (8 concurrent)
Basing on above data, we can draw a conclusion that dirty-limit
can hugely improve the system benchmark almost in every respect,
the "System Benchmarks Index Score" show it improve 35% performance
comparing with auto-converge during live migration.
4-vcpu parallel data(we run a test vm with 4c4g-scale):
|---------------------+--------+------------+---------------|
| UnixBench test item | Normal | Dirtylimit | Auto-converge |
|---------------------+--------+------------+---------------|
| dhry2reg | 7975 | 7146 | 5071 |
| whetstone-double | 3982 | 3561 | 2124 |
| Execl Throughput | 1882 | 1205 | 768 |
| File Copy - 1 | 1061 | 865 | 498 |
| File Copy - 2 | 676 | 491 | 519 |
| File Copy - 3 | 2260 | 923 | 1329 |
| Pipe Throughput | 3026 | 3009 | 1616 |
| Context Switching | 1219 | 1093 | 695 |
| Process Creation | 947 | 307 | 446 |
| Shell Scripts - 1 | 2469 | 977 | 989 |
| Shell Scripts - 2 | 2667 | 1275 | 984 |
| System Call Overhead| 1592 | 1459 | 692 |
| Index Score | 1976 | 1294 | 997 |
|---------------------+--------+------------+---------------|
For the parallel data, the "System Benchmarks Index Score" show it
also improve 29% performance.
In version 1, migration total time is shown as follows:
host cpu: Intel(R) Xeon(R) Platinum 8378A
host interface speed: 1000Mb/s
|-----------------------+----------------+-------------------|
| dirty memory size(MB) | Dirtylimit(ms) | Auto-converge(ms) |
|-----------------------+----------------+-------------------|
| 60 | 2014 | 2131 |
| 70 | 5381 | 12590 |
| 90 | 6037 | 33545 |
| 110 | 7660 | [*] |
|-----------------------+----------------+-------------------|
[*]: This case means migration is not convergent.
In version 2, we post more comprehensive migration total time test data
as follows:
we update N MB on 4 cpus and sleep S us every time 1 MB data was updated.
test twice in each condition, data is shown as follow:
|-----------+--------+--------+----------------+-------------------|
| ring size | N (MB) | S (us) | Dirtylimit(ms) | Auto-converge(ms) |
|-----------+--------+--------+----------------+-------------------|
| 1024 | 1024 | 1000 | 44951 | 191780 |
| 1024 | 1024 | 1000 | 44546 | 185341 |
| 1024 | 1024 | 500 | 46505 | 203545 |
| 1024 | 1024 | 500 | 45469 | 909945 |
| 1024 | 1024 | 0 | 61858 | [*] |
| 1024 | 1024 | 0 | 57922 | [*] |
| 1024 | 2048 | 0 | 91982 | [*] |
| 1024 | 2048 | 0 | 90388 | [*] |
| 2048 | 128 | 10000 | 14511 | 25971 |
| 2048 | 128 | 10000 | 13472 | 26294 |
| 2048 | 1024 | 10000 | 44244 | 26294 |
| 2048 | 1024 | 10000 | 45099 | 157701 |
| 2048 | 1024 | 500 | 51105 | [*] |
| 2048 | 1024 | 500 | 49648 | [*] |
| 2048 | 1024 | 0 | 229031 | [*] |
| 2048 | 1024 | 0 | 154282 | [*] |
|-----------+--------+--------+----------------+-------------------|
[*]: This case means migration is not convergent.
Not that the larger ring size is, the less sensitively dirty-limit responds,
so we should choose a optimal ring size base on the test data with different
scale vm.
We also test the effect of "x-vcpu-dirty-limit-period" parameter on
migration total time. test twice in each condition, data is shown
as follows:
|-----------+--------+--------+-------------+----------------------|
| ring size | N (MB) | S (us) | Period (ms) | migration total time |
|-----------+--------+--------+-------------+----------------------|
| 2048 | 1024 | 10000 | 100 | [*] |
| 2048 | 1024 | 10000 | 100 | [*] |
| 2048 | 1024 | 10000 | 300 | 156795 |
| 2048 | 1024 | 10000 | 300 | 118179 |
| 2048 | 1024 | 10000 | 500 | 44244 |
| 2048 | 1024 | 10000 | 500 | 45099 |
| 2048 | 1024 | 10000 | 700 | 41871 |
| 2048 | 1024 | 10000 | 700 | 42582 |
| 2048 | 1024 | 10000 | 1000 | 41430 |
| 2048 | 1024 | 10000 | 1000 | 40383 |
| 2048 | 1024 | 10000 | 1500 | 42030 |
| 2048 | 1024 | 10000 | 1500 | 42598 |
| 2048 | 1024 | 10000 | 2000 | 41694 |
| 2048 | 1024 | 10000 | 2000 | 42403 |
| 2048 | 1024 | 10000 | 3000 | 43538 |
| 2048 | 1024 | 10000 | 3000 | 43010 |
|-----------+--------+--------+-------------+----------------------|
It shows that x-vcpu-dirty-limit-period should be configured with 1000 ms
in above condition.
Please review, any comments and suggestions are very appreciated, thanks
Yong
Hyman Huang (9):
dirtylimit: Fix overflow when computing MB
softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit"
qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
qapi/migration: Introduce vcpu-dirty-limit parameters
migration: Introduce dirty-limit capability
migration: Refactor auto-converge capability logic
migration: Implement dirty-limit convergence algo
migration: Extend query-migrate to provide dirty page limit info
tests: Add migration dirty-limit capability test
Peter Xu (1):
kvm: dirty-ring: Fix race with vcpu creation
accel/kvm/kvm-all.c | 9 ++
include/sysemu/dirtylimit.h | 2 +
migration/migration-hmp-cmds.c | 26 ++++++
migration/migration.c | 88 ++++++++++++++++++
migration/migration.h | 1 +
migration/ram.c | 63 ++++++++++---
migration/trace-events | 1 +
qapi/migration.json | 64 ++++++++++++--
softmmu/dirtylimit.c | 91 ++++++++++++++++---
tests/qtest/migration-test.c | 157 +++++++++++++++++++++++++++++++++
10 files changed, 470 insertions(+), 32 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 01/10] dirtylimit: Fix overflow when computing MB
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
@ 2023-02-16 16:18 ` huangy81
2023-02-16 16:18 ` [PATCH v4 02/10] softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit" huangy81
` (9 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Coverity points out a overflow problem when computing MB,
dirty_ring_size and TARGET_PAGE_SIZE are both 32 bits,
multiplication will be done as a 32-bit operation, which
could overflow. Simplify the formula.
Meanwhile, fix spelling mistake of variable name.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
softmmu/dirtylimit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index c56f0f58c8..065ed18afc 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -235,14 +235,14 @@ static inline int64_t dirtylimit_dirty_ring_full_time(uint64_t dirtyrate)
{
static uint64_t max_dirtyrate;
uint32_t dirty_ring_size = kvm_dirty_ring_size();
- uint64_t dirty_ring_size_meory_MB =
- dirty_ring_size * TARGET_PAGE_SIZE >> 20;
+ uint32_t dirty_ring_size_memory_MB =
+ dirty_ring_size >> (20 - TARGET_PAGE_BITS);
if (max_dirtyrate < dirtyrate) {
max_dirtyrate = dirtyrate;
}
- return dirty_ring_size_meory_MB * 1000000 / max_dirtyrate;
+ return dirty_ring_size_memory_MB * 1000000ULL / max_dirtyrate;
}
static inline bool dirtylimit_done(uint64_t quota,
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 02/10] softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit"
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
2023-02-16 16:18 ` [PATCH v4 01/10] dirtylimit: Fix overflow when computing MB huangy81
@ 2023-02-16 16:18 ` huangy81
2023-02-16 16:18 ` [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation huangy81
` (8 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
dirty_rate paraemter of hmp command "set_vcpu_dirty_limit" is invalid
if less than 0, so add parameter check for it.
Note that this patch also delete the unsolicited help message and
clean up the code.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
softmmu/dirtylimit.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index 065ed18afc..dcab9bf2b1 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -514,14 +514,15 @@ void hmp_set_vcpu_dirty_limit(Monitor *mon, const QDict *qdict)
int64_t cpu_index = qdict_get_try_int(qdict, "cpu_index", -1);
Error *err = NULL;
- qmp_set_vcpu_dirty_limit(!!(cpu_index != -1), cpu_index, dirty_rate, &err);
- if (err) {
- hmp_handle_error(mon, err);
- return;
+ if (dirty_rate < 0) {
+ error_setg(&err, "invalid dirty page limit %ld", dirty_rate);
+ goto out;
}
- monitor_printf(mon, "[Please use 'info vcpu_dirty_limit' to query "
- "dirty limit for virtual CPU]\n");
+ qmp_set_vcpu_dirty_limit(!!(cpu_index != -1), cpu_index, dirty_rate, &err);
+
+out:
+ hmp_handle_error(mon, err);
}
static struct DirtyLimitInfo *dirtylimit_query_vcpu(int cpu_index)
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
2023-02-16 16:18 ` [PATCH v4 01/10] dirtylimit: Fix overflow when computing MB huangy81
2023-02-16 16:18 ` [PATCH v4 02/10] softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit" huangy81
@ 2023-02-16 16:18 ` huangy81
2023-04-04 13:32 ` Paolo Bonzini
2023-02-16 16:18 ` [PATCH v4 04/10] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter huangy81
` (7 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson
From: Peter Xu <peterx@redhat.com>
It's possible that we want to reap a dirty ring on a vcpu that is during
creation, because the vcpu is put onto list (CPU_FOREACH visible) before
initialization of the structures. In this case:
qemu_init_vcpu
x86_cpu_realizefn
cpu_exec_realizefn
cpu_list_add <---- can be probed by CPU_FOREACH
qemu_init_vcpu
cpus_accel->create_vcpu_thread(cpu);
kvm_init_vcpu
map kvm_dirty_gfns <--- kvm_dirty_gfns valid
Don't try to reap dirty ring on vcpus during creation or it'll crash.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2124756
Reported-by: Xiaohui Li <xiaohli@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
accel/kvm/kvm-all.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 9b26582655..47483cdfa0 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -685,6 +685,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu)
uint32_t ring_size = s->kvm_dirty_ring_size;
uint32_t count = 0, fetch = cpu->kvm_fetch_index;
+ /*
+ * It's possible that we race with vcpu creation code where the vcpu is
+ * put onto the vcpus list but not yet initialized the dirty ring
+ * structures. If so, skip it.
+ */
+ if (!cpu->created) {
+ return 0;
+ }
+
assert(dirty_gfns && ring_size);
trace_kvm_dirty_ring_reap_vcpu(cpu->cpu_index);
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 04/10] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (2 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation huangy81
@ 2023-02-16 16:18 ` huangy81
2023-02-16 16:18 ` [PATCH v4 05/10] qapi/migration: Introduce vcpu-dirty-limit parameters huangy81
` (6 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Introduce "x-vcpu-dirty-limit-period" migration experimental
parameter, which is in the range of 1 to 1000ms and used to
make dirtyrate calculation period configurable.
Currently with the "x-vcpu-dirty-limit-period" varies, the
total time of live migration changes, test results show the
optimal value of "x-vcpu-dirty-limit-period" ranges from
500ms to 1000 ms. "x-vcpu-dirty-limit-period" should be made
stable once it proves best value can not be determined with
developer's experiments.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
migration/migration-hmp-cmds.c | 8 ++++++++
migration/migration.c | 27 +++++++++++++++++++++++++++
qapi/migration.json | 33 ++++++++++++++++++++++++++-------
3 files changed, 61 insertions(+), 7 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index ef25bc8929..3bc751bec9 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -345,6 +345,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
}
}
}
+
+ monitor_printf(mon, "%s: %" PRIu64 " ms\n",
+ MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD),
+ params->x_vcpu_dirty_limit_period);
}
qapi_free_MigrationParameters(params);
@@ -601,6 +605,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
error_setg(&err, "The block-bitmap-mapping parameter can only be set "
"through QMP");
break;
+ case MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD:
+ p->has_x_vcpu_dirty_limit_period = true;
+ visit_type_size(v, param, &p->x_vcpu_dirty_limit_period, &err);
+ break;
default:
assert(0);
}
diff --git a/migration/migration.c b/migration/migration.c
index 90fca70cb7..6162f048ae 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -119,6 +119,8 @@
#define DEFAULT_MIGRATE_ANNOUNCE_ROUNDS 5
#define DEFAULT_MIGRATE_ANNOUNCE_STEP 100
+#define DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT_PERIOD 1000 /* microsecond */
+
static NotifierList migration_state_notifiers =
NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -1016,6 +1018,9 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
s->parameters.block_bitmap_mapping);
}
+ params->has_x_vcpu_dirty_limit_period = true;
+ params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
+
return params;
}
@@ -1660,6 +1665,15 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp)
}
#endif
+ if (params->has_x_vcpu_dirty_limit_period &&
+ (params->x_vcpu_dirty_limit_period < 1 ||
+ params->x_vcpu_dirty_limit_period > 1000)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ "x-vcpu-dirty-limit-period",
+ "a value between 1 and 1000");
+ return false;
+ }
+
return true;
}
@@ -1759,6 +1773,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
dest->has_block_bitmap_mapping = true;
dest->block_bitmap_mapping = params->block_bitmap_mapping;
}
+
+ if (params->has_x_vcpu_dirty_limit_period) {
+ dest->x_vcpu_dirty_limit_period = params->x_vcpu_dirty_limit_period;
+ }
}
static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1881,6 +1899,11 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
QAPI_CLONE(BitmapMigrationNodeAliasList,
params->block_bitmap_mapping);
}
+
+ if (params->has_x_vcpu_dirty_limit_period) {
+ s->parameters.x_vcpu_dirty_limit_period =
+ params->x_vcpu_dirty_limit_period;
+ }
}
void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
@@ -4513,6 +4536,9 @@ static Property migration_properties[] = {
DEFINE_PROP_STRING("tls-creds", MigrationState, parameters.tls_creds),
DEFINE_PROP_STRING("tls-hostname", MigrationState, parameters.tls_hostname),
DEFINE_PROP_STRING("tls-authz", MigrationState, parameters.tls_authz),
+ DEFINE_PROP_UINT64("x-vcpu-dirty-limit-period", MigrationState,
+ parameters.x_vcpu_dirty_limit_period,
+ DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT_PERIOD),
/* Migration capabilities */
DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
@@ -4602,6 +4628,7 @@ static void migration_instance_init(Object *obj)
params->has_announce_max = true;
params->has_announce_rounds = true;
params->has_announce_step = true;
+ params->has_x_vcpu_dirty_limit_period = true;
qemu_sem_init(&ms->postcopy_pause_sem, 0);
qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
diff --git a/qapi/migration.json b/qapi/migration.json
index c84fa10e86..f43e4061b4 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -775,9 +775,13 @@
# names are mapped to themselves. Nodes are mapped to their
# block device name if there is one, and to their node name
# otherwise. (Since 5.2)
+# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty limit during
+# live migration. Should be in the range 1 to 1000ms,
+# defaults to 1000ms. (Since 8.0)
#
# Features:
-# @unstable: Member @x-checkpoint-delay is experimental.
+# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
+# are experimental.
#
# Since: 2.4
##
@@ -795,8 +799,9 @@
'multifd-channels',
'xbzrle-cache-size', 'max-postcopy-bandwidth',
'max-cpu-throttle', 'multifd-compression',
- 'multifd-zlib-level' ,'multifd-zstd-level',
- 'block-bitmap-mapping' ] }
+ 'multifd-zlib-level', 'multifd-zstd-level',
+ 'block-bitmap-mapping',
+ { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] } ] }
##
# @MigrateSetParameters:
@@ -941,8 +946,13 @@
# block device name if there is one, and to their node name
# otherwise. (Since 5.2)
#
+# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty limit during
+# live migration. Should be in the range 1 to 1000ms,
+# defaults to 1000ms. (Since 8.0)
+#
# Features:
-# @unstable: Member @x-checkpoint-delay is experimental.
+# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
+# are experimental.
#
# Since: 2.4
##
@@ -976,7 +986,9 @@
'*multifd-compression': 'MultiFDCompression',
'*multifd-zlib-level': 'uint8',
'*multifd-zstd-level': 'uint8',
- '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } }
+ '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
+ '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
+ 'features': [ 'unstable' ] } } }
##
# @migrate-set-parameters:
@@ -1141,8 +1153,13 @@
# block device name if there is one, and to their node name
# otherwise. (Since 5.2)
#
+# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty limit during
+# live migration. Should be in the range 1 to 1000ms,
+# defaults to 1000ms. (Since 8.0)
+#
# Features:
-# @unstable: Member @x-checkpoint-delay is experimental.
+# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
+# are experimental.
#
# Since: 2.4
##
@@ -1174,7 +1191,9 @@
'*multifd-compression': 'MultiFDCompression',
'*multifd-zlib-level': 'uint8',
'*multifd-zstd-level': 'uint8',
- '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ] } }
+ '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
+ '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
+ 'features': [ 'unstable' ] } } }
##
# @query-migrate-parameters:
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 05/10] qapi/migration: Introduce vcpu-dirty-limit parameters
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (3 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 04/10] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter huangy81
@ 2023-02-16 16:18 ` huangy81
2023-02-16 16:18 ` [PATCH v4 06/10] migration: Introduce dirty-limit capability huangy81
` (5 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Introduce "vcpu-dirty-limit" migration parameter used
to limit dirty page rate during live migration.
"vcpu-dirty-limit" and "x-vcpu-dirty-limit-period" are
two dirty-limit-related migration parameters, which can
be set before and during live migration by qmp
migrate-set-parameters.
This two parameters are used to help implement the dirty
page rate limit algo of migration.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Acked-by: Peter Xu <peterx@redhat.com>
---
migration/migration-hmp-cmds.c | 8 ++++++++
migration/migration.c | 23 +++++++++++++++++++++++
qapi/migration.json | 18 +++++++++++++++---
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 3bc751bec9..a61ec80d9d 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -349,6 +349,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "%s: %" PRIu64 " ms\n",
MigrationParameter_str(MIGRATION_PARAMETER_X_VCPU_DIRTY_LIMIT_PERIOD),
params->x_vcpu_dirty_limit_period);
+
+ monitor_printf(mon, "%s: %" PRIu64 " MB/s\n",
+ MigrationParameter_str(MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT),
+ params->vcpu_dirty_limit);
}
qapi_free_MigrationParameters(params);
@@ -609,6 +613,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
p->has_x_vcpu_dirty_limit_period = true;
visit_type_size(v, param, &p->x_vcpu_dirty_limit_period, &err);
break;
+ case MIGRATION_PARAMETER_VCPU_DIRTY_LIMIT:
+ p->has_vcpu_dirty_limit = true;
+ visit_type_size(v, param, &p->vcpu_dirty_limit, &err);
+ break;
default:
assert(0);
}
diff --git a/migration/migration.c b/migration/migration.c
index 6162f048ae..e479c86575 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -120,6 +120,7 @@
#define DEFAULT_MIGRATE_ANNOUNCE_STEP 100
#define DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT_PERIOD 1000 /* microsecond */
+#define DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT 1 /* MB/s */
static NotifierList migration_state_notifiers =
NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -1021,6 +1022,9 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->has_x_vcpu_dirty_limit_period = true;
params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
+ params->has_vcpu_dirty_limit = true;
+ params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
+
return params;
}
@@ -1674,6 +1678,14 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
+ if (params->has_vcpu_dirty_limit &&
+ (params->vcpu_dirty_limit < 1)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ "vcpu_dirty_limit",
+ "is invalid, it must greater then 1 MB/s");
+ return false;
+ }
+
return true;
}
@@ -1777,6 +1789,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
if (params->has_x_vcpu_dirty_limit_period) {
dest->x_vcpu_dirty_limit_period = params->x_vcpu_dirty_limit_period;
}
+
+ if (params->has_vcpu_dirty_limit) {
+ dest->vcpu_dirty_limit = params->vcpu_dirty_limit;
+ }
}
static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1904,6 +1920,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
s->parameters.x_vcpu_dirty_limit_period =
params->x_vcpu_dirty_limit_period;
}
+ if (params->has_vcpu_dirty_limit) {
+ s->parameters.vcpu_dirty_limit = params->vcpu_dirty_limit;
+ }
}
void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
@@ -4539,6 +4558,9 @@ static Property migration_properties[] = {
DEFINE_PROP_UINT64("x-vcpu-dirty-limit-period", MigrationState,
parameters.x_vcpu_dirty_limit_period,
DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT_PERIOD),
+ DEFINE_PROP_UINT64("vcpu-dirty-limit", MigrationState,
+ parameters.vcpu_dirty_limit,
+ DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT),
/* Migration capabilities */
DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
@@ -4629,6 +4651,7 @@ static void migration_instance_init(Object *obj)
params->has_announce_rounds = true;
params->has_announce_step = true;
params->has_x_vcpu_dirty_limit_period = true;
+ params->has_vcpu_dirty_limit = true;
qemu_sem_init(&ms->postcopy_pause_sem, 0);
qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
diff --git a/qapi/migration.json b/qapi/migration.json
index f43e4061b4..d33cc2d582 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -779,6 +779,9 @@
# live migration. Should be in the range 1 to 1000ms,
# defaults to 1000ms. (Since 8.0)
#
+# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
+# Defaults to 1. (Since 8.0)
+#
# Features:
# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
# are experimental.
@@ -801,7 +804,8 @@
'max-cpu-throttle', 'multifd-compression',
'multifd-zlib-level', 'multifd-zstd-level',
'block-bitmap-mapping',
- { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] } ] }
+ { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
+ 'vcpu-dirty-limit'] }
##
# @MigrateSetParameters:
@@ -950,6 +954,9 @@
# live migration. Should be in the range 1 to 1000ms,
# defaults to 1000ms. (Since 8.0)
#
+# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
+# Defaults to 1. (Since 8.0)
+#
# Features:
# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
# are experimental.
@@ -988,7 +995,8 @@
'*multifd-zstd-level': 'uint8',
'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
'*x-vcpu-dirty-limit-period': { 'type': 'uint64',
- 'features': [ 'unstable' ] } } }
+ 'features': [ 'unstable' ] },
+ '*vcpu-dirty-limit': 'uint64'} }
##
# @migrate-set-parameters:
@@ -1157,6 +1165,9 @@
# live migration. Should be in the range 1 to 1000ms,
# defaults to 1000ms. (Since 8.0)
#
+# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
+# Defaults to 1. (Since 8.0)
+#
# Features:
# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
# are experimental.
@@ -1193,7 +1204,8 @@
'*multifd-zstd-level': 'uint8',
'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
'*x-vcpu-dirty-limit-period': { 'type': 'uint64',
- 'features': [ 'unstable' ] } } }
+ 'features': [ 'unstable' ] },
+ '*vcpu-dirty-limit': 'uint64'} }
##
# @query-migrate-parameters:
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 06/10] migration: Introduce dirty-limit capability
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (4 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 05/10] qapi/migration: Introduce vcpu-dirty-limit parameters huangy81
@ 2023-02-16 16:18 ` huangy81
2023-03-24 12:11 ` Markus Armbruster
2023-02-16 16:18 ` [PATCH v4 07/10] migration: Refactor auto-converge capability logic huangy81
` (4 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Introduce migration dirty-limit capability, which can
be turned on before live migration and limit dirty
page rate durty live migration.
Introduce migrate_dirty_limit function to help check
if dirty-limit capability enabled during live migration.
Meanwhile, refactor vcpu_dirty_rate_stat_collect
so that period can be configured instead of hardcoded.
dirty-limit capability is kind of like auto-converge
but using dirty limit instead of traditional cpu-throttle
to throttle guest down. To enable this feature, turn on
the dirty-limit capability before live migration using
migrate-set-capabilities, and set the parameters
"x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
to speed up convergence.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Acked-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 25 +++++++++++++++++++++++++
migration/migration.h | 1 +
qapi/migration.json | 4 +++-
softmmu/dirtylimit.c | 11 ++++++++++-
4 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index e479c86575..f890e5966a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -64,6 +64,7 @@
#include "yank_functions.h"
#include "sysemu/qtest.h"
#include "ui/qemu-spice.h"
+#include "sysemu/kvm.h"
#define MAX_THROTTLE (128 << 20) /* Migration transfer speed throttling */
@@ -1444,6 +1445,20 @@ static bool migrate_caps_check(bool *cap_list,
}
}
+ if (cap_list[MIGRATION_CAPABILITY_DIRTY_LIMIT]) {
+ if (cap_list[MIGRATION_CAPABILITY_AUTO_CONVERGE]) {
+ error_setg(errp, "dirty-limit conflicts with auto-converge"
+ " either of then available currently");
+ return false;
+ }
+
+ if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
+ error_setg(errp, "dirty-limit requires KVM with accelerator"
+ " property 'dirty-ring-size' set");
+ return false;
+ }
+ }
+
return true;
}
@@ -2635,6 +2650,15 @@ bool migrate_auto_converge(void)
return s->enabled_capabilities[MIGRATION_CAPABILITY_AUTO_CONVERGE];
}
+bool migrate_dirty_limit(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->enabled_capabilities[MIGRATION_CAPABILITY_DIRTY_LIMIT];
+}
+
bool migrate_zero_blocks(void)
{
MigrationState *s;
@@ -4583,6 +4607,7 @@ static Property migration_properties[] = {
DEFINE_PROP_MIG_CAP("x-zero-copy-send",
MIGRATION_CAPABILITY_ZERO_COPY_SEND),
#endif
+ DEFINE_PROP_MIG_CAP("x-dirty-limit", MIGRATION_CAPABILITY_DIRTY_LIMIT),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/migration/migration.h b/migration/migration.h
index 2da2f8a164..cd2e9bfeea 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -418,6 +418,7 @@ bool migrate_ignore_shared(void);
bool migrate_validate_uuid(void);
bool migrate_auto_converge(void);
+bool migrate_dirty_limit(void);
bool migrate_use_multifd(void);
bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
diff --git a/qapi/migration.json b/qapi/migration.json
index d33cc2d582..b7a92be055 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -477,6 +477,8 @@
# will be handled faster. This is a performance feature and
# should not affect the correctness of postcopy migration.
# (since 7.1)
+# @dirty-limit: Use dirty-limit to throttle down guest if enabled.
+# (since 8.0)
#
# Features:
# @unstable: Members @x-colo and @x-ignore-shared are experimental.
@@ -492,7 +494,7 @@
'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
{ 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
'validate-uuid', 'background-snapshot',
- 'zero-copy-send', 'postcopy-preempt'] }
+ 'zero-copy-send', 'postcopy-preempt', 'dirty-limit'] }
##
# @MigrationCapabilityStatus:
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index dcab9bf2b1..52d1b2c6fa 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -22,6 +22,8 @@
#include "exec/memory.h"
#include "hw/boards.h"
#include "sysemu/kvm.h"
+#include "migration/misc.h"
+#include "migration/migration.h"
#include "trace.h"
/*
@@ -74,11 +76,18 @@ static bool dirtylimit_quit;
static void vcpu_dirty_rate_stat_collect(void)
{
+ MigrationState *s = migrate_get_current();
VcpuStat stat;
int i = 0;
+ int64_t period = DIRTYLIMIT_CALC_TIME_MS;
+
+ if (migrate_dirty_limit() &&
+ migration_is_active(s)) {
+ period = s->parameters.x_vcpu_dirty_limit_period;
+ }
/* calculate vcpu dirtyrate */
- vcpu_calculate_dirtyrate(DIRTYLIMIT_CALC_TIME_MS,
+ vcpu_calculate_dirtyrate(period,
&stat,
GLOBAL_DIRTY_LIMIT,
false);
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 07/10] migration: Refactor auto-converge capability logic
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (5 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 06/10] migration: Introduce dirty-limit capability huangy81
@ 2023-02-16 16:18 ` huangy81
2023-02-16 16:18 ` [PATCH v4 08/10] migration: Implement dirty-limit convergence algo huangy81
` (3 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Check if block migration is running before throttling
guest down in auto-converge way.
Note that this modification is kind of like code clean,
because block migration does not depend on auto-converge
capability, so the order of checks can be adjusted.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Acked-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/migration/ram.c b/migration/ram.c
index 521912385d..3e5dff4068 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1200,7 +1200,11 @@ static void migration_trigger_throttle(RAMState *rs)
/* During block migration the auto-converge logic incorrectly detects
* that ram migration makes no progress. Avoid this by disabling the
* throttling logic during the bulk phase of block migration. */
- if (migrate_auto_converge() && !blk_mig_bulk_active()) {
+ if (blk_mig_bulk_active()) {
+ return;
+ }
+
+ if (migrate_auto_converge()) {
/* The following detection logic can be refined later. For now:
Check to see if the ratio between dirtied bytes and the approx.
amount of bytes that just got transferred since the last time
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 08/10] migration: Implement dirty-limit convergence algo
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (6 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 07/10] migration: Refactor auto-converge capability logic huangy81
@ 2023-02-16 16:18 ` huangy81
2023-02-16 16:18 ` [PATCH v4 09/10] migration: Extend query-migrate to provide dirty page limit info huangy81
` (2 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Implement dirty-limit convergence algo for live migration,
which is kind of like auto-converge algo but using dirty-limit
instead of cpu throttle to make migration convergent.
Enable dirty page limit if dirty_rate_high_cnt greater than 2
when dirty-limit capability enabled, Disable dirty-limit if
migration be cancled.
Note that "set_vcpu_dirty_limit", "cancel_vcpu_dirty_limit"
commands are not allowed during dirty-limit live migration.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
migration/migration.c | 3 ++
migration/ram.c | 63 ++++++++++++++++++++++++++++++++----------
migration/trace-events | 1 +
softmmu/dirtylimit.c | 22 +++++++++++++++
4 files changed, 74 insertions(+), 15 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index f890e5966a..7ccbc07257 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -256,6 +256,9 @@ void migration_cancel(const Error *error)
if (error) {
migrate_set_error(current_migration, error);
}
+ if (migrate_dirty_limit()) {
+ qmp_cancel_vcpu_dirty_limit(false, -1, NULL);
+ }
migrate_fd_cancel(current_migration);
}
diff --git a/migration/ram.c b/migration/ram.c
index 3e5dff4068..24d26b5135 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -45,6 +45,7 @@
#include "qapi/error.h"
#include "qapi/qapi-types-migration.h"
#include "qapi/qapi-events-migration.h"
+#include "qapi/qapi-commands-migration.h"
#include "qapi/qmp/qerror.h"
#include "trace.h"
#include "exec/ram_addr.h"
@@ -57,6 +58,8 @@
#include "qemu/iov.h"
#include "multifd.h"
#include "sysemu/runstate.h"
+#include "sysemu/dirtylimit.h"
+#include "sysemu/kvm.h"
#include "hw/boards.h" /* for machine_dump_guest_core() */
@@ -1188,6 +1191,30 @@ static void migration_update_rates(RAMState *rs, int64_t end_time)
}
}
+/*
+ * Enable dirty-limit to throttle down the guest
+ */
+static void migration_dirty_limit_guest(void)
+{
+ static int64_t quota_dirtyrate;
+ MigrationState *s = migrate_get_current();
+
+ /*
+ * If dirty limit already enabled and migration parameter
+ * vcpu-dirty-limit untouched.
+ */
+ if (dirtylimit_in_service() &&
+ quota_dirtyrate == s->parameters.vcpu_dirty_limit) {
+ return;
+ }
+
+ quota_dirtyrate = s->parameters.vcpu_dirty_limit;
+
+ /* Set or update quota dirty limit */
+ qmp_set_vcpu_dirty_limit(false, -1, quota_dirtyrate, NULL);
+ trace_migration_dirty_limit_guest(quota_dirtyrate);
+}
+
static void migration_trigger_throttle(RAMState *rs)
{
MigrationState *s = migrate_get_current();
@@ -1197,26 +1224,32 @@ static void migration_trigger_throttle(RAMState *rs)
uint64_t bytes_dirty_period = rs->num_dirty_pages_period * TARGET_PAGE_SIZE;
uint64_t bytes_dirty_threshold = bytes_xfer_period * threshold / 100;
- /* During block migration the auto-converge logic incorrectly detects
- * that ram migration makes no progress. Avoid this by disabling the
- * throttling logic during the bulk phase of block migration. */
- if (blk_mig_bulk_active()) {
- return;
- }
+ /*
+ * The following detection logic can be refined later. For now:
+ * 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.
+ */
- if (migrate_auto_converge()) {
- /* The following detection logic can be refined later. For now:
- 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. */
+ if ((bytes_dirty_period > bytes_dirty_threshold) &&
+ (++rs->dirty_rate_high_cnt >= 2)) {
+ rs->dirty_rate_high_cnt = 0;
+ /*
+ * During block migration the auto-converge logic incorrectly detects
+ * that ram migration makes no progress. Avoid this by disabling the
+ * throttling logic during the bulk phase of block migration
+ */
+ if (blk_mig_bulk_active()) {
+ return;
+ }
- if ((bytes_dirty_period > bytes_dirty_threshold) &&
- (++rs->dirty_rate_high_cnt >= 2)) {
+ if (migrate_auto_converge()) {
trace_migration_throttle();
- rs->dirty_rate_high_cnt = 0;
mig_throttle_guest_down(bytes_dirty_period,
bytes_dirty_threshold);
+ } else if (migrate_dirty_limit()) {
+ migration_dirty_limit_guest();
}
}
}
diff --git a/migration/trace-events b/migration/trace-events
index 67b65a70ff..a689807a49 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -91,6 +91,7 @@ migration_bitmap_sync_start(void) ""
migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64
migration_bitmap_clear_dirty(char *str, uint64_t start, uint64_t size, unsigned long page) "rb %s start 0x%"PRIx64" size 0x%"PRIx64" page 0x%lx"
migration_throttle(void) ""
+migration_dirty_limit_guest(int64_t dirtyrate) "guest dirty page rate limit %" PRIi64 " MB/s"
ram_discard_range(const char *rbname, uint64_t start, size_t len) "%s: start: %" PRIx64 " %zx"
ram_load_loop(const char *rbname, uint64_t addr, int flags, void *host) "%s: addr: 0x%" PRIx64 " flags: 0x%x host: %p"
ram_load_postcopy_loop(int channel, uint64_t addr, int flags) "chan=%d addr=0x%" PRIx64 " flags=0x%x"
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index 52d1b2c6fa..ae77ebe5c5 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -438,6 +438,8 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
int64_t cpu_index,
Error **errp)
{
+ MigrationState *ms = migrate_get_current();
+
if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
return;
}
@@ -451,6 +453,15 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
return;
}
+ if (migration_is_running(ms->state) &&
+ (!qemu_thread_is_self(&ms->thread)) &&
+ migrate_dirty_limit() &&
+ dirtylimit_in_service()) {
+ error_setg(errp, "can't cancel dirty page limit while"
+ " migration is running");
+ return;
+ }
+
dirtylimit_state_lock();
if (has_cpu_index) {
@@ -486,6 +497,8 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
uint64_t dirty_rate,
Error **errp)
{
+ MigrationState *ms = migrate_get_current();
+
if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
error_setg(errp, "dirty page limit feature requires KVM with"
" accelerator property 'dirty-ring-size' set'");
@@ -502,6 +515,15 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
return;
}
+ if (migration_is_running(ms->state) &&
+ (!qemu_thread_is_self(&ms->thread)) &&
+ migrate_dirty_limit() &&
+ dirtylimit_in_service()) {
+ error_setg(errp, "can't cancel dirty page limit while"
+ " migration is running");
+ return;
+ }
+
dirtylimit_state_lock();
if (!dirtylimit_in_service()) {
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 09/10] migration: Extend query-migrate to provide dirty page limit info
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (7 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 08/10] migration: Implement dirty-limit convergence algo huangy81
@ 2023-02-16 16:18 ` huangy81
2023-02-16 16:18 ` [PATCH v4 10/10] tests: Add migration dirty-limit capability test huangy81
2023-03-01 15:53 ` [PATCH v4 00/10] migration: introduce dirtylimit capability Hyman Huang
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Extend query-migrate to provide throttle time and estimated
ring full time with dirty-limit capability enabled, through which
we can observe if dirty limit take effect during live migration.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/sysemu/dirtylimit.h | 2 ++
migration/migration-hmp-cmds.c | 10 +++++++++
migration/migration.c | 10 +++++++++
qapi/migration.json | 15 ++++++++++++-
softmmu/dirtylimit.c | 39 ++++++++++++++++++++++++++++++++++
5 files changed, 75 insertions(+), 1 deletion(-)
diff --git a/include/sysemu/dirtylimit.h b/include/sysemu/dirtylimit.h
index 8d2c1f3a6b..410a2bc0b6 100644
--- a/include/sysemu/dirtylimit.h
+++ b/include/sysemu/dirtylimit.h
@@ -34,4 +34,6 @@ void dirtylimit_set_vcpu(int cpu_index,
void dirtylimit_set_all(uint64_t quota,
bool enable);
void dirtylimit_vcpu_execute(CPUState *cpu);
+int64_t dirtylimit_throttle_time_per_round(void);
+int64_t dirtylimit_ring_full_time(void);
#endif
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index a61ec80d9d..1f090faec5 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -171,6 +171,16 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->cpu_throttle_percentage);
}
+ if (info->has_dirty_limit_throttle_time_per_round) {
+ monitor_printf(mon, "dirty-limit throttle time: %" PRIi64 " us\n",
+ info->dirty_limit_throttle_time_per_round);
+ }
+
+ if (info->has_dirty_limit_ring_full_time) {
+ monitor_printf(mon, "dirty-limit ring full time: %" PRIi64 " us\n",
+ info->dirty_limit_ring_full_time);
+ }
+
if (info->has_postcopy_blocktime) {
monitor_printf(mon, "postcopy blocktime: %u\n",
info->postcopy_blocktime);
diff --git a/migration/migration.c b/migration/migration.c
index 7ccbc07257..1f1e1f2268 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -65,6 +65,7 @@
#include "sysemu/qtest.h"
#include "ui/qemu-spice.h"
#include "sysemu/kvm.h"
+#include "sysemu/dirtylimit.h"
#define MAX_THROTTLE (128 << 20) /* Migration transfer speed throttling */
@@ -1203,6 +1204,15 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s)
info->ram->remaining = ram_bytes_remaining();
info->ram->dirty_pages_rate = ram_counters.dirty_pages_rate;
}
+
+ if (migrate_dirty_limit() && dirtylimit_in_service()) {
+ info->has_dirty_limit_throttle_time_per_round = true;
+ info->dirty_limit_throttle_time_per_round =
+ dirtylimit_throttle_time_per_round();
+
+ info->has_dirty_limit_ring_full_time = true;
+ info->dirty_limit_ring_full_time = dirtylimit_us_ring_full();
+ }
}
static void populate_disk_info(MigrationInfo *info)
diff --git a/qapi/migration.json b/qapi/migration.json
index b7a92be055..f511771101 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -242,6 +242,17 @@
# Present and non-empty when migration is blocked.
# (since 6.0)
#
+# @dirty-limit-throttle-time-per-round: Maximum throttle time (in microseconds) of virtual
+# CPUs each dirty ring full round, which shows how
+# MigrationCapability dirty-limit affects the guest
+# during live migration. (since 8.0)
+#
+# @dirty-limit-ring-full-time: Estimated average dirty ring full time (in microseconds)
+# each dirty ring full round, note that the value equals
+# dirty ring memory size divided by average dirty page rate
+# of virtual CPU, which can be used to observe the average
+# memory load of virtual CPU indirectly. (since 8.0)
+#
# Since: 0.14
##
{ 'struct': 'MigrationInfo',
@@ -259,7 +270,9 @@
'*postcopy-blocktime' : 'uint32',
'*postcopy-vcpu-blocktime': ['uint32'],
'*compression': 'CompressionStats',
- '*socket-address': ['SocketAddress'] } }
+ '*socket-address': ['SocketAddress'],
+ '*dirty-limit-throttle-time-per-round': 'int64',
+ '*dirty-limit-ring-full-time': 'int64'} }
##
# @query-migrate:
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index ae77ebe5c5..3c07844a11 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -568,6 +568,45 @@ static struct DirtyLimitInfo *dirtylimit_query_vcpu(int cpu_index)
return info;
}
+/* Return the max throttle time of each virtual CPU */
+int64_t dirtylimit_throttle_time_per_round(void)
+{
+ CPUState *cpu;
+ int64_t max = 0;
+
+ CPU_FOREACH(cpu) {
+ if (cpu->throttle_us_per_full > max) {
+ max = cpu->throttle_us_per_full;
+ }
+ }
+
+ return max;
+}
+
+/*
+ * Estimate average dirty ring full time of each virtaul CPU.
+ * Return -1 if guest doesn't dirty memory.
+ */
+int64_t dirtylimit_us_ring_full(void)
+{
+ CPUState *cpu;
+ uint64_t curr_rate = 0;
+ int nvcpus = 0;
+
+ CPU_FOREACH(cpu) {
+ if (cpu->running) {
+ nvcpus++;
+ curr_rate += vcpu_dirty_rate_get(cpu->cpu_index);
+ }
+ }
+
+ if (!curr_rate || !nvcpus) {
+ return -1;
+ }
+
+ return dirtylimit_dirty_ring_full_time(curr_rate / nvcpus);
+}
+
static struct DirtyLimitInfoList *dirtylimit_query_all(void)
{
int i, index;
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v4 10/10] tests: Add migration dirty-limit capability test
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (8 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 09/10] migration: Extend query-migrate to provide dirty page limit info huangy81
@ 2023-02-16 16:18 ` huangy81
2023-03-01 15:53 ` [PATCH v4 00/10] migration: introduce dirtylimit capability Hyman Huang
10 siblings, 0 replies; 23+ messages in thread
From: huangy81 @ 2023-02-16 16:18 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson, Hyman Huang(黄勇)
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Add migration dirty-limit capability test if kernel support
dirty ring.
Migration dirty-limit capability introduce dirty limit
capability, two parameters: x-vcpu-dirty-limit-period and
vcpu-dirty-limit are introduced to implement the live
migration with dirty limit.
The test case does the following things:
1. start src, dst vm and enable dirty-limit capability
2. start migrate and set cancel it to check if dirty limit
stop working.
3. restart dst vm
4. start migrate and enable dirty-limit capability
5. check if migration satisfy the convergence condition
during pre-switchover phase.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
tests/qtest/migration-test.c | 157 +++++++++++++++++++++++++++++++++++
1 file changed, 157 insertions(+)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 109bc8e7b1..6aad86e572 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2434,6 +2434,161 @@ static void test_vcpu_dirty_limit(void)
dirtylimit_stop_vm(vm);
}
+static void migrate_dirty_limit_wait_showup(QTestState *from,
+ const int64_t period,
+ const int64_t value)
+{
+ /* Enable dirty limit capability */
+ migrate_set_capability(from, "dirty-limit", true);
+
+ /* Set dirty limit parameters */
+ migrate_set_parameter_int(from, "x-vcpu-dirty-limit-period", period);
+ migrate_set_parameter_int(from, "vcpu-dirty-limit", value);
+
+ /* Make sure migrate can't converge */
+ migrate_ensure_non_converge(from);
+
+ /* To check limit rate after precopy */
+ migrate_set_capability(from, "pause-before-switchover", true);
+
+ /* Wait for the serial output from the source */
+ wait_for_serial("src_serial");
+}
+
+/*
+ * This test does:
+ * source target
+ * migrate_incoming
+ * migrate
+ * migrate_cancel
+ * restart target
+ * migrate
+ *
+ * And see that if dirty limit works correctly
+ */
+static void test_migrate_dirty_limit(void)
+{
+ g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+ QTestState *from, *to;
+ int64_t remaining, throttle_us_per_full;
+ /*
+ * We want the test to be stable and as fast as possible.
+ * E.g., with 1Gb/s bandwith migration may pass without dirty limit,
+ * so we need to decrease a bandwidth.
+ */
+ const int64_t dirtylimit_period = 1000, dirtylimit_value = 50;
+ const int64_t max_bandwidth = 400000000; /* ~400Mb/s */
+ const int64_t downtime_limit = 250; /* 250ms */
+ /*
+ * We migrate through unix-socket (> 500Mb/s).
+ * Thus, expected migration speed ~= bandwidth limit (< 500Mb/s).
+ * So, we can predict expected_threshold
+ */
+ const int64_t expected_threshold = max_bandwidth * downtime_limit / 1000;
+ int max_try_count = 10;
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ .use_dirty_ring = true,
+ },
+ .listen_uri = uri,
+ .connect_uri = uri,
+ };
+
+ /* Start src, dst vm */
+ if (test_migrate_start(&from, &to, args.listen_uri, &args.start)) {
+ return;
+ }
+
+ /* Prepare for dirty limit migration and wait src vm show up */
+ migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
+
+ /* Start migrate */
+ migrate_qmp(from, uri, "{}");
+
+ /* Wait for dirty limit throttle begin */
+ throttle_us_per_full = 0;
+ while (throttle_us_per_full == 0) {
+ throttle_us_per_full =
+ read_migrate_property_int(from,
+ "dirty-limit-throttle-time-per-round");
+ usleep(100);
+ g_assert_false(got_stop);
+ }
+
+ /* Now cancel migrate and wait for dirty limit throttle switch off */
+ migrate_cancel(from);
+ wait_for_migration_status(from, "cancelled", NULL);
+
+ /* Check if dirty limit throttle switched off, set timeout 1ms */
+ do {
+ throttle_us_per_full =
+ read_migrate_property_int(from,
+ "dirty-limit-throttle-time-per-round");
+ usleep(100);
+ g_assert_false(got_stop);
+ } while (throttle_us_per_full != 0 && --max_try_count);
+
+ /* Assert dirty limit is not in service */
+ g_assert_cmpint(throttle_us_per_full, ==, 0);
+
+ args = (MigrateCommon) {
+ .start = {
+ .only_target = true,
+ .use_dirty_ring = true,
+ },
+ .listen_uri = uri,
+ .connect_uri = uri,
+ };
+
+ /* Restart dst vm, src vm already show up so we needn't wait anymore */
+ if (test_migrate_start(&from, &to, args.listen_uri, &args.start)) {
+ return;
+ }
+
+ /* Start migrate */
+ migrate_qmp(from, uri, "{}");
+
+ /* Wait for dirty limit throttle begin */
+ throttle_us_per_full = 0;
+ while (throttle_us_per_full == 0) {
+ throttle_us_per_full =
+ read_migrate_property_int(from,
+ "dirty-limit-throttle-time-per-round");
+ usleep(100);
+ g_assert_false(got_stop);
+ }
+
+ /*
+ * The dirty limit rate should equals the return value of
+ * query-vcpu-dirty-limit if dirty limit cap set
+ */
+ g_assert_cmpint(dirtylimit_value, ==, get_limit_rate(from));
+
+ /* Now, we have tested if dirty limit works, let it converge */
+ migrate_set_parameter_int(from, "downtime-limit", downtime_limit);
+ migrate_set_parameter_int(from, "max-bandwidth", max_bandwidth);
+
+ /*
+ * Wait for pre-switchover status to check if migration
+ * satisfy the convergence condition
+ */
+ wait_for_migration_status(from, "pre-switchover", NULL);
+
+ remaining = read_ram_property_int(from, "remaining");
+ g_assert_cmpint(remaining, <,
+ (expected_threshold + expected_threshold / 100));
+
+ migrate_continue(from, "pre-switchover");
+
+ qtest_qmp_eventwait(to, "RESUME");
+
+ wait_for_serial("dest_serial");
+ wait_for_migration_complete(from);
+
+ test_migrate_end(from, to, true);
+}
+
static bool kvm_dirty_ring_supported(void)
{
#if defined(__linux__) && defined(HOST_X86_64)
@@ -2604,6 +2759,8 @@ int main(int argc, char **argv)
test_precopy_unix_dirty_ring);
qtest_add_func("/migration/vcpu_dirty_limit",
test_vcpu_dirty_limit);
+ qtest_add_func("/migration/dirty_limit",
+ test_migrate_dirty_limit);
}
ret = g_test_run();
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 00/10] migration: introduce dirtylimit capability
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
` (9 preceding siblings ...)
2023-02-16 16:18 ` [PATCH v4 10/10] tests: Add migration dirty-limit capability test huangy81
@ 2023-03-01 15:53 ` Hyman Huang
2023-03-24 7:27 ` Hyman Huang
10 siblings, 1 reply; 23+ messages in thread
From: Hyman Huang @ 2023-03-01 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: Markus Armbruster, Peter Xu, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Paolo Bonzini, Eric Blake,
Peter Maydell, Richard Henderson
Ping ?
在 2023/2/17 0:18, huangy81@chinatelecom.cn 写道:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>
> v4:
> 1. Polish the docs and update the release version suggested by Markus
> 2. Rename the migrate exported info "dirty-limit-throttle-time-per-round"
> to "dirty-limit-throttle-time-per-full".
>
> The following 5 commits hasn't been acked or reviewed yet:
>
> kvm: dirty-ring: Fix race with vcpu creation
> qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
> migration: Implement dirty-limit convergence algo
> migration: Extend query-migrate to provide dirty page limit info
> tests: Add migration dirty-limit capability test
>
> Ping David and Juan.
>
> Please review if you have time. Thanks.
>
> Yong
>
> v3(resend):
> - fix the syntax error of the topic.
>
> v3:
> This version make some modifications inspired by Peter and Markus
> as following:
> 1. Do the code clean up in [PATCH v2 02/11] suggested by Markus
> 2. Replace the [PATCH v2 03/11] with a much simpler patch posted by
> Peter to fix the following bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=2124756
> 3. Fix the error path of migrate_params_check in [PATCH v2 04/11]
> pointed out by Markus. Enrich the commit message to explain why
> x-vcpu-dirty-limit-period an unstable parameter.
> 4. Refactor the dirty-limit convergence algo in [PATCH v2 07/11]
> suggested by Peter:
> a. apply blk_mig_bulk_active check before enable dirty-limit
> b. drop the unhelpful check function before enable dirty-limit
> c. change the migration_cancel logic, just cancel dirty-limit
> only if dirty-limit capability turned on.
> d. abstract a code clean commit [PATCH v3 07/10] to adjust
> the check order before enable auto-converge
> 5. Change the name of observing indexes during dirty-limit live
> migration to make them more easy-understanding. Use the
> maximum throttle time of vpus as "dirty-limit-throttle-time-per-full"
> 6. Fix some grammatical and spelling errors pointed out by Markus
> and enrich the document about the dirty-limit live migration
> observing indexes "dirty-limit-ring-full-time"
> and "dirty-limit-throttle-time-per-full"
> 7. Change the default value of x-vcpu-dirty-limit-period to 1000ms,
> which is optimal value pointed out in cover letter in that
> testing environment.
> 8. Drop the 2 guestperf test commits [PATCH v2 10/11],
> [PATCH v2 11/11] and post them with a standalone series in the
> future.
>
> Thanks Peter and Markus sincerely for the passionate, efficient
> and careful comments and suggestions.
>
> Please review.
>
> Yong
>
> v2:
> This version make a little bit modifications comparing with
> version 1 as following:
> 1. fix the overflow issue reported by Peter Maydell
> 2. add parameter check for hmp "set_vcpu_dirty_limit" command
> 3. fix the racing issue between dirty ring reaper thread and
> Qemu main thread.
> 4. add migrate parameter check for x-vcpu-dirty-limit-period
> and vcpu-dirty-limit.
> 5. add the logic to forbid hmp/qmp commands set_vcpu_dirty_limit,
> cancel_vcpu_dirty_limit during dirty-limit live migration when
> implement dirty-limit convergence algo.
> 6. add capability check to ensure auto-converge and dirty-limit
> are mutually exclusive.
> 7. pre-check if kvm dirty ring size is configured before setting
> dirty-limit migrate parameter
>
> A more comprehensive test was done comparing with version 1.
>
> The following are test environment:
> -------------------------------------------------------------
> a. Host hardware info:
>
> CPU:
> Intel(R) Xeon(R) Gold 5218 CPU @ 2.30GHz
>
> CPU(s): 64
> On-line CPU(s) list: 0-63
> Thread(s) per core: 2
> Core(s) per socket: 16
> Socket(s): 2
> NUMA node(s): 2
>
> NUMA node0 CPU(s): 0-15,32-47
> NUMA node1 CPU(s): 16-31,48-63
>
> Memory:
> Hynix 503Gi
>
> Interface:
> Intel Corporation Ethernet Connection X722 for 1GbE (rev 09)
> Speed: 1000Mb/s
>
> b. Host software info:
>
> OS: ctyunos release 2
> Kernel: 4.19.90-2102.2.0.0066.ctl2.x86_64
> Libvirt baseline version: libvirt-6.9.0
> Qemu baseline version: qemu-5.0
>
> c. vm scale
> CPU: 4
> Memory: 4G
> -------------------------------------------------------------
>
> All the supplementary test data shown as follows are basing on
> above test environment.
>
> In version 1, we post a test data from unixbench as follows:
>
> $ taskset -c 8-15 ./Run -i 2 -c 8 {unixbench test item}
>
> host cpu: Intel(R) Xeon(R) Platinum 8378A
> host interface speed: 1000Mb/s
> |---------------------+--------+------------+---------------|
> | UnixBench test item | Normal | Dirtylimit | Auto-converge |
> |---------------------+--------+------------+---------------|
> | dhry2reg | 32800 | 32786 | 25292 |
> | whetstone-double | 10326 | 10315 | 9847 |
> | pipe | 15442 | 15271 | 14506 |
> | context1 | 7260 | 6235 | 4514 |
> | spawn | 3663 | 3317 | 3249 |
> | syscall | 4669 | 4667 | 3841 |
> |---------------------+--------+------------+---------------|
>
> In version 2, we post a supplementary test data that do not use
> taskset and make the scenario more general, see as follows:
>
> $ ./Run
>
> per-vcpu data:
> |---------------------+--------+------------+---------------|
> | UnixBench test item | Normal | Dirtylimit | Auto-converge |
> |---------------------+--------+------------+---------------|
> | dhry2reg | 2991 | 2902 | 1722 |
> | whetstone-double | 1018 | 1006 | 627 |
> | Execl Throughput | 955 | 320 | 660 |
> | File Copy - 1 | 2362 | 805 | 1325 |
> | File Copy - 2 | 1500 | 1406 | 643 |
> | File Copy - 3 | 4778 | 2160 | 1047 |
> | Pipe Throughput | 1181 | 1170 | 842 |
> | Context Switching | 192 | 224 | 198 |
> | Process Creation | 490 | 145 | 95 |
> | Shell Scripts - 1 | 1284 | 565 | 610 |
> | Shell Scripts - 2 | 2368 | 900 | 1040 |
> | System Call Overhead| 983 | 948 | 698 |
> | Index Score | 1263 | 815 | 600 |
> |---------------------+--------+------------+---------------|
> Note:
> File Copy - 1: File Copy 1024 bufsize 2000 maxblocks
> File Copy - 2: File Copy 256 bufsize 500 maxblocks
> File Copy - 3: File Copy 4096 bufsize 8000 maxblocks
> Shell Scripts - 1: Shell Scripts (1 concurrent)
> Shell Scripts - 2: Shell Scripts (8 concurrent)
>
> Basing on above data, we can draw a conclusion that dirty-limit
> can hugely improve the system benchmark almost in every respect,
> the "System Benchmarks Index Score" show it improve 35% performance
> comparing with auto-converge during live migration.
>
> 4-vcpu parallel data(we run a test vm with 4c4g-scale):
> |---------------------+--------+------------+---------------|
> | UnixBench test item | Normal | Dirtylimit | Auto-converge |
> |---------------------+--------+------------+---------------|
> | dhry2reg | 7975 | 7146 | 5071 |
> | whetstone-double | 3982 | 3561 | 2124 |
> | Execl Throughput | 1882 | 1205 | 768 |
> | File Copy - 1 | 1061 | 865 | 498 |
> | File Copy - 2 | 676 | 491 | 519 |
> | File Copy - 3 | 2260 | 923 | 1329 |
> | Pipe Throughput | 3026 | 3009 | 1616 |
> | Context Switching | 1219 | 1093 | 695 |
> | Process Creation | 947 | 307 | 446 |
> | Shell Scripts - 1 | 2469 | 977 | 989 |
> | Shell Scripts - 2 | 2667 | 1275 | 984 |
> | System Call Overhead| 1592 | 1459 | 692 |
> | Index Score | 1976 | 1294 | 997 |
> |---------------------+--------+------------+---------------|
>
> For the parallel data, the "System Benchmarks Index Score" show it
> also improve 29% performance.
>
> In version 1, migration total time is shown as follows:
>
> host cpu: Intel(R) Xeon(R) Platinum 8378A
> host interface speed: 1000Mb/s
> |-----------------------+----------------+-------------------|
> | dirty memory size(MB) | Dirtylimit(ms) | Auto-converge(ms) |
> |-----------------------+----------------+-------------------|
> | 60 | 2014 | 2131 |
> | 70 | 5381 | 12590 |
> | 90 | 6037 | 33545 |
> | 110 | 7660 | [*] |
> |-----------------------+----------------+-------------------|
> [*]: This case means migration is not convergent.
>
> In version 2, we post more comprehensive migration total time test data
> as follows:
>
> we update N MB on 4 cpus and sleep S us every time 1 MB data was updated.
> test twice in each condition, data is shown as follow:
>
> |-----------+--------+--------+----------------+-------------------|
> | ring size | N (MB) | S (us) | Dirtylimit(ms) | Auto-converge(ms) |
> |-----------+--------+--------+----------------+-------------------|
> | 1024 | 1024 | 1000 | 44951 | 191780 |
> | 1024 | 1024 | 1000 | 44546 | 185341 |
> | 1024 | 1024 | 500 | 46505 | 203545 |
> | 1024 | 1024 | 500 | 45469 | 909945 |
> | 1024 | 1024 | 0 | 61858 | [*] |
> | 1024 | 1024 | 0 | 57922 | [*] |
> | 1024 | 2048 | 0 | 91982 | [*] |
> | 1024 | 2048 | 0 | 90388 | [*] |
> | 2048 | 128 | 10000 | 14511 | 25971 |
> | 2048 | 128 | 10000 | 13472 | 26294 |
> | 2048 | 1024 | 10000 | 44244 | 26294 |
> | 2048 | 1024 | 10000 | 45099 | 157701 |
> | 2048 | 1024 | 500 | 51105 | [*] |
> | 2048 | 1024 | 500 | 49648 | [*] |
> | 2048 | 1024 | 0 | 229031 | [*] |
> | 2048 | 1024 | 0 | 154282 | [*] |
> |-----------+--------+--------+----------------+-------------------|
> [*]: This case means migration is not convergent.
>
> Not that the larger ring size is, the less sensitively dirty-limit responds,
> so we should choose a optimal ring size base on the test data with different
> scale vm.
>
> We also test the effect of "x-vcpu-dirty-limit-period" parameter on
> migration total time. test twice in each condition, data is shown
> as follows:
>
> |-----------+--------+--------+-------------+----------------------|
> | ring size | N (MB) | S (us) | Period (ms) | migration total time |
> |-----------+--------+--------+-------------+----------------------|
> | 2048 | 1024 | 10000 | 100 | [*] |
> | 2048 | 1024 | 10000 | 100 | [*] |
> | 2048 | 1024 | 10000 | 300 | 156795 |
> | 2048 | 1024 | 10000 | 300 | 118179 |
> | 2048 | 1024 | 10000 | 500 | 44244 |
> | 2048 | 1024 | 10000 | 500 | 45099 |
> | 2048 | 1024 | 10000 | 700 | 41871 |
> | 2048 | 1024 | 10000 | 700 | 42582 |
> | 2048 | 1024 | 10000 | 1000 | 41430 |
> | 2048 | 1024 | 10000 | 1000 | 40383 |
> | 2048 | 1024 | 10000 | 1500 | 42030 |
> | 2048 | 1024 | 10000 | 1500 | 42598 |
> | 2048 | 1024 | 10000 | 2000 | 41694 |
> | 2048 | 1024 | 10000 | 2000 | 42403 |
> | 2048 | 1024 | 10000 | 3000 | 43538 |
> | 2048 | 1024 | 10000 | 3000 | 43010 |
> |-----------+--------+--------+-------------+----------------------|
>
> It shows that x-vcpu-dirty-limit-period should be configured with 1000 ms
> in above condition.
>
> Please review, any comments and suggestions are very appreciated, thanks
>
> Yong
>
> Hyman Huang (9):
> dirtylimit: Fix overflow when computing MB
> softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit"
> qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
> qapi/migration: Introduce vcpu-dirty-limit parameters
> migration: Introduce dirty-limit capability
> migration: Refactor auto-converge capability logic
> migration: Implement dirty-limit convergence algo
> migration: Extend query-migrate to provide dirty page limit info
> tests: Add migration dirty-limit capability test
>
> Peter Xu (1):
> kvm: dirty-ring: Fix race with vcpu creation
>
> accel/kvm/kvm-all.c | 9 ++
> include/sysemu/dirtylimit.h | 2 +
> migration/migration-hmp-cmds.c | 26 ++++++
> migration/migration.c | 88 ++++++++++++++++++
> migration/migration.h | 1 +
> migration/ram.c | 63 ++++++++++---
> migration/trace-events | 1 +
> qapi/migration.json | 64 ++++++++++++--
> softmmu/dirtylimit.c | 91 ++++++++++++++++---
> tests/qtest/migration-test.c | 157 +++++++++++++++++++++++++++++++++
> 10 files changed, 470 insertions(+), 32 deletions(-)
>
--
Best regard
Hyman Huang(黄勇)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 00/10] migration: introduce dirtylimit capability
2023-03-01 15:53 ` [PATCH v4 00/10] migration: introduce dirtylimit capability Hyman Huang
@ 2023-03-24 7:27 ` Hyman Huang
0 siblings, 0 replies; 23+ messages in thread
From: Hyman Huang @ 2023-03-24 7:27 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Markus Armbruster, Peter Xu, qemu-devel, huangy81
Ping again, to make sure this series not be forgotten. :)
Please review the last three commit if you are free.
Thanks,
Yong
在 2023/3/1 23:53, Hyman Huang 写道:
> Ping ?
>
> 在 2023/2/17 0:18, huangy81@chinatelecom.cn 写道:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> v4:
>> 1. Polish the docs and update the release version suggested by Markus
>> 2. Rename the migrate exported info "dirty-limit-throttle-time-per-round"
>> to "dirty-limit-throttle-time-per-full".
>>
>> The following 5 commits hasn't been acked or reviewed yet:
>>
>> kvm: dirty-ring: Fix race with vcpu creation
>> qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
>> migration: Implement dirty-limit convergence algo
>> migration: Extend query-migrate to provide dirty page limit info
>> tests: Add migration dirty-limit capability test
>>
>> Ping David and Juan.
>>
>> Please review if you have time. Thanks.
>>
>> Yong
>>
>> v3(resend):
>> - fix the syntax error of the topic.
>>
>> v3:
>> This version make some modifications inspired by Peter and Markus
>> as following:
>> 1. Do the code clean up in [PATCH v2 02/11] suggested by Markus
>> 2. Replace the [PATCH v2 03/11] with a much simpler patch posted by
>> Peter to fix the following bug:
>> https://bugzilla.redhat.com/show_bug.cgi?id=2124756
>> 3. Fix the error path of migrate_params_check in [PATCH v2 04/11]
>> pointed out by Markus. Enrich the commit message to explain why
>> x-vcpu-dirty-limit-period an unstable parameter.
>> 4. Refactor the dirty-limit convergence algo in [PATCH v2 07/11]
>> suggested by Peter:
>> a. apply blk_mig_bulk_active check before enable dirty-limit
>> b. drop the unhelpful check function before enable dirty-limit
>> c. change the migration_cancel logic, just cancel dirty-limit
>> only if dirty-limit capability turned on.
>> d. abstract a code clean commit [PATCH v3 07/10] to adjust
>> the check order before enable auto-converge
>> 5. Change the name of observing indexes during dirty-limit live
>> migration to make them more easy-understanding. Use the
>> maximum throttle time of vpus as "dirty-limit-throttle-time-per-full"
>> 6. Fix some grammatical and spelling errors pointed out by Markus
>> and enrich the document about the dirty-limit live migration
>> observing indexes "dirty-limit-ring-full-time"
>> and "dirty-limit-throttle-time-per-full"
>> 7. Change the default value of x-vcpu-dirty-limit-period to 1000ms,
>> which is optimal value pointed out in cover letter in that
>> testing environment.
>> 8. Drop the 2 guestperf test commits [PATCH v2 10/11],
>> [PATCH v2 11/11] and post them with a standalone series in the
>> future.
>>
>> Thanks Peter and Markus sincerely for the passionate, efficient
>> and careful comments and suggestions.
>>
>> Please review.
>>
>> Yong
>>
>> v2:
>> This version make a little bit modifications comparing with
>> version 1 as following:
>> 1. fix the overflow issue reported by Peter Maydell
>> 2. add parameter check for hmp "set_vcpu_dirty_limit" command
>> 3. fix the racing issue between dirty ring reaper thread and
>> Qemu main thread.
>> 4. add migrate parameter check for x-vcpu-dirty-limit-period
>> and vcpu-dirty-limit.
>> 5. add the logic to forbid hmp/qmp commands set_vcpu_dirty_limit,
>> cancel_vcpu_dirty_limit during dirty-limit live migration when
>> implement dirty-limit convergence algo.
>> 6. add capability check to ensure auto-converge and dirty-limit
>> are mutually exclusive.
>> 7. pre-check if kvm dirty ring size is configured before setting
>> dirty-limit migrate parameter
>>
>> A more comprehensive test was done comparing with version 1.
>>
>> The following are test environment:
>> -------------------------------------------------------------
>> a. Host hardware info:
>>
>> CPU:
>> Intel(R) Xeon(R) Gold 5218 CPU @ 2.30GHz
>>
>> CPU(s): 64
>> On-line CPU(s) list: 0-63
>> Thread(s) per core: 2
>> Core(s) per socket: 16
>> Socket(s): 2
>> NUMA node(s): 2
>>
>> NUMA node0 CPU(s): 0-15,32-47
>> NUMA node1 CPU(s): 16-31,48-63
>>
>> Memory:
>> Hynix 503Gi
>>
>> Interface:
>> Intel Corporation Ethernet Connection X722 for 1GbE (rev 09)
>> Speed: 1000Mb/s
>>
>> b. Host software info:
>>
>> OS: ctyunos release 2
>> Kernel: 4.19.90-2102.2.0.0066.ctl2.x86_64
>> Libvirt baseline version: libvirt-6.9.0
>> Qemu baseline version: qemu-5.0
>>
>> c. vm scale
>> CPU: 4
>> Memory: 4G
>> -------------------------------------------------------------
>>
>> All the supplementary test data shown as follows are basing on
>> above test environment.
>>
>> In version 1, we post a test data from unixbench as follows:
>>
>> $ taskset -c 8-15 ./Run -i 2 -c 8 {unixbench test item}
>>
>> host cpu: Intel(R) Xeon(R) Platinum 8378A
>> host interface speed: 1000Mb/s
>> |---------------------+--------+------------+---------------|
>> | UnixBench test item | Normal | Dirtylimit | Auto-converge |
>> |---------------------+--------+------------+---------------|
>> | dhry2reg | 32800 | 32786 | 25292 |
>> | whetstone-double | 10326 | 10315 | 9847 |
>> | pipe | 15442 | 15271 | 14506 |
>> | context1 | 7260 | 6235 | 4514 |
>> | spawn | 3663 | 3317 | 3249 |
>> | syscall | 4669 | 4667 | 3841 |
>> |---------------------+--------+------------+---------------|
>>
>> In version 2, we post a supplementary test data that do not use
>> taskset and make the scenario more general, see as follows:
>>
>> $ ./Run
>>
>> per-vcpu data:
>> |---------------------+--------+------------+---------------|
>> | UnixBench test item | Normal | Dirtylimit | Auto-converge |
>> |---------------------+--------+------------+---------------|
>> | dhry2reg | 2991 | 2902 | 1722 |
>> | whetstone-double | 1018 | 1006 | 627 |
>> | Execl Throughput | 955 | 320 | 660 |
>> | File Copy - 1 | 2362 | 805 | 1325 |
>> | File Copy - 2 | 1500 | 1406 | 643 |
>> | File Copy - 3 | 4778 | 2160 | 1047 |
>> | Pipe Throughput | 1181 | 1170 | 842 |
>> | Context Switching | 192 | 224 | 198 |
>> | Process Creation | 490 | 145 | 95 |
>> | Shell Scripts - 1 | 1284 | 565 | 610 |
>> | Shell Scripts - 2 | 2368 | 900 | 1040 |
>> | System Call Overhead| 983 | 948 | 698 |
>> | Index Score | 1263 | 815 | 600 |
>> |---------------------+--------+------------+---------------|
>> Note:
>> File Copy - 1: File Copy 1024 bufsize 2000 maxblocks
>> File Copy - 2: File Copy 256 bufsize 500 maxblocks
>> File Copy - 3: File Copy 4096 bufsize 8000 maxblocks
>> Shell Scripts - 1: Shell Scripts (1 concurrent)
>> Shell Scripts - 2: Shell Scripts (8 concurrent)
>>
>> Basing on above data, we can draw a conclusion that dirty-limit
>> can hugely improve the system benchmark almost in every respect,
>> the "System Benchmarks Index Score" show it improve 35% performance
>> comparing with auto-converge during live migration.
>>
>> 4-vcpu parallel data(we run a test vm with 4c4g-scale):
>> |---------------------+--------+------------+---------------|
>> | UnixBench test item | Normal | Dirtylimit | Auto-converge |
>> |---------------------+--------+------------+---------------|
>> | dhry2reg | 7975 | 7146 | 5071 |
>> | whetstone-double | 3982 | 3561 | 2124 |
>> | Execl Throughput | 1882 | 1205 | 768 |
>> | File Copy - 1 | 1061 | 865 | 498 |
>> | File Copy - 2 | 676 | 491 | 519 |
>> | File Copy - 3 | 2260 | 923 | 1329 |
>> | Pipe Throughput | 3026 | 3009 | 1616 |
>> | Context Switching | 1219 | 1093 | 695 |
>> | Process Creation | 947 | 307 | 446 |
>> | Shell Scripts - 1 | 2469 | 977 | 989 |
>> | Shell Scripts - 2 | 2667 | 1275 | 984 |
>> | System Call Overhead| 1592 | 1459 | 692 |
>> | Index Score | 1976 | 1294 | 997 |
>> |---------------------+--------+------------+---------------|
>>
>> For the parallel data, the "System Benchmarks Index Score" show it
>> also improve 29% performance.
>>
>> In version 1, migration total time is shown as follows:
>>
>> host cpu: Intel(R) Xeon(R) Platinum 8378A
>> host interface speed: 1000Mb/s
>> |-----------------------+----------------+-------------------|
>> | dirty memory size(MB) | Dirtylimit(ms) | Auto-converge(ms) |
>> |-----------------------+----------------+-------------------|
>> | 60 | 2014 | 2131 |
>> | 70 | 5381 | 12590 |
>> | 90 | 6037 | 33545 |
>> | 110 | 7660 | [*] |
>> |-----------------------+----------------+-------------------|
>> [*]: This case means migration is not convergent.
>>
>> In version 2, we post more comprehensive migration total time test data
>> as follows:
>>
>> we update N MB on 4 cpus and sleep S us every time 1 MB data was updated.
>> test twice in each condition, data is shown as follow:
>>
>> |-----------+--------+--------+----------------+-------------------|
>> | ring size | N (MB) | S (us) | Dirtylimit(ms) | Auto-converge(ms) |
>> |-----------+--------+--------+----------------+-------------------|
>> | 1024 | 1024 | 1000 | 44951 | 191780 |
>> | 1024 | 1024 | 1000 | 44546 | 185341 |
>> | 1024 | 1024 | 500 | 46505 | 203545 |
>> | 1024 | 1024 | 500 | 45469 | 909945 |
>> | 1024 | 1024 | 0 | 61858 | [*] |
>> | 1024 | 1024 | 0 | 57922 | [*] |
>> | 1024 | 2048 | 0 | 91982 | [*] |
>> | 1024 | 2048 | 0 | 90388 | [*] |
>> | 2048 | 128 | 10000 | 14511 | 25971 |
>> | 2048 | 128 | 10000 | 13472 | 26294 |
>> | 2048 | 1024 | 10000 | 44244 | 26294 |
>> | 2048 | 1024 | 10000 | 45099 | 157701 |
>> | 2048 | 1024 | 500 | 51105 | [*] |
>> | 2048 | 1024 | 500 | 49648 | [*] |
>> | 2048 | 1024 | 0 | 229031 | [*] |
>> | 2048 | 1024 | 0 | 154282 | [*] |
>> |-----------+--------+--------+----------------+-------------------|
>> [*]: This case means migration is not convergent.
>>
>> Not that the larger ring size is, the less sensitively dirty-limit
>> responds,
>> so we should choose a optimal ring size base on the test data with
>> different
>> scale vm.
>>
>> We also test the effect of "x-vcpu-dirty-limit-period" parameter on
>> migration total time. test twice in each condition, data is shown
>> as follows:
>>
>> |-----------+--------+--------+-------------+----------------------|
>> | ring size | N (MB) | S (us) | Period (ms) | migration total time |
>> |-----------+--------+--------+-------------+----------------------|
>> | 2048 | 1024 | 10000 | 100 | [*] |
>> | 2048 | 1024 | 10000 | 100 | [*] |
>> | 2048 | 1024 | 10000 | 300 | 156795 |
>> | 2048 | 1024 | 10000 | 300 | 118179 |
>> | 2048 | 1024 | 10000 | 500 | 44244 |
>> | 2048 | 1024 | 10000 | 500 | 45099 |
>> | 2048 | 1024 | 10000 | 700 | 41871 |
>> | 2048 | 1024 | 10000 | 700 | 42582 |
>> | 2048 | 1024 | 10000 | 1000 | 41430 |
>> | 2048 | 1024 | 10000 | 1000 | 40383 |
>> | 2048 | 1024 | 10000 | 1500 | 42030 |
>> | 2048 | 1024 | 10000 | 1500 | 42598 |
>> | 2048 | 1024 | 10000 | 2000 | 41694 |
>> | 2048 | 1024 | 10000 | 2000 | 42403 |
>> | 2048 | 1024 | 10000 | 3000 | 43538 |
>> | 2048 | 1024 | 10000 | 3000 | 43010 |
>> |-----------+--------+--------+-------------+----------------------|
>>
>> It shows that x-vcpu-dirty-limit-period should be configured with 1000 ms
>> in above condition.
>>
>> Please review, any comments and suggestions are very appreciated, thanks
>>
>> Yong
>>
>> Hyman Huang (9):
>> dirtylimit: Fix overflow when computing MB
>> softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit"
>> qapi/migration: Introduce x-vcpu-dirty-limit-period parameter
>> qapi/migration: Introduce vcpu-dirty-limit parameters
>> migration: Introduce dirty-limit capability
>> migration: Refactor auto-converge capability logic
>> migration: Implement dirty-limit convergence algo
>> migration: Extend query-migrate to provide dirty page limit info
>> tests: Add migration dirty-limit capability test
>>
>> Peter Xu (1):
>> kvm: dirty-ring: Fix race with vcpu creation
>>
>> accel/kvm/kvm-all.c | 9 ++
>> include/sysemu/dirtylimit.h | 2 +
>> migration/migration-hmp-cmds.c | 26 ++++++
>> migration/migration.c | 88 ++++++++++++++++++
>> migration/migration.h | 1 +
>> migration/ram.c | 63 ++++++++++---
>> migration/trace-events | 1 +
>> qapi/migration.json | 64 ++++++++++++--
>> softmmu/dirtylimit.c | 91 ++++++++++++++++---
>> tests/qtest/migration-test.c | 157 +++++++++++++++++++++++++++++++++
>> 10 files changed, 470 insertions(+), 32 deletions(-)
>>
>
--
Best regard
Hyman Huang(黄勇)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 06/10] migration: Introduce dirty-limit capability
2023-02-16 16:18 ` [PATCH v4 06/10] migration: Introduce dirty-limit capability huangy81
@ 2023-03-24 12:11 ` Markus Armbruster
[not found] ` <f70dbc9b-e722-ad77-e22d-12c339f5ff4d@chinatelecom.cn>
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2023-03-24 12:11 UTC (permalink / raw)
To: huangy81
Cc: qemu-devel, Peter Xu, Dr. David Alan Gilbert, Juan Quintela,
Thomas Huth, Paolo Bonzini, Eric Blake, Peter Maydell,
Richard Henderson
huangy81@chinatelecom.cn writes:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>
> Introduce migration dirty-limit capability, which can
> be turned on before live migration and limit dirty
> page rate durty live migration.
>
> Introduce migrate_dirty_limit function to help check
> if dirty-limit capability enabled during live migration.
>
> Meanwhile, refactor vcpu_dirty_rate_stat_collect
> so that period can be configured instead of hardcoded.
>
> dirty-limit capability is kind of like auto-converge
> but using dirty limit instead of traditional cpu-throttle
> to throttle guest down. To enable this feature, turn on
> the dirty-limit capability before live migration using
> migrate-set-capabilities, and set the parameters
> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
> to speed up convergence.
>
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> Acked-by: Peter Xu <peterx@redhat.com>
[...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index d33cc2d582..b7a92be055 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -477,6 +477,8 @@
> # will be handled faster. This is a performance feature and
> # should not affect the correctness of postcopy migration.
> # (since 7.1)
> +# @dirty-limit: Use dirty-limit to throttle down guest if enabled.
> +# (since 8.0)
Feels too terse. What exactly is used and how? It's not the capability
itself (although the text sure sounds like it). I guess it's the thing
you set with command set-vcpu-dirty-limit.
Is that used only when the capability is set?
> #
> # Features:
> # @unstable: Members @x-colo and @x-ignore-shared are experimental.
> @@ -492,7 +494,7 @@
> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
> 'validate-uuid', 'background-snapshot',
> - 'zero-copy-send', 'postcopy-preempt'] }
> + 'zero-copy-send', 'postcopy-preempt', 'dirty-limit'] }
>
> ##
> # @MigrationCapabilityStatus:
[...]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 06/10] migration: Introduce dirty-limit capability
[not found] ` <f70dbc9b-e722-ad77-e22d-12c339f5ff4d@chinatelecom.cn>
@ 2023-03-24 14:32 ` Markus Armbruster
2023-03-26 7:29 ` Hyman Huang
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2023-03-24 14:32 UTC (permalink / raw)
To: Hyman Huang
Cc: qemu-devel, Peter Xu, Dr. David Alan Gilbert, Juan Quintela,
Thomas Huth, Paolo Bonzini, Eric Blake, Peter Maydell,
Richard Henderson
Hyman Huang <huangy81@chinatelecom.cn> writes:
> 在 2023/3/24 20:11, Markus Armbruster 写道:
>> huangy81@chinatelecom.cn writes:
>>
>>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>
>>> Introduce migration dirty-limit capability, which can
>>> be turned on before live migration and limit dirty
>>> page rate durty live migration.
>>>
>>> Introduce migrate_dirty_limit function to help check
>>> if dirty-limit capability enabled during live migration.
>>>
>>> Meanwhile, refactor vcpu_dirty_rate_stat_collect
>>> so that period can be configured instead of hardcoded.
>>>
>>> dirty-limit capability is kind of like auto-converge
>>> but using dirty limit instead of traditional cpu-throttle
>>> to throttle guest down. To enable this feature, turn on
>>> the dirty-limit capability before live migration using
>>> migrate-set-capabilities, and set the parameters
>>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
>>> to speed up convergence.
>>>
>>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>> Acked-by: Peter Xu <peterx@redhat.com>
>> [...]
>>
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index d33cc2d582..b7a92be055 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -477,6 +477,8 @@
>>> # will be handled faster. This is a performance feature and
>>> # should not affect the correctness of postcopy migration.
>>> # (since 7.1)
>>> +# @dirty-limit: Use dirty-limit to throttle down guest if enabled.
>>> +# (since 8.0)
>>
>> Feels too terse. What exactly is used and how? It's not the capability
>> itself (although the text sure sounds like it). I guess it's the thing
>> you set with command set-vcpu-dirty-limit.
>>
>> Is that used only when the capability is set?
>
> Yes, live migration set "dirty-limit" value when that capability is set,
> the comment changes to "Apply the algorithm of dirty page rate limit to throttle down guest if capability is set, rather than auto-converge".
>
> Please continue to polish the doc if needed. Thanks.
Let's see whether I understand.
Throttling happens only during migration.
There are two throttling algorithms: "auto-converge" (default) and
"dirty page rate limit".
The latter can be tuned with set-vcpu-dirty-limit.
Correct?
What happens when migration capability dirty-limit is enabled, but the
user hasn't set a limit with set-vcpu-dirty-limit, or canceled it with
cancel-vcpu-dirty-limit?
>>> #
>>> # Features:
>>> # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>>> @@ -492,7 +494,7 @@
>>> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
>>> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>>> 'validate-uuid', 'background-snapshot',
>>> - 'zero-copy-send', 'postcopy-preempt'] }
>>> + 'zero-copy-send', 'postcopy-preempt', 'dirty-limit'] }
>>> ##
>>> # @MigrationCapabilityStatus:
>> [...]
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 06/10] migration: Introduce dirty-limit capability
2023-03-24 14:32 ` Markus Armbruster
@ 2023-03-26 7:29 ` Hyman Huang
2023-03-27 6:41 ` Markus Armbruster
0 siblings, 1 reply; 23+ messages in thread
From: Hyman Huang @ 2023-03-26 7:29 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Xu, Dr. David Alan Gilbert, Juan Quintela,
Thomas Huth, Paolo Bonzini, Eric Blake, Peter Maydell,
Richard Henderson
在 2023/3/24 22:32, Markus Armbruster 写道:
> Hyman Huang <huangy81@chinatelecom.cn> writes:
>
>> 在 2023/3/24 20:11, Markus Armbruster 写道:
>>> huangy81@chinatelecom.cn writes:
>>>
>>>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>>
>>>> Introduce migration dirty-limit capability, which can
>>>> be turned on before live migration and limit dirty
>>>> page rate durty live migration.
>>>>
>>>> Introduce migrate_dirty_limit function to help check
>>>> if dirty-limit capability enabled during live migration.
>>>>
>>>> Meanwhile, refactor vcpu_dirty_rate_stat_collect
>>>> so that period can be configured instead of hardcoded.
>>>>
>>>> dirty-limit capability is kind of like auto-converge
>>>> but using dirty limit instead of traditional cpu-throttle
>>>> to throttle guest down. To enable this feature, turn on
>>>> the dirty-limit capability before live migration using
>>>> migrate-set-capabilities, and set the parameters
>>>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
>>>> to speed up convergence.
>>>>
>>>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>> Acked-by: Peter Xu <peterx@redhat.com>
>>> [...]
>>>
>>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>>> index d33cc2d582..b7a92be055 100644
>>>> --- a/qapi/migration.json
>>>> +++ b/qapi/migration.json
>>>> @@ -477,6 +477,8 @@
>>>> # will be handled faster. This is a performance feature and
>>>> # should not affect the correctness of postcopy migration.
>>>> # (since 7.1)
>>>> +# @dirty-limit: Use dirty-limit to throttle down guest if enabled.
>>>> +# (since 8.0)
>>>
>>> Feels too terse. What exactly is used and how? It's not the capability
>>> itself (although the text sure sounds like it). I guess it's the thing
>>> you set with command set-vcpu-dirty-limit.
>>>
>>> Is that used only when the capability is set?
>>
>> Yes, live migration set "dirty-limit" value when that capability is set,
>> the comment changes to "Apply the algorithm of dirty page rate limit to throttle down guest if capability is set, rather than auto-converge".
>>
>> Please continue to polish the doc if needed. Thanks.
>
> Let's see whether I understand.
>
> Throttling happens only during migration.
>
> There are two throttling algorithms: "auto-converge" (default) and
> "dirty page rate limit".
>
> The latter can be tuned with set-vcpu-dirty-limit.
>
> Correct?
Yes
>
> What happens when migration capability dirty-limit is enabled, but the
> user hasn't set a limit with set-vcpu-dirty-limit, or canceled it with
> cancel-vcpu-dirty-limit?
dirty-limit capability use the default value if user hasn't set. In the
path of cancel-vcpu-dirty-limit, canceling should be check and not be
allowed if migration is in process.
see the following code in commit:
[PATCH v4 08/10] migration: Implement dirty-limit convergence algo
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -438,6 +438,8 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
int64_t cpu_index,
Error **errp)
{
+ MigrationState *ms = migrate_get_current();
+
if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
return;
}
@@ -451,6 +453,15 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
return;
}
+ if (migration_is_running(ms->state) &&
+ (!qemu_thread_is_self(&ms->thread)) &&
+ migrate_dirty_limit() &&
+ dirtylimit_in_service()) {
+ error_setg(errp, "can't cancel dirty page limit while"
+ " migration is running");
+ return;
+ }
+
dirtylimit_state_lock();
if (has_cpu_index) {
@@ -486,6 +497,8 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
uint64_t dirty_rate,
Error **errp)
{
+ MigrationState *ms = migrate_get_current();
+
if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
error_setg(errp, "dirty page limit feature requires KVM with"
" accelerator property 'dirty-ring-size' set'");
@@ -502,6 +515,15 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
return;
}
+ if (migration_is_running(ms->state) &&
+ (!qemu_thread_is_self(&ms->thread)) &&
+ migrate_dirty_limit() &&
+ dirtylimit_in_service()) {
+ error_setg(errp, "can't cancel dirty page limit while"
+ " migration is running");
+ return;
+ }
+
dirtylimit_state_lock();
if (!dirtylimit_in_service()) {
--
>
>>>> #
>>>> # Features:
>>>> # @unstable: Members @x-colo and @x-ignore-shared are experimental.
>>>> @@ -492,7 +494,7 @@
>>>> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
>>>> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
>>>> 'validate-uuid', 'background-snapshot',
>>>> - 'zero-copy-send', 'postcopy-preempt'] }
>>>> + 'zero-copy-send', 'postcopy-preempt', 'dirty-limit'] }
>>>> ##
>>>> # @MigrationCapabilityStatus:
>>> [...]
>>>
>
--
Best regard
Hyman Huang(黄勇)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 06/10] migration: Introduce dirty-limit capability
2023-03-26 7:29 ` Hyman Huang
@ 2023-03-27 6:41 ` Markus Armbruster
2023-03-28 5:28 ` Hyman Huang
0 siblings, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2023-03-27 6:41 UTC (permalink / raw)
To: Hyman Huang
Cc: qemu-devel, Peter Xu, Dr. David Alan Gilbert, Juan Quintela,
Thomas Huth, Paolo Bonzini, Eric Blake, Peter Maydell,
Richard Henderson
Hyman Huang <huangy81@chinatelecom.cn> writes:
> 在 2023/3/24 22:32, Markus Armbruster 写道:
>> Hyman Huang <huangy81@chinatelecom.cn> writes:
>>
>>> 在 2023/3/24 20:11, Markus Armbruster 写道:
>>>> huangy81@chinatelecom.cn writes:
>>>>
>>>>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>>>
>>>>> Introduce migration dirty-limit capability, which can
>>>>> be turned on before live migration and limit dirty
>>>>> page rate durty live migration.
>>>>>
>>>>> Introduce migrate_dirty_limit function to help check
>>>>> if dirty-limit capability enabled during live migration.
>>>>>
>>>>> Meanwhile, refactor vcpu_dirty_rate_stat_collect
>>>>> so that period can be configured instead of hardcoded.
>>>>>
>>>>> dirty-limit capability is kind of like auto-converge
>>>>> but using dirty limit instead of traditional cpu-throttle
>>>>> to throttle guest down. To enable this feature, turn on
>>>>> the dirty-limit capability before live migration using
>>>>> migrate-set-capabilities, and set the parameters
>>>>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
>>>>> to speed up convergence.
>>>>>
>>>>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>>> Acked-by: Peter Xu <peterx@redhat.com>
>>>> [...]
>>>>
>>>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>>>> index d33cc2d582..b7a92be055 100644
>>>>> --- a/qapi/migration.json
>>>>> +++ b/qapi/migration.json
>>>>> @@ -477,6 +477,8 @@
>>>>> # will be handled faster. This is a performance feature and
>>>>> # should not affect the correctness of postcopy migration.
>>>>> # (since 7.1)
>>>>> +# @dirty-limit: Use dirty-limit to throttle down guest if enabled.
>>>>> +# (since 8.0)
>>>>
>>>> Feels too terse. What exactly is used and how? It's not the capability
>>>> itself (although the text sure sounds like it). I guess it's the thing
>>>> you set with command set-vcpu-dirty-limit.
>>>>
>>>> Is that used only when the capability is set?
>>>
>>> Yes, live migration set "dirty-limit" value when that capability is set,
>>> the comment changes to "Apply the algorithm of dirty page rate limit to throttle down guest if capability is set, rather than auto-converge".
>>>
>>> Please continue to polish the doc if needed. Thanks.
>>
>> Let's see whether I understand.
>>
>> Throttling happens only during migration.
>>
>> There are two throttling algorithms: "auto-converge" (default) and
>> "dirty page rate limit".
>>
>> The latter can be tuned with set-vcpu-dirty-limit.
>> Correct?
>
> Yes
>
>> What happens when migration capability dirty-limit is enabled, but the
>> user hasn't set a limit with set-vcpu-dirty-limit, or canceled it with
>> cancel-vcpu-dirty-limit?
>
> dirty-limit capability use the default value if user hasn't set.
What is the default value? I can't find it in the doc comments.
> In the path of cancel-vcpu-dirty-limit, canceling should be check and not be allowed if migration is in process.
Can you change the dirty limit with set-vcpu-dirty-limit while migration
is in progress? Let's see...
Has the dirty limit any effect while migration is not in progress?
> see the following code in commit:
> [PATCH v4 08/10] migration: Implement dirty-limit convergence algo
>
> --- a/softmmu/dirtylimit.c
> +++ b/softmmu/dirtylimit.c
> @@ -438,6 +438,8 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
> int64_t cpu_index,
> Error **errp)
> {
> + MigrationState *ms = migrate_get_current();
> +
> if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
> return;
> }
> @@ -451,6 +453,15 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
> return;
> }
>
> + if (migration_is_running(ms->state) &&
> + (!qemu_thread_is_self(&ms->thread)) &&
> + migrate_dirty_limit() &&
> + dirtylimit_in_service()) {
> + error_setg(errp, "can't cancel dirty page limit while"
> + " migration is running");
> + return;
> + }
We can get here even when migration_is_running() is true. Seems to
contradict your claim "no cancel while migration is in progress". Am I
confused?
Please drop the superfluous parenthesis around !qemu_thread_is_self().
> +
> dirtylimit_state_lock();
>
> if (has_cpu_index) {
> @@ -486,6 +497,8 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
> uint64_t dirty_rate,
> Error **errp)
> {
> + MigrationState *ms = migrate_get_current();
> +
> if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
> error_setg(errp, "dirty page limit feature requires KVM with"
> " accelerator property 'dirty-ring-size' set'");
> @@ -502,6 +515,15 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
> return;
> }
>
> + if (migration_is_running(ms->state) &&
> + (!qemu_thread_is_self(&ms->thread)) &&
> + migrate_dirty_limit() &&
> + dirtylimit_in_service()) {
> + error_setg(errp, "can't cancel dirty page limit while"
> + " migration is running");
Same condition, i.e. we dirty limit change is possible exactly when
cancel is. Correct?
> + return;
> + }
> +
> dirtylimit_state_lock();
>
> if (!dirtylimit_in_service()) {
Maybe it's just me still not understanding things, but the entire
interface feels overly complicated.
Here's my current mental model of what you're trying to provide.
There are two throttling algorithms: "auto-converge" (default) and
"dirty page rate limit". The user can select one.
The latter works with a user-configurable dirty limit.
Changing these configuration bits is only possible in certain states.
Which ones is not clear to me, yet.
Is this accurate and complete?
Are commands set-vcpu-dirty-limit, cancel-vcpu-dirty-limit,
query-vcpu-dirty-limit useful without this series?
If not, then committing them as stable interfaces was clearly premature.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 06/10] migration: Introduce dirty-limit capability
2023-03-27 6:41 ` Markus Armbruster
@ 2023-03-28 5:28 ` Hyman Huang
0 siblings, 0 replies; 23+ messages in thread
From: Hyman Huang @ 2023-03-28 5:28 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Peter Xu, Dr. David Alan Gilbert, Juan Quintela,
Thomas Huth, Paolo Bonzini, Eric Blake, Peter Maydell,
Richard Henderson
在 2023/3/27 14:41, Markus Armbruster 写道:
> Hyman Huang <huangy81@chinatelecom.cn> writes:
>
>> 在 2023/3/24 22:32, Markus Armbruster 写道:
>>> Hyman Huang <huangy81@chinatelecom.cn> writes:
>>>
>>>> 在 2023/3/24 20:11, Markus Armbruster 写道:
>>>>> huangy81@chinatelecom.cn writes:
>>>>>
>>>>>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>>>>
>>>>>> Introduce migration dirty-limit capability, which can
>>>>>> be turned on before live migration and limit dirty
>>>>>> page rate durty live migration.
>>>>>>
>>>>>> Introduce migrate_dirty_limit function to help check
>>>>>> if dirty-limit capability enabled during live migration.
>>>>>>
>>>>>> Meanwhile, refactor vcpu_dirty_rate_stat_collect
>>>>>> so that period can be configured instead of hardcoded.
>>>>>>
>>>>>> dirty-limit capability is kind of like auto-converge
>>>>>> but using dirty limit instead of traditional cpu-throttle
>>>>>> to throttle guest down. To enable this feature, turn on
>>>>>> the dirty-limit capability before live migration using
>>>>>> migrate-set-capabilities, and set the parameters
>>>>>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
>>>>>> to speed up convergence.
>>>>>>
>>>>>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>>>> Acked-by: Peter Xu <peterx@redhat.com>
>>>>> [...]
>>>>>
>>>>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>>>>> index d33cc2d582..b7a92be055 100644
>>>>>> --- a/qapi/migration.json
>>>>>> +++ b/qapi/migration.json
>>>>>> @@ -477,6 +477,8 @@
>>>>>> # will be handled faster. This is a performance feature and
>>>>>> # should not affect the correctness of postcopy migration.
>>>>>> # (since 7.1)
>>>>>> +# @dirty-limit: Use dirty-limit to throttle down guest if enabled.
>>>>>> +# (since 8.0)
>>>>>
>>>>> Feels too terse. What exactly is used and how? It's not the capability
>>>>> itself (although the text sure sounds like it). I guess it's the thing
>>>>> you set with command set-vcpu-dirty-limit.
>>>>>
>>>>> Is that used only when the capability is set?
>>>>
>>>> Yes, live migration set "dirty-limit" value when that capability is set,
>>>> the comment changes to "Apply the algorithm of dirty page rate limit to throttle down guest if capability is set, rather than auto-converge".
>>>>
>>>> Please continue to polish the doc if needed. Thanks.
>>>
>>> Let's see whether I understand.
>>>
>>> Throttling happens only during migration.
>>>
>>> There are two throttling algorithms: "auto-converge" (default) and
>>> "dirty page rate limit".
>>>
>>> The latter can be tuned with set-vcpu-dirty-limit.
>>> Correct?
>>
>> Yes
>>
>>> What happens when migration capability dirty-limit is enabled, but the
>>> user hasn't set a limit with set-vcpu-dirty-limit, or canceled it with
>>> cancel-vcpu-dirty-limit?
>>
>> dirty-limit capability use the default value if user hasn't set.
>
> What is the default value? I can't find it in the doc comments.
The default value is 1MB/s， i'll add it to the doc comments.
>
>> In the path of cancel-vcpu-dirty-limit, canceling should be check and not be allowed if migration is in process.
>
> Can you change the dirty limit with set-vcpu-dirty-limit while migration
> is in progress? Let's see...
No, this is not allowed.
>
> Has the dirty limit any effect while migration is not in progress?
Like the auto-converge capability, dirty-limit capability has no effect
if migration is not in progress.
>
>> see the following code in commit:
>> [PATCH v4 08/10] migration: Implement dirty-limit convergence algo
>>
>> --- a/softmmu/dirtylimit.c
>> +++ b/softmmu/dirtylimit.c
>> @@ -438,6 +438,8 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
>> int64_t cpu_index,
>> Error **errp)
>> {
>> + MigrationState *ms = migrate_get_current();
>> +
>> if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
>> return;
>> }
>> @@ -451,6 +453,15 @@ void qmp_cancel_vcpu_dirty_limit(bool has_cpu_index,
>> return;
>> }
>>
>> + if (migration_is_running(ms->state) &&
>> + (!qemu_thread_is_self(&ms->thread)) &&
>> + migrate_dirty_limit() &&
>> + dirtylimit_in_service()) {
>> + error_setg(errp, "can't cancel dirty page limit while"
>> + " migration is running");
>> + return;
>> + }
>
> We can get here even when migration_is_running() is true. Seems to
> contradict your claim "no cancel while migration is in progress". Am I
> confused?
>
> Please drop the superfluous parenthesis around !qemu_thread_is_self().
>
>> +
>> dirtylimit_state_lock();
>>
>> if (has_cpu_index) {
>> @@ -486,6 +497,8 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
>> uint64_t dirty_rate,
>> Error **errp)
>> {
>> + MigrationState *ms = migrate_get_current();
>> +
>> if (!kvm_enabled() || !kvm_dirty_ring_enabled()) {
>> error_setg(errp, "dirty page limit feature requires KVM with"
>> " accelerator property 'dirty-ring-size' set'");
>> @@ -502,6 +515,15 @@ void qmp_set_vcpu_dirty_limit(bool has_cpu_index,
>> return;
>> }
>>
>> + if (migration_is_running(ms->state) &&
>> + (!qemu_thread_is_self(&ms->thread)) &&
>> + migrate_dirty_limit() &&
>> + dirtylimit_in_service()) {
>> + error_setg(errp, "can't cancel dirty page limit while"
>> + " migration is running");
>
> Same condition, i.e. we dirty limit change is possible exactly when
> cancel is. Correct?
>
>> + return;
>> + }
>> +
>> dirtylimit_state_lock();
>>
>> if (!dirtylimit_in_service()) {
>
> Maybe it's just me still not understanding things, but the entire
> interface feels overly complicated.
>
> Here's my current mental model of what you're trying to provide.
>
> There are two throttling algorithms: "auto-converge" (default) and
> "dirty page rate limit". The user can select one.
>
> The latter works with a user-configurable dirty limit.
>
Yes
> Changing these configuration bits is only possible in certain states.
> Which ones is not clear to me, yet.
Ok, i'll add doc comments to explain under what condition the
configuration can be changed.
>
> Is this accurate and complete?
>
> Are commands set-vcpu-dirty-limit, cancel-vcpu-dirty-limit,
> query-vcpu-dirty-limit useful without this series?
Yes, the two are independent of each other.
>
> If not, then committing them as stable interfaces was clearly premature.
>
--
Best regard
Hyman Huang(黄勇)
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation
2023-02-16 16:18 ` [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation huangy81
@ 2023-04-04 13:32 ` Paolo Bonzini
2023-04-04 14:10 ` Peter Xu
0 siblings, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2023-04-04 13:32 UTC (permalink / raw)
To: huangy81, qemu-devel, Peter Xu
Cc: Markus Armbruster, Dr. David Alan Gilbert, Juan Quintela,
Thomas Huth, Eric Blake, Peter Maydell, Richard Henderson
On 2/16/23 17:18, huangy81@chinatelecom.cn wrote:
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 9b26582655..47483cdfa0 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -685,6 +685,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu)
> uint32_t ring_size = s->kvm_dirty_ring_size;
> uint32_t count = 0, fetch = cpu->kvm_fetch_index;
>
> + /*
> + * It's possible that we race with vcpu creation code where the vcpu is
> + * put onto the vcpus list but not yet initialized the dirty ring
> + * structures. If so, skip it.
> + */
> + if (!cpu->created) {
> + return 0;
> + }
> +
Is there a lock that protects cpu->created?
If you don't want to use a lock you need to use qatomic_load_acquire
together with
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index fed20ffb5dd2..15b64e7f4592 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -525,7 +525,7 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms)
/* signal CPU creation */
void cpu_thread_signal_created(CPUState *cpu)
{
- cpu->created = true;
+ qatomic_store_release(&cpu->created, true);
qemu_cond_signal(&qemu_cpu_cond);
}
Paolo
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation
2023-04-04 13:32 ` Paolo Bonzini
@ 2023-04-04 14:10 ` Peter Xu
2023-04-04 16:08 ` Paolo Bonzini
0 siblings, 1 reply; 23+ messages in thread
From: Peter Xu @ 2023-04-04 14:10 UTC (permalink / raw)
To: Paolo Bonzini
Cc: huangy81, qemu-devel, Markus Armbruster, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Eric Blake, Peter Maydell,
Richard Henderson
Hi, Paolo!
On Tue, Apr 04, 2023 at 03:32:38PM +0200, Paolo Bonzini wrote:
> On 2/16/23 17:18, huangy81@chinatelecom.cn wrote:
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index 9b26582655..47483cdfa0 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -685,6 +685,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, CPUState *cpu)
> > uint32_t ring_size = s->kvm_dirty_ring_size;
> > uint32_t count = 0, fetch = cpu->kvm_fetch_index;
> > + /*
> > + * It's possible that we race with vcpu creation code where the vcpu is
> > + * put onto the vcpus list but not yet initialized the dirty ring
> > + * structures. If so, skip it.
> > + */
> > + if (!cpu->created) {
> > + return 0;
> > + }
> > +
>
> Is there a lock that protects cpu->created?
>
> If you don't want to use a lock you need to use qatomic_load_acquire
> together with
>
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index fed20ffb5dd2..15b64e7f4592 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -525,7 +525,7 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms)
> /* signal CPU creation */
> void cpu_thread_signal_created(CPUState *cpu)
> {
> - cpu->created = true;
> + qatomic_store_release(&cpu->created, true);
> qemu_cond_signal(&qemu_cpu_cond);
> }
Makes sense.
When looking at such a possible race, I also found that when destroying the
vcpu we may have another relevant issue, where we flip "vcpu->created"
after destroying the vcpu. IIUC it means the same issue can occur when
vcpu unplugged?
Meanwhile I think the memory ordering trick won't play there, because
firstly to do that we'll need to update created==false:
- kvm_destroy_vcpu(cpu);
cpu_thread_signal_destroyed(cpu);
+ kvm_destroy_vcpu(cpu);
And even if we order the operations we still cannot assume the data is safe
to access even if created==true.
Maybe we'd better need (unfortunately) a per-vcpu mutex to protect both
cases?
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation
2023-04-04 14:10 ` Peter Xu
@ 2023-04-04 16:08 ` Paolo Bonzini
2023-04-04 16:36 ` Peter Xu
0 siblings, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2023-04-04 16:08 UTC (permalink / raw)
To: Peter Xu
Cc: Hyman, qemu-devel, Markus Armbruster, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Eric Blake, Peter Maydell,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]
Il mar 4 apr 2023, 16:11 Peter Xu <peterx@redhat.com> ha scritto:
> Hi, Paolo!
>
> On Tue, Apr 04, 2023 at 03:32:38PM +0200, Paolo Bonzini wrote:
> > On 2/16/23 17:18, huangy81@chinatelecom.cn wrote:
> > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > > index 9b26582655..47483cdfa0 100644
> > > --- a/accel/kvm/kvm-all.c
> > > +++ b/accel/kvm/kvm-all.c
> > > @@ -685,6 +685,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState
> *s, CPUState *cpu)
> > > uint32_t ring_size = s->kvm_dirty_ring_size;
> > > uint32_t count = 0, fetch = cpu->kvm_fetch_index;
> > > + /*
> > > + * It's possible that we race with vcpu creation code where the
> vcpu is
> > > + * put onto the vcpus list but not yet initialized the dirty ring
> > > + * structures. If so, skip it.
> > > + */
> > > + if (!cpu->created) {
> > > + return 0;
> > > + }
> > > +
> >
> > Is there a lock that protects cpu->created?
> >
> > If you don't want to use a lock you need to use qatomic_load_acquire
> > together with
> >
> > diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> > index fed20ffb5dd2..15b64e7f4592 100644
> > --- a/softmmu/cpus.c
> > +++ b/softmmu/cpus.c
> > @@ -525,7 +525,7 @@ void qemu_cond_timedwait_iothread(QemuCond *cond,
> int ms)
> > /* signal CPU creation */
> > void cpu_thread_signal_created(CPUState *cpu)
> > {
> > - cpu->created = true;
> > + qatomic_store_release(&cpu->created, true);
> > qemu_cond_signal(&qemu_cpu_cond);
> > }
>
> Makes sense.
>
> When looking at such a possible race, I also found that when destroying the
> vcpu we may have another relevant issue, where we flip "vcpu->created"
> after destroying the vcpu. IIUC it means the same issue can occur when
> vcpu unplugged?
>
> Meanwhile I think the memory ordering trick won't play there, because
> firstly to do that we'll need to update created==false:
>
> - kvm_destroy_vcpu(cpu);
> cpu_thread_signal_destroyed(cpu);
> + kvm_destroy_vcpu(cpu);
>
> And even if we order the operations we still cannot assume the data is safe
> to access even if created==true.
>
Yes, this would need some kind of synchronize_rcu() before clearing
created, and rcu_read_lock() when reading the dirty ring.
(Note that synchronize_rcu can only be used outside BQL. The alternative
would be to defer what's after created=false using call_rcu().
Maybe we'd better need (unfortunately) a per-vcpu mutex to protect both
> cases?
If RCU can work it's obviously better, but if not then yes. It's per-CPU so
it's only about the complexity, not the overhead.
Paolo
>
[-- Attachment #2: Type: text/html, Size: 3958 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation
2023-04-04 16:08 ` Paolo Bonzini
@ 2023-04-04 16:36 ` Peter Xu
2023-04-04 16:45 ` Paolo Bonzini
0 siblings, 1 reply; 23+ messages in thread
From: Peter Xu @ 2023-04-04 16:36 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Hyman, qemu-devel, Markus Armbruster, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Eric Blake, Peter Maydell,
Richard Henderson
On Tue, Apr 04, 2023 at 06:08:41PM +0200, Paolo Bonzini wrote:
> Il mar 4 apr 2023, 16:11 Peter Xu <peterx@redhat.com> ha scritto:
>
> > Hi, Paolo!
> >
> > On Tue, Apr 04, 2023 at 03:32:38PM +0200, Paolo Bonzini wrote:
> > > On 2/16/23 17:18, huangy81@chinatelecom.cn wrote:
> > > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > > > index 9b26582655..47483cdfa0 100644
> > > > --- a/accel/kvm/kvm-all.c
> > > > +++ b/accel/kvm/kvm-all.c
> > > > @@ -685,6 +685,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState
> > *s, CPUState *cpu)
> > > > uint32_t ring_size = s->kvm_dirty_ring_size;
> > > > uint32_t count = 0, fetch = cpu->kvm_fetch_index;
> > > > + /*
> > > > + * It's possible that we race with vcpu creation code where the
> > vcpu is
> > > > + * put onto the vcpus list but not yet initialized the dirty ring
> > > > + * structures. If so, skip it.
> > > > + */
> > > > + if (!cpu->created) {
> > > > + return 0;
> > > > + }
> > > > +
> > >
> > > Is there a lock that protects cpu->created?
> > >
> > > If you don't want to use a lock you need to use qatomic_load_acquire
> > > together with
> > >
> > > diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> > > index fed20ffb5dd2..15b64e7f4592 100644
> > > --- a/softmmu/cpus.c
> > > +++ b/softmmu/cpus.c
> > > @@ -525,7 +525,7 @@ void qemu_cond_timedwait_iothread(QemuCond *cond,
> > int ms)
> > > /* signal CPU creation */
> > > void cpu_thread_signal_created(CPUState *cpu)
> > > {
> > > - cpu->created = true;
> > > + qatomic_store_release(&cpu->created, true);
> > > qemu_cond_signal(&qemu_cpu_cond);
> > > }
> >
> > Makes sense.
> >
> > When looking at such a possible race, I also found that when destroying the
> > vcpu we may have another relevant issue, where we flip "vcpu->created"
> > after destroying the vcpu. IIUC it means the same issue can occur when
> > vcpu unplugged?
> >
> > Meanwhile I think the memory ordering trick won't play there, because
> > firstly to do that we'll need to update created==false:
> >
> > - kvm_destroy_vcpu(cpu);
> > cpu_thread_signal_destroyed(cpu);
> > + kvm_destroy_vcpu(cpu);
> >
> > And even if we order the operations we still cannot assume the data is safe
> > to access even if created==true.
> >
>
> Yes, this would need some kind of synchronize_rcu() before clearing
> created, and rcu_read_lock() when reading the dirty ring.
>
> (Note that synchronize_rcu can only be used outside BQL. The alternative
> would be to defer what's after created=false using call_rcu().
>
> Maybe we'd better need (unfortunately) a per-vcpu mutex to protect both
> > cases?
>
>
> If RCU can work it's obviously better, but if not then yes. It's per-CPU so
> it's only about the complexity, not the overhead.
Oh.. I just noticed that both vcpu creation and destruction will require
BQL, while right now dirty ring reaping also requires BQL (taken at all
callers of kvm_dirty_ring_reap()).. so I assume even the current patch will
be race-free already?
I'm not sure whether it's ideal, though, I think having BQL at least makes
sure there's no concurrent memory updates so the slot IDs will be solid
during the dirty ring reaping, but I can't remember the details. However
that seems to be a separate topic to be discussed..
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation
2023-04-04 16:36 ` Peter Xu
@ 2023-04-04 16:45 ` Paolo Bonzini
0 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2023-04-04 16:45 UTC (permalink / raw)
To: Peter Xu
Cc: Hyman, qemu-devel, Markus Armbruster, Dr. David Alan Gilbert,
Juan Quintela, Thomas Huth, Eric Blake, Peter Maydell,
Richard Henderson
On 4/4/23 18:36, Peter Xu wrote:
> Oh.. I just noticed that both vcpu creation and destruction will require
> BQL, while right now dirty ring reaping also requires BQL (taken at all
> callers of kvm_dirty_ring_reap()).. so I assume even the current patch will
> be race-free already?
Oh, indeed! Queued then, thanks.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2023-04-04 16:46 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-16 16:18 [PATCH v4 00/10] migration: introduce dirtylimit capability huangy81
2023-02-16 16:18 ` [PATCH v4 01/10] dirtylimit: Fix overflow when computing MB huangy81
2023-02-16 16:18 ` [PATCH v4 02/10] softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit" huangy81
2023-02-16 16:18 ` [PATCH v4 03/10] kvm: dirty-ring: Fix race with vcpu creation huangy81
2023-04-04 13:32 ` Paolo Bonzini
2023-04-04 14:10 ` Peter Xu
2023-04-04 16:08 ` Paolo Bonzini
2023-04-04 16:36 ` Peter Xu
2023-04-04 16:45 ` Paolo Bonzini
2023-02-16 16:18 ` [PATCH v4 04/10] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter huangy81
2023-02-16 16:18 ` [PATCH v4 05/10] qapi/migration: Introduce vcpu-dirty-limit parameters huangy81
2023-02-16 16:18 ` [PATCH v4 06/10] migration: Introduce dirty-limit capability huangy81
2023-03-24 12:11 ` Markus Armbruster
[not found] ` <f70dbc9b-e722-ad77-e22d-12c339f5ff4d@chinatelecom.cn>
2023-03-24 14:32 ` Markus Armbruster
2023-03-26 7:29 ` Hyman Huang
2023-03-27 6:41 ` Markus Armbruster
2023-03-28 5:28 ` Hyman Huang
2023-02-16 16:18 ` [PATCH v4 07/10] migration: Refactor auto-converge capability logic huangy81
2023-02-16 16:18 ` [PATCH v4 08/10] migration: Implement dirty-limit convergence algo huangy81
2023-02-16 16:18 ` [PATCH v4 09/10] migration: Extend query-migrate to provide dirty page limit info huangy81
2023-02-16 16:18 ` [PATCH v4 10/10] tests: Add migration dirty-limit capability test huangy81
2023-03-01 15:53 ` [PATCH v4 00/10] migration: introduce dirtylimit capability Hyman Huang
2023-03-24 7:27 ` Hyman Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).