On 1/11/19 5:57 PM, Markus Armbruster wrote:
guangrong.xiao@xxxxxxxxx writes:
From: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxx>
Currently we have two behaviors if all threads are busy to do compression,
the main thread mush wait one of them becoming free if @compress-wait-thread
set to on or the main thread can directly return without wait and post
the page out as normal one
Both of them have its profits and short-comes, however, if the bandwidth is
not limited extremely so that compression can not use out all of it bandwidth,
at the same time, the migration process is easily throttled if we posted too
may pages as normal pages. None of them can work properly under this case
In order to use the bandwidth more effectively, we introduce the third
behavior, adaptive, which make the main thread wait if there is no bandwidth
left or let the page go out as normal page if there has enough bandwidth to
make sure the migration process will not be throttled
Signed-off-by: Xiao Guangrong <xiaoguangrong@xxxxxxxxxxx>
---
hmp.c | 6 ++-
migration/migration.c | 116 ++++++++++++++++++++++++++++++++++++++++++++------
migration/migration.h | 13 ++++++
migration/ram.c | 116 +++++++++++++++++++++++++++++++++++++++++++++-----
qapi/migration.json | 39 +++++++++++------
You neglected to cc: the QAPI schema maintainers.
scripts/get_maintainer.pl can help you find the maintainers to cc: on
your patches.
Thank you for pointing it out, i will pay more attention on it.
5 files changed, 251 insertions(+), 39 deletions(-)
[...]
diff --git a/qapi/migration.json b/qapi/migration.json
index c5babd03b0..0220a0945b 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -93,11 +93,16 @@
#
# @compression-rate: rate of compressed size
#
+# @compress-no-wait-weight: it controls how many pages are directly posted
+# out as normal page when all compression threads are currently busy.
+# Only available if compress-wait-thread = adaptive. (Since 3.2)
"Only available" suggests the member is optional.
+#
# Since: 3.1
##
{ 'struct': 'CompressionStats',
'data': {'pages': 'int', 'busy': 'int', 'busy-rate': 'number',
- 'compressed-size': 'int', 'compression-rate': 'number' } }
+ 'compressed-size': 'int', 'compression-rate': 'number',
+ 'compress-no-wait-weight': 'int'} }
It isn't. Should it be optional? If not, what's its value when
compress-wait-thread isn't adaptive?
It'd be better to make it optional... i will fix it. ：)
##
# @MigrationStatus:
@@ -489,9 +494,13 @@
# the compression thread count is an integer between 1 and 255.
#
# @compress-wait-thread: Controls behavior when all compression threads are
-# currently busy. If true (default), wait for a free
-# compression thread to become available; otherwise,
-# send the page uncompressed. (Since 3.1)
+# currently busy. If 'true/on' (default), wait for a free
+# compression thread to become available; if 'false/off', send the
+# page uncompressed. (Since 3.1)
+# If it is 'adaptive', the behavior is adaptively controlled based on
+# the rate limit. If it has enough bandwidth, it acts as
+# compress-wait-thread is off. (Since 3.2)
+#
#
# @decompress-threads: Set decompression thread count to be used in live
# migration, the decompression thread count is an integer between 1
@@ -577,9 +586,12 @@
# @compress-threads: compression thread count
#
# @compress-wait-thread: Controls behavior when all compression threads are
-# currently busy. If true (default), wait for a free
-# compression thread to become available; otherwise,
-# send the page uncompressed. (Since 3.1)
+# currently busy. If 'true/on' (default), wait for a free
+# compression thread to become available; if 'false/off', send the
+# page uncompressed. (Since 3.1)
+# If it is 'adaptive', the behavior is adaptively controlled based on
+# the rate limit. If it has enough bandwidth, it acts as
+# compress-wait-thread is off. (Since 3.2)
#
# @decompress-threads: decompression thread count
#
@@ -655,7 +667,7 @@
{ 'struct': 'MigrateSetParameters',
'data': { '*compress-level': 'int',
'*compress-threads': 'int',
- '*compress-wait-thread': 'bool',
+ '*compress-wait-thread': 'str',
Compatibility break.
You can add a separate flag like you did in v1 if I understand your cover
letter correctly. Awkward.
You can use a suitable alternate of bool and enum.
‘alternate’ seems a good solution to me, will fix. :)
'str' is not a good idea, because it defeats introspection.
will fix.
'*decompress-threads': 'int',
'*cpu-throttle-initial': 'int',
'*cpu-throttle-increment': 'int',
@@ -697,9 +709,12 @@
# @compress-threads: compression thread count
#
# @compress-wait-thread: Controls behavior when all compression threads are
-# currently busy. If true (default), wait for a free
-# compression thread to become available; otherwise,
-# send the page uncompressed. (Since 3.1)
+# currently busy. If 'true/on' (default), wait for a free
+# compression thread to become available; if 'false/off', send the
+# page uncompressed. (Since 3.1)
+# If it is 'adaptive', the behavior is adaptively controlled based on
+# the rate limit. If it has enough bandwidth, it acts as
+# compress-wait-thread is off. (Since 3.2)
#
# @decompress-threads: decompression thread count
#
@@ -771,7 +786,7 @@
{ 'struct': 'MigrationParameters',
'data': { '*compress-level': 'uint8',
'*compress-threads': 'uint8',
- '*compress-wait-thread': 'bool',
+ '*compress-wait-thread': 'str',
Likewise.
will fix it too.
