[PATCH RESEND v3 00/10] migration: introduce dirtylimit capability

[PATCH RESEND v3 00/10] migration: introduce dirtylimit capability

[huangy81]

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

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: Export dirty-limit time info for observation
  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.c        |  87 ++++++++++++++++++++++++
 migration/migration.h        |   1 +
 migration/ram.c              |  63 ++++++++++++++----
 migration/trace-events       |   1 +
 monitor/hmp-cmds.c           |  26 ++++++++
 qapi/migration.json          |  65 +++++++++++++++---
 softmmu/dirtylimit.c         |  91 ++++++++++++++++++++++---
 tests/qtest/migration-test.c | 154 +++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 467 insertions(+), 32 deletions(-)

-- 
1.8.3.1

[PATCH RESEND v3 01/10] dirtylimit: Fix overflow when computing MB

[huangy81]

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 1266855..940d238 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -236,14 +236,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,
-- 
1.8.3.1

[PATCH RESEND v3 02/10] softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit"

[huangy81]

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 940d238..53b66d5 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -515,14 +515,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)
-- 
1.8.3.1

[PATCH RESEND v3 03/10] kvm: dirty-ring: Fix race with vcpu creation

[huangy81]

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 f99b0be..ff26b07 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -683,6 +683,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);
 
-- 
1.8.3.1

[PATCH RESEND v3 04/10] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter

[huangy81]

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>
---
 migration/migration.c | 26 ++++++++++++++++++++++++++
 monitor/hmp-cmds.c    |  8 ++++++++
 qapi/migration.json   | 34 +++++++++++++++++++++++++++-------
 3 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index f485eea..1439d61 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -116,6 +116,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);
 
@@ -963,6 +965,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;
 }
 
@@ -1582,6 +1587,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;
 }
 
@@ -1681,6 +1695,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)
@@ -1803,6 +1821,10 @@ 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)
@@ -4404,6 +4426,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),
@@ -4495,6 +4520,7 @@ static void migration_instance_init(Object *obj)
     params->has_tls_creds = true;
     params->has_tls_hostname = true;
     params->has_tls_authz = 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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 01b789a..a3170ca 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -513,6 +513,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);
@@ -1332,6 +1336,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/qapi/migration.json b/qapi/migration.json
index 88ecf86..c428bcd 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -776,8 +776,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 7.3)
+#
 # 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 +800,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 +947,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 7.3)
+#
 # 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 +987,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 +1154,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 7.3)
+#
 # 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 +1192,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:
-- 
1.8.3.1

[PATCH RESEND v3 05/10] qapi/migration: Introduce vcpu-dirty-limit parameters

[huangy81]

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.c | 23 +++++++++++++++++++++++
 monitor/hmp-cmds.c    |  8 ++++++++
 qapi/migration.json   | 18 +++++++++++++++---
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 1439d61..fd11c63 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -117,6 +117,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);
@@ -968,6 +969,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;
 }
 
@@ -1596,6 +1600,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",
+                   "a value greater than or equal to 1");
+        return false;
+    }
+
     return true;
 }
 
@@ -1699,6 +1711,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)
@@ -1825,6 +1841,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)
@@ -4429,6 +4448,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),
@@ -4521,6 +4543,7 @@ static void migration_instance_init(Object *obj)
     params->has_tls_hostname = true;
     params->has_tls_authz = 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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index a3170ca..9ad6ee5 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -517,6 +517,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);
@@ -1340,6 +1344,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/qapi/migration.json b/qapi/migration.json
index c428bcd..7e868a1 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -780,6 +780,9 @@
 #                             live migration. Should be in the range 1 to 1000ms,
 #                             defaults to 1000ms. (Since 7.3)
 #
+# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
+#                    Defaults to 1. (Since 7.3)
+#
 # Features:
 # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
 #            are experimental.
@@ -802,7 +805,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:
@@ -951,6 +955,9 @@
 #                             live migration. Should be in the range 1 to 1000ms,
 #                             defaults to 1000ms. (Since 7.3)
 #
+# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
+#                    Defaults to 1. (Since 7.3)
+#
 # Features:
 # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
 #            are experimental.
@@ -989,7 +996,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:
@@ -1158,6 +1166,9 @@
 #                             live migration. Should be in the range 1 to 1000ms,
 #                             defaults to 1000ms. (Since 7.3)
 #
+# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
+#                    Defaults to 1. (Since 7.3)
+#
 # Features:
 # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
 #            are experimental.
@@ -1194,7 +1205,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:
-- 
1.8.3.1

[PATCH RESEND v3 06/10] migration: Introduce dirty-limit capability

[huangy81]

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 fd11c63..702e7f4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -61,6 +61,7 @@
 #include "sysemu/cpus.h"
 #include "yank_functions.h"
 #include "sysemu/qtest.h"
+#include "sysemu/kvm.h"
 
 #define MAX_THROTTLE  (128 << 20)      /* Migration transfer speed throttling */
 
@@ -1366,6 +1367,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"
+                       " only one of them is 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;
 }
 
@@ -2544,6 +2559,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;
@@ -4473,6 +4497,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 cdad8ac..7fbb9f8 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -409,6 +409,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 7e868a1..6055fdc 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 7.3)
 #
 # 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 53b66d5..2a07200 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -23,6 +23,8 @@
 #include "exec/memory.h"
 #include "hw/boards.h"
 #include "sysemu/kvm.h"
+#include "migration/misc.h"
+#include "migration/migration.h"
 #include "trace.h"
 
 /*
@@ -75,11 +77,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);
-- 
1.8.3.1

[PATCH RESEND v3 07/10] migration: Refactor auto-converge capability logic

[huangy81]

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>
---
 migration/ram.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 1338e47..5e66652 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1151,7 +1151,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
-- 
1.8.3.1

[PATCH RESEND v3 08/10] migration: Implement dirty-limit convergence algo

[huangy81]

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>
---
 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 702e7f4..127d0fe 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -240,6 +240,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 5e66652..78b9167 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() */
 
@@ -1139,6 +1142,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();
@@ -1148,26 +1175,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 57003ed..33a2666 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 2a07200..b63032c 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -439,6 +439,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;
     }
@@ -452,6 +454,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, "dirty-limit live migration is running, do"
+                   " not allow dirty page limit to be canceled manually");
+        return;
+    }
+
     dirtylimit_state_lock();
 
     if (has_cpu_index) {
@@ -487,6 +498,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'");
@@ -503,6 +516,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, "dirty-limit live migration is running, do"
+                   " not allow dirty page limit to be configured manually");
+        return;
+    }
+
     dirtylimit_state_lock();
 
     if (!dirtylimit_in_service()) {
-- 
1.8.3.1

[PATCH RESEND v3 09/10] migration: Export dirty-limit time info for observation

[huangy81]

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Export dirty limit throttle time and estimated ring full
time, through which we can observe if dirty limit take
effect during live migration.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 include/sysemu/dirtylimit.h |  2 ++
 migration/migration.c       | 10 ++++++++++
 monitor/hmp-cmds.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 8d2c1f3..f15e01d 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_full(void);
+int64_t dirtylimit_ring_full_time(void);
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index 127d0fe..3f92389 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -62,6 +62,7 @@
 #include "yank_functions.h"
 #include "sysemu/qtest.h"
 #include "sysemu/kvm.h"
+#include "sysemu/dirtylimit.h"
 
 #define MAX_THROTTLE  (128 << 20)      /* Migration transfer speed throttling */
 
@@ -1114,6 +1115,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_full = true;
+        info->dirty_limit_throttle_time_per_full =
+                            dirtylimit_throttle_time_per_full();
+
+        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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9ad6ee5..c3aaba3 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -339,6 +339,16 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
                        info->cpu_throttle_percentage);
     }
 
+    if (info->has_dirty_limit_throttle_time_per_full) {
+        monitor_printf(mon, "dirty-limit throttle time: %" PRIi64 " us\n",
+                       info->dirty_limit_throttle_time_per_full);
+    }
+
+    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/qapi/migration.json b/qapi/migration.json
index 6055fdc..ae7d22d 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-full: Maximum throttle time (in microseconds) of virtual
+#                                      CPUs each dirty ring full round, used to observe
+#                                      if dirty-limit take effect during live migration.
+#                                      (since 7.3)
+#
+# @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 7.3)
+#
 # 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-full': 'int64',
+           '*dirty-limit-ring-full-time': 'int64'} }
 
 ##
 # @query-migrate:
diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c
index b63032c..06de099 100644
--- a/softmmu/dirtylimit.c
+++ b/softmmu/dirtylimit.c
@@ -569,6 +569,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_full(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;
-- 
1.8.3.1

[PATCH RESEND v3 10/10] tests: Add migration dirty-limit capability test

[huangy81]

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 | 154 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 442998d..03b47f5 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2422,6 +2422,158 @@ 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-full");
+        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-full");
+        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-full");
+        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)
@@ -2592,6 +2744,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();
-- 
1.8.3.1

Re: [PATCH RESEND v3 00/10] migration: introduce dirtylimit capability

[Hyman]

Ping ？

在 2022/12/4 1:09, huangy81@chinatelecom.cn 写道:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> 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: Export dirty-limit time info for observation
>    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.c        |  87 ++++++++++++++++++++++++
>   migration/migration.h        |   1 +
>   migration/ram.c              |  63 ++++++++++++++----
>   migration/trace-events       |   1 +
>   monitor/hmp-cmds.c           |  26 ++++++++
>   qapi/migration.json          |  65 +++++++++++++++---
>   softmmu/dirtylimit.c         |  91 ++++++++++++++++++++++---
>   tests/qtest/migration-test.c | 154 +++++++++++++++++++++++++++++++++++++++++++
>   10 files changed, 467 insertions(+), 32 deletions(-)
> 

Re: [PATCH RESEND v3 07/10] migration: Refactor auto-converge capability logic

[Peter Xu]

On Sun, Dec 04, 2022 at 01:09:10AM +0800, huangy81@chinatelecom.cn wrote:
> 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>

-- 
Peter Xu

Re: [PATCH RESEND v3 00/10] migration: introduce dirtylimit capability

[Hyman Huang]

Ping,

Hi, David, how about the commit about live migration:
[PATCH RESEND v3 08/10] migration: Implement dirty-limit convergence algo.

在 2022/12/4 1:09, huangy81@chinatelecom.cn 写道:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> 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: Export dirty-limit time info for observation
>    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.c        |  87 ++++++++++++++++++++++++
>   migration/migration.h        |   1 +
>   migration/ram.c              |  63 ++++++++++++++----
>   migration/trace-events       |   1 +
>   monitor/hmp-cmds.c           |  26 ++++++++
>   qapi/migration.json          |  65 +++++++++++++++---
>   softmmu/dirtylimit.c         |  91 ++++++++++++++++++++++---
>   tests/qtest/migration-test.c | 154 +++++++++++++++++++++++++++++++++++++++++++
>   10 files changed, 467 insertions(+), 32 deletions(-)
> 

-- 
Best regard

Hyman Huang(黄勇)

Re: [PATCH RESEND v3 00/10] migration: introduce dirtylimit capability

[Markus Armbruster]

My sincere apologies for not replying sooner.

This needs a rebase now.  But let me have a look at it first.

Re: [PATCH RESEND v3 04/10] qapi/migration: Introduce x-vcpu-dirty-limit-period parameter

[Markus Armbruster]

huangy81@chinatelecom.cn writes:

> 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.

I'm not a native speaker, but let me try to polish your prose anyway:

  The value of "x-vcpu-dirty-limit-period" affects how long live migration
  takes.  In my testing, the optimal value of "x-vcpu-dirty-limit-period"
  ranges from 500ms to 1000 ms.  If we can find a single value that is
  good enough for all use cases that matter, "x-vcpu-dirty-limit-period"
  can go away.  Else, "x-vcpu-dirty-limit-period" should be made stable.

Does this capture your intent?

> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

[...]

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 88ecf86..c428bcd 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -776,8 +776,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 7.3)

8.0, not 7.3, unless we change our versioning habits.  More of the same
below and later in this series; I'm not flagging it again.

> +#
>  # Features:
> -# @unstable: Member @x-checkpoint-delay is experimental.
> +# @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
> +#            are experimental.
>  #
>  # Since: 2.4
>  ##

[...]

Re: [PATCH RESEND v3 08/10] migration: Implement dirty-limit convergence algo

[Markus Armbruster]

huangy81@chinatelecom.cn writes:

> 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.

Only during live migration, or also during migration of a stopped guest?
If the latter, the easy fix is to scratch "live".

> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> ---
>  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 702e7f4..127d0fe 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -240,6 +240,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 5e66652..78b9167 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() */
>  
> @@ -1139,6 +1142,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();
> @@ -1148,26 +1175,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 57003ed..33a2666 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 2a07200..b63032c 100644
> --- a/softmmu/dirtylimit.c
> +++ b/softmmu/dirtylimit.c
> @@ -439,6 +439,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;
>      }
> @@ -452,6 +454,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, "dirty-limit live migration is running, do"
> +                   " not allow dirty page limit to be canceled manually");

"do not allow" sounds like a request.  What about "can't cancel dirty
page limit while migration is running"?

> +        return;
> +    }
> +
>      dirtylimit_state_lock();
>  
>      if (has_cpu_index) {
> @@ -487,6 +498,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'");
> @@ -503,6 +516,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, "dirty-limit live migration is running, do"
> +                   " not allow dirty page limit to be configured manually");

Likewise.

> +        return;
> +    }
> +
>      dirtylimit_state_lock();
>  
>      if (!dirtylimit_in_service()) {

Re: [PATCH RESEND v3 09/10] migration: Export dirty-limit time info for observation

[Markus Armbruster]

huangy81@chinatelecom.cn writes:

> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>
> Export dirty limit throttle time and estimated ring full
> time, through which we can observe if dirty limit take
> effect during live migration.

Suggest something like "Extend query-migrate to provide ..." both here
and in subject.

> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> ---
>  include/sysemu/dirtylimit.h |  2 ++
>  migration/migration.c       | 10 ++++++++++
>  monitor/hmp-cmds.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 8d2c1f3..f15e01d 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_full(void);
> +int64_t dirtylimit_ring_full_time(void);
>  #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index 127d0fe..3f92389 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -62,6 +62,7 @@
>  #include "yank_functions.h"
>  #include "sysemu/qtest.h"
>  #include "sysemu/kvm.h"
> +#include "sysemu/dirtylimit.h"
>  
>  #define MAX_THROTTLE  (128 << 20)      /* Migration transfer speed throttling */
>  
> @@ -1114,6 +1115,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_full = true;
> +        info->dirty_limit_throttle_time_per_full =
> +                            dirtylimit_throttle_time_per_full();
> +
> +        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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 9ad6ee5..c3aaba3 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -339,6 +339,16 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>                         info->cpu_throttle_percentage);
>      }
>  
> +    if (info->has_dirty_limit_throttle_time_per_full) {
> +        monitor_printf(mon, "dirty-limit throttle time: %" PRIi64 " us\n",
> +                       info->dirty_limit_throttle_time_per_full);
> +    }
> +
> +    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);
> +    }

I discuss naming below.  If we change the names, we probably want to
change the string literals here, too.

> +
>      if (info->has_postcopy_blocktime) {
>          monitor_printf(mon, "postcopy blocktime: %u\n",
>                         info->postcopy_blocktime);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 6055fdc..ae7d22d 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-full: Maximum throttle time (in microseconds) of virtual
> +#                                      CPUs each dirty ring full round, used to observe

What's a dirty "ring full round"?  Is it a migration round?  Something
else?

> +#                                      if dirty-limit take effect during live migration.

takes effect

I think "if dirty-limit takes effect" isn't quite right.  We can use
this to observe how MigrationCapability dirty-limit affects the guest.
What about "shows how MigrationCapability dirty-limit affects the
guest"?

Even though dirty-limit-throttle-time-per-full is quite long, it still
feels abbreviated.  Full what?  What about
dirty-limit-throttle-time-per-round?  Naming is hard.

> +#                                      (since 7.3)
> +#
> +# @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 7.3)
> +#

Uff.

What is estimated?  The average amount of time the dirty ring (whatever
that is) is full in each migration round?

Aside: our doc comment syntax can push text blocks far to the right.
Not good.  Also not your fault, and not your problem to fix.

>  # 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-full': 'int64',
> +           '*dirty-limit-ring-full-time': 'int64'} }
>  
>  ##
>  # @query-migrate:

[...]

Re: [PATCH RESEND v3 09/10] migration: Export dirty-limit time info for observation

[Hyman Huang]

在 2023/1/11 22:38, Markus Armbruster 写道:
> huangy81@chinatelecom.cn writes:
> 
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> Export dirty limit throttle time and estimated ring full
>> time, through which we can observe if dirty limit take
>> effect during live migration.
> 
> Suggest something like "Extend query-migrate to provide ..." both here
> and in subject.
> 
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> ---
>>   include/sysemu/dirtylimit.h |  2 ++
>>   migration/migration.c       | 10 ++++++++++
>>   monitor/hmp-cmds.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 8d2c1f3..f15e01d 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_full(void);
>> +int64_t dirtylimit_ring_full_time(void);
>>   #endif
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 127d0fe..3f92389 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -62,6 +62,7 @@
>>   #include "yank_functions.h"
>>   #include "sysemu/qtest.h"
>>   #include "sysemu/kvm.h"
>> +#include "sysemu/dirtylimit.h"
>>   
>>   #define MAX_THROTTLE  (128 << 20)      /* Migration transfer speed throttling */
>>   
>> @@ -1114,6 +1115,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_full = true;
>> +        info->dirty_limit_throttle_time_per_full =
>> +                            dirtylimit_throttle_time_per_full();
>> +
>> +        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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> index 9ad6ee5..c3aaba3 100644
>> --- a/monitor/hmp-cmds.c
>> +++ b/monitor/hmp-cmds.c
>> @@ -339,6 +339,16 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>>                          info->cpu_throttle_percentage);
>>       }
>>   
>> +    if (info->has_dirty_limit_throttle_time_per_full) {
>> +        monitor_printf(mon, "dirty-limit throttle time: %" PRIi64 " us\n",
>> +                       info->dirty_limit_throttle_time_per_full);
>> +    }
>> +
>> +    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);
>> +    }
> 
> I discuss naming below.  If we change the names, we probably want to
> change the string literals here, too.
> 
>> +
>>       if (info->has_postcopy_blocktime) {
>>           monitor_printf(mon, "postcopy blocktime: %u\n",
>>                          info->postcopy_blocktime);
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 6055fdc..ae7d22d 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-full: Maximum throttle time (in microseconds) of virtual
>> +#                                      CPUs each dirty ring full round, used to observe
> 
> What's a dirty "ring full round"?  Is it a migration round?  Something
> else?
No, not migration round.

When dirty ring is full and return to userspace due to the exception 
"KVM_EXIT_DIRTY_RING_FULL"。 The "dirty-limit" logic would make vcpu 
sleep some time, which is adjusted dynamically every exception, and 
which is represented by the "dirty-limit-throttle-time-per-full".

As to "dirty ring full round" (sorry about that i failed to make it 
clear), It represents one round that dirty ring of vCPU is full.
> 
>> +#                                      if dirty-limit take effect during live migration.
> 
> takes effect
> 
> I think "if dirty-limit takes effect" isn't quite right.  We can use
> this to observe how MigrationCapability dirty-limit affects the guest.
> What about "shows how MigrationCapability dirty-limit affects the
> guest"?
It's better than what i described.

> 
> Even though dirty-limit-throttle-time-per-full is quite long, it still
> feels abbreviated.  Full what?  What about
> dirty-limit-throttle-time-per-round?  Naming is hard.
It's ok

> 
>> +#                                      (since 7.3)
>> +#
>> +# @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 7.3)
>> +#
> 
> Uff.
> 
> What is estimated?  The average amount of time the dirty ring (whatever
> that is) is full in each migration round?
Yes. Since we could not monitor the actual time, the value is calculated 
by the formula:
dirty ring full time = dirty ring size / dirty page rate.

> 
> Aside: our doc comment syntax can push text blocks far to the right.
> Not good.  Also not your fault, and not your problem to fix.
> 
>>   # 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-full': 'int64',
>> +           '*dirty-limit-ring-full-time': 'int64'} }
>>   
>>   ##
>>   # @query-migrate:
> 
> [...]
> 

-- 
Best regard

Hyman Huang(黄勇)

Re: [PATCH RESEND v3 08/10] migration: Implement dirty-limit convergence algo

[Hyman Huang]

在 2023/1/11 22:11, Markus Armbruster 写道:
> huangy81@chinatelecom.cn writes:
> 
>> 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.
> 
> Only during live migration, or also during migration of a stopped guest?
> If the latter, the easy fix is to scratch "live".
> 
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> ---
>>   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 702e7f4..127d0fe 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -240,6 +240,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 5e66652..78b9167 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() */
>>   
>> @@ -1139,6 +1142,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();
>> @@ -1148,26 +1175,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 57003ed..33a2666 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 2a07200..b63032c 100644
>> --- a/softmmu/dirtylimit.c
>> +++ b/softmmu/dirtylimit.c
>> @@ -439,6 +439,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;
>>       }
>> @@ -452,6 +454,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, "dirty-limit live migration is running, do"
>> +                   " not allow dirty page limit to be canceled manually");
> 
> "do not allow" sounds like a request.  What about "can't cancel dirty
> page limit while migration is running"?
Ok, i'll modify next version according to comment and previous comments 
in other commits.

Thanks Markus for the comments.

Yong

> 
>> +        return;
>> +    }
>> +
>>       dirtylimit_state_lock();
>>   
>>       if (has_cpu_index) {
>> @@ -487,6 +498,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'");
>> @@ -503,6 +516,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, "dirty-limit live migration is running, do"
>> +                   " not allow dirty page limit to be configured manually");
> 
> Likewise.
> 
>> +        return;
>> +    }
>> +
>>       dirtylimit_state_lock();
>>   
>>       if (!dirtylimit_in_service()) {
> 

-- 
Best regard

Hyman Huang(黄勇)

[PATCH RESEND v3 00/10] migration: introduce dirtylimit capability

[huangy81]

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

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: Export dirty-limit time info for observation
  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.c        |  87 ++++++++++++++++++++++++
 migration/migration.h        |   1 +
 migration/ram.c              |  63 ++++++++++++++----
 migration/trace-events       |   1 +
 monitor/hmp-cmds.c           |  26 ++++++++
 qapi/migration.json          |  65 +++++++++++++++---
 softmmu/dirtylimit.c         |  91 ++++++++++++++++++++++---
 tests/qtest/migration-test.c | 154 +++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 467 insertions(+), 32 deletions(-)

-- 
1.8.3.1