On Fri, Aug 4, 2023 at 11:03 PM Andrei Gudkov <gudkov.and...@huawei.com>
wrote:
> Introduces alternative argument calc-time-ms, which is the
> the same as calc-time but accepts millisecond value.
> Millisecond granularity allows to make predictions whether
> migration will succeed or not. To do this, calculate dirty
> rate with calc-time-ms set to max allowed downtime, convert
> measured rate into volume of dirtied memory, and divide by
> network throughput. If the value is lower than max allowed
>
Not for the patch, I'm just curious about how the predication
decides the network throughput, I mean QEMU predicts
if migration will converge based on how fast it sends the data,
not the actual bandwidth of the interface, which one the
prediction uses?
> downtime, then migration will converge.
>
> Measurement results for single thread randomly writing to
> a 1/4/24GiB memory region:
>
> +--------------+-----------------------------------------------+
> | calc-time-ms | dirty rate MiB/s |
> | +----------------+---------------+--------------+
> | | theoretical | page-sampling | dirty-bitmap |
> | | (at 3M wr/sec) | | |
> +--------------+----------------+---------------+--------------+
> | 1GiB |
> +--------------+----------------+---------------+--------------+
> | 100 | 6996 | 7100 | 3192 |
> | 200 | 4606 | 4660 | 2655 |
> | 300 | 3305 | 3280 | 2371 |
> | 400 | 2534 | 2525 | 2154 |
> | 500 | 2041 | 2044 | 1871 |
> | 750 | 1365 | 1341 | 1358 |
> | 1000 | 1024 | 1052 | 1025 |
> | 1500 | 683 | 678 | 684 |
> | 2000 | 512 | 507 | 513 |
> +--------------+----------------+---------------+--------------+
> | 4GiB |
> +--------------+----------------+---------------+--------------+
> | 100 | 10232 | 8880 | 4070 |
> | 200 | 8954 | 8049 | 3195 |
> | 300 | 7889 | 7193 | 2881 |
> | 400 | 6996 | 6530 | 2700 |
> | 500 | 6245 | 5772 | 2312 |
> | 750 | 4829 | 4586 | 2465 |
> | 1000 | 3865 | 3780 | 2178 |
> | 1500 | 2694 | 2633 | 2004 |
> | 2000 | 2041 | 2031 | 1789 |
> +--------------+----------------+---------------+--------------+
> | 24GiB |
> +--------------+----------------+---------------+--------------+
> | 100 | 11495 | 8640 | 5597 |
> | 200 | 11226 | 8616 | 3527 |
> | 300 | 10965 | 8386 | 2355 |
> | 400 | 10713 | 8370 | 2179 |
> | 500 | 10469 | 8196 | 2098 |
> | 750 | 9890 | 7885 | 2556 |
> | 1000 | 9354 | 7506 | 2084 |
> | 1500 | 8397 | 6944 | 2075 |
> | 2000 | 7574 | 6402 | 2062 |
> +--------------+----------------+---------------+--------------+
>
> Theoretical values are computed according to the following formula:
> size * (1 - (1-(4096/size))^(time*wps)) / (time * 2^20),
> where size is in bytes, time is in seconds, and wps is number of
> writes per second.
>
> Signed-off-by: Andrei Gudkov <gudkov.and...@huawei.com>
> ---
> qapi/migration.json | 14 ++++++--
> migration/dirtyrate.h | 12 ++++---
> migration/dirtyrate.c | 81 +++++++++++++++++++++++++------------------
> 3 files changed, 67 insertions(+), 40 deletions(-)
>
> [...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 8843e74b59..82493d6a57 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1849,7 +1849,11 @@
> # @start-time: start time in units of second for calculation
> #
> # @calc-time: time period for which dirty page rate was measured
> -# (in seconds)
> +# (rounded down to seconds).
>
Does there need an extra comment to emphasize that calc-time shows
zero if the calc-time-ms is lower than 1000?
> +#
> +# @calc-time-ms: actual time period for which dirty page rate was
> +# measured (in milliseconds). Value may be larger than requested
> +# time period due to measurement overhead.
> #
> # @sample-pages: number of sampled pages per GiB of guest memory.
> # Valid only in page-sampling mode (Since 6.1)
> @@ -1866,6 +1870,7 @@
> 'status': 'DirtyRateStatus',
> 'start-time': 'int64',
> 'calc-time': 'int64',
> + 'calc-time-ms': 'int64',
> 'sample-pages': 'uint64',
> 'mode': 'DirtyRateMeasureMode',
> '*vcpu-dirty-rate': [ 'DirtyRateVcpu' ] } }
> @@ -1908,6 +1913,10 @@
> # dirty during @calc-time period, further writes to this page will
> # not increase dirty page rate anymore.
> #
> +# @calc-time-ms: the same as @calc-time but in milliseconds. These
> +# two arguments are mutually exclusive. Exactly one of them must
> +# be specified. (Since 8.1)
> +#
> # @sample-pages: number of sampled pages per each GiB of guest memory.
> # Default value is 512. For 4KiB guest pages this corresponds to
> # sampling ratio of 0.2%. This argument is used only in page
> @@ -1925,7 +1934,8 @@
> # 'sample-pages': 512} }
> # <- { "return": {} }
> ##
> -{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64',
> +{ 'command': 'calc-dirty-rate', 'data': {'*calc-time': 'int64',
> + '*calc-time-ms': 'int64',
> '*sample-pages': 'int',
> '*mode': 'DirtyRateMeasureMode'}
> }
>
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index 594a5c0bb6..869c060941 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -31,10 +31,12 @@
> #define MIN_RAMBLOCK_SIZE 128
>
> /*
> - * Take 1s as minimum time for calculation duration
> + * Allowed range for dirty page rate calculation (in milliseconds).
> + * Lower limit relates to the smallest realistic downtime it
> + * makes sense to impose on migration.
> */
> -#define MIN_FETCH_DIRTYRATE_TIME_SEC 1
> -#define MAX_FETCH_DIRTYRATE_TIME_SEC 60
> +#define MIN_CALC_TIME_MS 50
> +#define MAX_CALC_TIME_MS 60000
>
> /*
> * Take 1/16 pages in 1G as the maxmum sample page count
> @@ -44,7 +46,7 @@
>
> struct DirtyRateConfig {
> uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
> - int64_t sample_period_seconds; /* time duration between two sampling
> */
> + int64_t calc_time_ms; /* desired calculation time (in milliseconds) */
> DirtyRateMeasureMode mode; /* mode of dirtyrate measurement */
> };
>
> @@ -73,7 +75,7 @@ typedef struct SampleVMStat {
> struct DirtyRateStat {
> int64_t dirty_rate; /* dirty rate in MB/s */
> int64_t start_time; /* calculation start time in units of second */
> - int64_t calc_time; /* time duration of two sampling in units of
> second */
> + int64_t calc_time_ms; /* actual calculation time (in milliseconds) */
> uint64_t sample_pages; /* sample pages per GB */
> union {
> SampleVMStat page_sampling;
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 84f1b0fb20..90fb336329 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -57,6 +57,8 @@ static int64_t dirty_stat_wait(int64_t msec, int64_t
> initial_time)
> msec = current_time - initial_time;
> } else {
> g_usleep((msec + initial_time - current_time) * 1000);
> + /* g_usleep may overshoot */
> + msec = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - initial_time;
>
The optimization could be a standalone commit along with the content
below(see the following comment)?
> }
>
> return msec;
> @@ -77,9 +79,12 @@ static int64_t do_calculate_dirtyrate(DirtyPageRecord
> dirty_pages,
> {
> uint64_t increased_dirty_pages =
> dirty_pages.end_pages - dirty_pages.start_pages;
> - uint64_t memory_size_MiB =
> qemu_target_pages_to_MiB(increased_dirty_pages);
> -
> - return memory_size_MiB * 1000 / calc_time_ms;
> + /*
> + * multiply by 1000ms/s _before_ converting down to megabytes
> + * to avoid losing precision
> + */
> + return qemu_target_pages_to_MiB(increased_dirty_pages * 1000) /
> + calc_time_ms;
>
Code optimization, could be in a standalone commit.
> }
>
> void global_dirty_log_change(unsigned int flag, bool start)
> @@ -183,10 +188,9 @@ retry:
> return duration;
> }
>
> -static bool is_sample_period_valid(int64_t sec)
> +static bool is_calc_time_valid(int64_t msec)
> {
> - if (sec < MIN_FETCH_DIRTYRATE_TIME_SEC ||
> - sec > MAX_FETCH_DIRTYRATE_TIME_SEC) {
> + if ((msec < MIN_CALC_TIME_MS) || (msec > MAX_CALC_TIME_MS)) {
> return false;
> }
>
> @@ -219,7 +223,8 @@ static struct DirtyRateInfo
> *query_dirty_rate_info(void)
>
> info->status = CalculatingState;
> info->start_time = DirtyStat.start_time;
> - info->calc_time = DirtyStat.calc_time;
> + info->calc_time_ms = DirtyStat.calc_time_ms;
> + info->calc_time = DirtyStat.calc_time_ms / 1000;
> info->sample_pages = DirtyStat.sample_pages;
> info->mode = dirtyrate_mode;
>
> @@ -258,7 +263,7 @@ static void init_dirtyrate_stat(int64_t start_time,
> {
> DirtyStat.dirty_rate = -1;
> DirtyStat.start_time = start_time;
> - DirtyStat.calc_time = config.sample_period_seconds;
> + DirtyStat.calc_time_ms = config.calc_time_ms;
> DirtyStat.sample_pages = config.sample_pages_per_gigabytes;
>
> switch (config.mode) {
> @@ -568,7 +573,6 @@ static inline void dirtyrate_manual_reset_protect(void)
>
> static void calculate_dirtyrate_dirty_bitmap(struct DirtyRateConfig
> config)
> {
> - int64_t msec = 0;
> int64_t start_time;
> DirtyPageRecord dirty_pages;
>
> @@ -596,9 +600,7 @@ static void calculate_dirtyrate_dirty_bitmap(struct
> DirtyRateConfig config)
> start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> DirtyStat.start_time = start_time / 1000;
>
> - msec = config.sample_period_seconds * 1000;
> - msec = dirty_stat_wait(msec, start_time);
> - DirtyStat.calc_time = msec / 1000;
> + DirtyStat.calc_time_ms = dirty_stat_wait(config.calc_time_ms,
> start_time);
>
> /*
> * do two things.
> @@ -609,12 +611,12 @@ static void calculate_dirtyrate_dirty_bitmap(struct
> DirtyRateConfig config)
>
> record_dirtypages_bitmap(&dirty_pages, false);
>
> - DirtyStat.dirty_rate = do_calculate_dirtyrate(dirty_pages, msec);
> + DirtyStat.dirty_rate = do_calculate_dirtyrate(dirty_pages,
> + DirtyStat.calc_time_ms);
> }
>
> static void calculate_dirtyrate_dirty_ring(struct DirtyRateConfig config)
> {
> - int64_t duration;
> uint64_t dirtyrate = 0;
> uint64_t dirtyrate_sum = 0;
> int i = 0;
> @@ -625,12 +627,10 @@ static void calculate_dirtyrate_dirty_ring(struct
> DirtyRateConfig config)
> DirtyStat.start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
>
> /* calculate vcpu dirtyrate */
> - duration = vcpu_calculate_dirtyrate(config.sample_period_seconds *
> 1000,
> - &DirtyStat.dirty_ring,
> - GLOBAL_DIRTY_DIRTY_RATE,
> - true);
> -
> - DirtyStat.calc_time = duration / 1000;
> + DirtyStat.calc_time_ms = vcpu_calculate_dirtyrate(config.calc_time_ms,
> +
> &DirtyStat.dirty_ring,
> +
> GLOBAL_DIRTY_DIRTY_RATE,
> + true);
>
> /* calculate vm dirtyrate */
> for (i = 0; i < DirtyStat.dirty_ring.nvcpu; i++) {
> @@ -646,7 +646,6 @@ static void calculate_dirtyrate_sample_vm(struct
> DirtyRateConfig config)
> {
> struct RamblockDirtyInfo *block_dinfo = NULL;
> int block_count = 0;
> - int64_t msec = 0;
> int64_t initial_time;
>
> rcu_read_lock();
> @@ -656,17 +655,16 @@ static void calculate_dirtyrate_sample_vm(struct
> DirtyRateConfig config)
> }
> rcu_read_unlock();
>
> - msec = config.sample_period_seconds * 1000;
> - msec = dirty_stat_wait(msec, initial_time);
> + DirtyStat.calc_time_ms = dirty_stat_wait(config.calc_time_ms,
> + initial_time);
> DirtyStat.start_time = initial_time / 1000;
> - DirtyStat.calc_time = msec / 1000;
>
> rcu_read_lock();
> if (!compare_page_hash_info(block_dinfo, block_count)) {
> goto out;
> }
>
> - update_dirtyrate(msec);
> + update_dirtyrate(DirtyStat.calc_time_ms);
>
> out:
> rcu_read_unlock();
> @@ -711,7 +709,10 @@ void *get_dirtyrate_thread(void *arg)
> return NULL;
> }
>
> -void qmp_calc_dirty_rate(int64_t calc_time,
> +void qmp_calc_dirty_rate(bool has_calc_time,
> + int64_t calc_time,
> + bool has_calc_time_ms,
> + int64_t calc_time_ms,
> bool has_sample_pages,
> int64_t sample_pages,
> bool has_mode,
> @@ -731,10 +732,21 @@ void qmp_calc_dirty_rate(int64_t calc_time,
> return;
> }
>
> - if (!is_sample_period_valid(calc_time)) {
> - error_setg(errp, "calc-time is out of range[%d, %d].",
> - MIN_FETCH_DIRTYRATE_TIME_SEC,
> - MAX_FETCH_DIRTYRATE_TIME_SEC);
> + if ((int)has_calc_time + (int)has_calc_time_ms != 1) {
> + error_setg(errp, "Exactly one of calc-time and calc-time-ms must"
> + " be specified");
> + return;
> + }
> + if (has_calc_time) {
> + /*
> + * The worst thing that can happen due to overflow is that
> + * invalid value will become valid.
> + */
> + calc_time_ms = calc_time * 1000;
> + }
> + if (!is_calc_time_valid(calc_time_ms)) {
> + error_setg(errp, "Calculation time is out of range[%dms, %dms].",
> + MIN_CALC_TIME_MS, MAX_CALC_TIME_MS);
> return;
> }
>
> @@ -781,7 +793,7 @@ void qmp_calc_dirty_rate(int64_t calc_time,
> return;
> }
>
> - config.sample_period_seconds = calc_time;
> + config.calc_time_ms = calc_time_ms;
> config.sample_pages_per_gigabytes = sample_pages;
> config.mode = mode;
>
> @@ -867,8 +879,11 @@ void hmp_calc_dirty_rate(Monitor *mon, const QDict
> *qdict)
> mode = DIRTY_RATE_MEASURE_MODE_DIRTY_RING;
> }
>
> - qmp_calc_dirty_rate(sec, has_sample_pages, sample_pages, true,
> - mode, &err);
> + qmp_calc_dirty_rate(true, sec, /* calc_time */
> + false, 0, /* calc_time_ms */
> + has_sample_pages, sample_pages,
> + true, mode,
> + &err);
> if (err) {
> hmp_handle_error(mon, err);
> return;
> --
> 2.30.2
>
> The patch set works for me, and I'm inclined to split it into two commits
as I point out above.
Thanks
Yong
--
Best regards
