* [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
* 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
* [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
* 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
* [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