* [PATCH v8 1/5] docs/migration: add qatzip compression feature
2024-08-20 17:09 [PATCH v8 0/5] Implement QATzip compression method Yichen Wang
@ 2024-08-20 17:09 ` Yichen Wang
2024-08-22 13:04 ` Fabiano Rosas
2024-08-20 17:09 ` [PATCH v8 2/5] meson: Introduce 'qatzip' feature to the build system Yichen Wang
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Yichen Wang @ 2024-08-20 17:09 UTC (permalink / raw)
To: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel
Cc: Hao Xiang, Liu, Yuan1, Zou, Nanhai, Ho-Ren (Jack) Chuang,
Yichen Wang, Xiaoning Ding
From: Yuan Liu <yuan1.liu@intel.com>
add Intel QATzip compression method introduction
Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
---
docs/devel/migration/features.rst | 1 +
docs/devel/migration/qatzip-compression.rst | 165 ++++++++++++++++++++
2 files changed, 166 insertions(+)
create mode 100644 docs/devel/migration/qatzip-compression.rst
diff --git a/docs/devel/migration/features.rst b/docs/devel/migration/features.rst
index 58f8fd9e16..8f431d52f9 100644
--- a/docs/devel/migration/features.rst
+++ b/docs/devel/migration/features.rst
@@ -14,3 +14,4 @@ Migration has plenty of features to support different use cases.
CPR
qpl-compression
uadk-compression
+ qatzip-compression
diff --git a/docs/devel/migration/qatzip-compression.rst b/docs/devel/migration/qatzip-compression.rst
new file mode 100644
index 0000000000..862b383164
--- /dev/null
+++ b/docs/devel/migration/qatzip-compression.rst
@@ -0,0 +1,165 @@
+==================
+QATzip Compression
+==================
+In scenarios with limited network bandwidth, the ``QATzip`` solution can help
+users save a lot of host CPU resources by accelerating compression and
+decompression through the Intel QuickAssist Technology(``QAT``) hardware.
+
+
+The following test was conducted using 8 multifd channels and 10Gbps network
+bandwidth. The results show that, compared to zstd, ``QATzip`` significantly
+saves CPU resources on the sender and reduces migration time. Compared to the
+uncompressed solution, ``QATzip`` greatly improves the dirty page processing
+capability, indicated by the Pages per Second metric, and also reduces the
+total migration time.
+
+::
+
+ VM Configuration: 16 vCPU and 64G memory
+ VM Workload: all vCPUs are idle and 54G memory is filled with Silesia data.
+ QAT Devices: 4
+ |-----------|--------|---------|----------|----------|------|------|
+ |8 Channels |Total |down |throughput|pages per | send | recv |
+ | |time(ms)|time(ms) |(mbps) |second | cpu %| cpu% |
+ |-----------|--------|---------|----------|----------|------|------|
+ |qatzip | 16630| 28| 10467| 2940235| 160| 360|
+ |-----------|--------|---------|----------|----------|------|------|
+ |zstd | 20165| 24| 8579| 2391465| 810| 340|
+ |-----------|--------|---------|----------|----------|------|------|
+ |none | 46063| 40| 10848| 330240| 45| 85|
+ |-----------|--------|---------|----------|----------|------|------|
+
+
+QATzip Compression Framework
+============================
+
+``QATzip`` is a user space library which builds on top of the Intel QuickAssist
+Technology to provide extended accelerated compression and decompression
+services.
+
+For more ``QATzip`` introduction, please refer to `QATzip Introduction
+<https://github.com/intel/QATzip?tab=readme-ov-file#introductionl>`_
+
+::
+
+ +----------------+
+ | MultiFd Thread |
+ +-------+--------+
+ |
+ | compress/decompress
+ +-------+--------+
+ | QATzip library |
+ +-------+--------+
+ |
+ +-------+--------+
+ | QAT library |
+ +-------+--------+
+ | user space
+ --------+---------------------
+ | kernel space
+ +------+-------+
+ | QAT Driver |
+ +------+-------+
+ |
+ +------+-------+
+ | QAT Devices |
+ +--------------+
+
+
+QATzip Installation
+-------------------
+
+The ``QATzip`` installation package has been integrated into some Linux
+distributions and can be installed directly. For example, the Ubuntu Server
+24.04 LTS system can be installed using below command
+
+.. code-block:: shell
+
+ #apt search qatzip
+ libqatzip-dev/noble 1.2.0-0ubuntu3 amd64
+ Intel QuickAssist user space library development files
+
+ libqatzip3/noble 1.2.0-0ubuntu3 amd64
+ Intel QuickAssist user space library
+
+ qatzip/noble,now 1.2.0-0ubuntu3 amd64 [installed]
+ Compression user-space tool for Intel QuickAssist Technology
+
+ #sudo apt install libqatzip-dev libqatzip3 qatzip
+
+If your system does not support the ``QATzip`` installation package, you can
+use the source code to build and install, please refer to `QATzip source code installation
+<https://github.com/intel/QATzip?tab=readme-ov-file#build-intel-quickassist-technology-driver>`_
+
+QAT Hardware Deployment
+-----------------------
+
+``QAT`` supports physical functions(PFs) and virtual functions(VFs) for
+deployment, and users can configure ``QAT`` resources for migration according
+to actual needs. For more details about ``QAT`` deployment, please refer to
+`Intel QuickAssist Technology Documentation
+<https://intel.github.io/quickassist/index.html>`_
+
+For more ``QAT`` hardware introduction, please refer to `intel-quick-assist-technology-overview
+<https://www.intel.com/content/www/us/en/architecture-and-technology/intel-quick-assist-technology-overview.html>`_
+
+How To Use QATzip Compression
+=============================
+
+1 - Install ``QATzip`` library
+
+2 - Build ``QEMU`` with ``--enable-qatzip`` parameter
+
+ E.g. configure --target-list=x86_64-softmmu --enable-kvm ``--enable-qatzip``
+
+3 - Set ``migrate_set_parameter multifd-compression qatzip``
+
+4 - Set ``migrate_set_parameter multifd-qatzip-level comp_level``, the default
+comp_level value is 1, and it supports levels from 1 to 9
+
+QAT Memory Requirements
+=======================
+
+The user needs to reserve system memory for the QAT memory management to
+allocate DMA memory. The size of the reserved system memory depends on the
+number of devices used for migration and the number of multifd channels.
+
+Because memory usage depends on QAT configuration, please refer to `QAT Memory
+Driver Queries
+<https://intel.github.io/quickassist/PG/infrastructure_debugability.html?highlight=memory>`_
+for memory usage calculation.
+
+.. list-table:: An example of a PF used for migration
+ :header-rows: 1
+
+ * - Number of channels
+ - Sender memory usage
+ - Receiver memory usage
+ * - 2
+ - 10M
+ - 10M
+ * - 4
+ - 12M
+ - 14M
+ * - 8
+ - 16M
+ - 20M
+
+How To Choose Between QATzip and QPL
+====================================
+Starting from 4th Gen Intel Xeon Scalable processors, codenamed Sapphire Rapids
+processor(``SPR``), multiple built-in accelerators are supported including
+``QAT`` and ``IAA``. The former can accelerate ``QATzip`` and the latter is
+used to accelerate ``QPL``.
+
+Here are some suggestions:
+
+1 - If the live migration scenario is limited by network bandwidth and ``QAT``
+hardware resources exceed ``IAA``, use the ``QATzip`` method, which can save a
+lot of host CPU resources for compression.
+
+2 - If the system cannot support shared virtual memory (SVM) technology, use
+the ``QATzip`` method because ``QPL`` performance is not good without SVM
+support.
+
+3 - For other scenarios, use the ``QPL`` method first.
--
Yichen Wang
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v8 1/5] docs/migration: add qatzip compression feature
2024-08-20 17:09 ` [PATCH v8 1/5] docs/migration: add qatzip compression feature Yichen Wang
@ 2024-08-22 13:04 ` Fabiano Rosas
0 siblings, 0 replies; 13+ messages in thread
From: Fabiano Rosas @ 2024-08-22 13:04 UTC (permalink / raw)
To: Yichen Wang, Peter Xu, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel
Cc: Hao Xiang, Liu, Yuan1, Zou, Nanhai, Ho-Ren (Jack) Chuang,
Yichen Wang, Xiaoning Ding
Yichen Wang <yichen.wang@bytedance.com> writes:
> From: Yuan Liu <yuan1.liu@intel.com>
>
> add Intel QATzip compression method introduction
>
> Reviewed-by: Nanhai Zou <nanhai.zou@intel.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
> Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v8 2/5] meson: Introduce 'qatzip' feature to the build system
2024-08-20 17:09 [PATCH v8 0/5] Implement QATzip compression method Yichen Wang
2024-08-20 17:09 ` [PATCH v8 1/5] docs/migration: add qatzip compression feature Yichen Wang
@ 2024-08-20 17:09 ` Yichen Wang
2024-08-20 17:09 ` [PATCH v8 3/5] migration: Add migration parameters for QATzip Yichen Wang
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Yichen Wang @ 2024-08-20 17:09 UTC (permalink / raw)
To: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel
Cc: Hao Xiang, Liu, Yuan1, Zou, Nanhai, Ho-Ren (Jack) Chuang,
Yichen Wang, Xiaoning Ding, Bryan Zhang
From: Bryan Zhang <bryan.zhang@bytedance.com>
Add a 'qatzip' feature, which is automatically disabled, and which
depends on the QATzip library if enabled.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
---
meson.build | 10 ++++++++++
meson_options.txt | 2 ++
scripts/meson-buildoptions.sh | 3 +++
3 files changed, 15 insertions(+)
diff --git a/meson.build b/meson.build
index fbda17c987..b89b713e79 100644
--- a/meson.build
+++ b/meson.build
@@ -1262,6 +1262,14 @@ if not get_option('uadk').auto() or have_system
uadk = declare_dependency(dependencies: [libwd, libwd_comp])
endif
endif
+
+qatzip = not_found
+if not get_option('qatzip').auto() or have_system
+ qatzip = dependency('qatzip', version: '>=1.1.2',
+ required: get_option('qatzip'),
+ method: 'pkg-config')
+endif
+
virgl = not_found
have_vhost_user_gpu = have_tools and host_os == 'linux' and pixman.found()
@@ -2412,6 +2420,7 @@ config_host_data.set('CONFIG_STATX_MNT_ID', has_statx_mnt_id)
config_host_data.set('CONFIG_ZSTD', zstd.found())
config_host_data.set('CONFIG_QPL', qpl.found())
config_host_data.set('CONFIG_UADK', uadk.found())
+config_host_data.set('CONFIG_QATZIP', qatzip.found())
config_host_data.set('CONFIG_FUSE', fuse.found())
config_host_data.set('CONFIG_FUSE_LSEEK', fuse_lseek.found())
config_host_data.set('CONFIG_SPICE_PROTOCOL', spice_protocol.found())
@@ -4535,6 +4544,7 @@ summary_info += {'lzfse support': liblzfse}
summary_info += {'zstd support': zstd}
summary_info += {'Query Processing Library support': qpl}
summary_info += {'UADK Library support': uadk}
+summary_info += {'qatzip support': qatzip}
summary_info += {'NUMA host support': numa}
summary_info += {'capstone': capstone}
summary_info += {'libpmem support': libpmem}
diff --git a/meson_options.txt b/meson_options.txt
index 0269fa0f16..f7b652b30d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -261,6 +261,8 @@ option('qpl', type : 'feature', value : 'auto',
description: 'Query Processing Library support')
option('uadk', type : 'feature', value : 'auto',
description: 'UADK Library support')
+option('qatzip', type: 'feature', value: 'auto',
+ description: 'QATzip compression support')
option('fuse', type: 'feature', value: 'auto',
description: 'FUSE block device export')
option('fuse_lseek', type : 'feature', value : 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index c97079a38c..5f377a6d81 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -163,6 +163,7 @@ meson_options_help() {
printf "%s\n" ' pixman pixman support'
printf "%s\n" ' plugins TCG plugins via shared library loading'
printf "%s\n" ' png PNG support with libpng'
+ printf "%s\n" ' qatzip QATzip compression support'
printf "%s\n" ' qcow1 qcow1 image format support'
printf "%s\n" ' qed qed image format support'
printf "%s\n" ' qga-vss build QGA VSS support (broken with MinGW)'
@@ -427,6 +428,8 @@ _meson_option_parse() {
--enable-png) printf "%s" -Dpng=enabled ;;
--disable-png) printf "%s" -Dpng=disabled ;;
--prefix=*) quote_sh "-Dprefix=$2" ;;
+ --enable-qatzip) printf "%s" -Dqatzip=enabled ;;
+ --disable-qatzip) printf "%s" -Dqatzip=disabled ;;
--enable-qcow1) printf "%s" -Dqcow1=enabled ;;
--disable-qcow1) printf "%s" -Dqcow1=disabled ;;
--enable-qed) printf "%s" -Dqed=enabled ;;
--
Yichen Wang
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v8 3/5] migration: Add migration parameters for QATzip
2024-08-20 17:09 [PATCH v8 0/5] Implement QATzip compression method Yichen Wang
2024-08-20 17:09 ` [PATCH v8 1/5] docs/migration: add qatzip compression feature Yichen Wang
2024-08-20 17:09 ` [PATCH v8 2/5] meson: Introduce 'qatzip' feature to the build system Yichen Wang
@ 2024-08-20 17:09 ` Yichen Wang
2024-08-21 11:56 ` Prasad Pandit
2024-08-20 17:09 ` [PATCH v8 4/5] migration: Introduce 'qatzip' compression method Yichen Wang
2024-08-20 17:09 ` [PATCH v8 5/5] tests/migration: Add integration test for " Yichen Wang
4 siblings, 1 reply; 13+ messages in thread
From: Yichen Wang @ 2024-08-20 17:09 UTC (permalink / raw)
To: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel
Cc: Hao Xiang, Liu, Yuan1, Zou, Nanhai, Ho-Ren (Jack) Chuang,
Yichen Wang, Xiaoning Ding, Bryan Zhang
From: Bryan Zhang <bryan.zhang@bytedance.com>
Adds support for migration parameters to control QATzip compression
level and to enable/disable software fallback when QAT hardware is
unavailable. This is a preparatory commit for a subsequent commit that
will actually use QATzip compression.
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
---
migration/migration-hmp-cmds.c | 4 ++++
migration/options.c | 34 ++++++++++++++++++++++++++++++++++
migration/options.h | 1 +
qapi/migration.json | 18 ++++++++++++++++++
4 files changed, 57 insertions(+)
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 7d608d26e1..28165cfc9e 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -576,6 +576,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
p->has_multifd_zlib_level = true;
visit_type_uint8(v, param, &p->multifd_zlib_level, &err);
break;
+ case MIGRATION_PARAMETER_MULTIFD_QATZIP_LEVEL:
+ p->has_multifd_qatzip_level = true;
+ visit_type_uint8(v, param, &p->multifd_qatzip_level, &err);
+ break;
case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
p->has_multifd_zstd_level = true;
visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
diff --git a/migration/options.c b/migration/options.c
index 645f55003d..147cd2b8fd 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -55,6 +55,13 @@
#define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
/* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
#define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
+/*
+ * 1: best speed, ... 9: best compress ratio
+ * There is some nuance here. Refer to QATzip documentation to understand
+ * the mapping of QATzip levels to standard deflate levels.
+ */
+#define DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL 1
+
/* 0: means nocompress, 1: best speed, ... 20: best compress ratio */
#define DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL 1
@@ -123,6 +130,9 @@ Property migration_properties[] = {
DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
parameters.multifd_zlib_level,
DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
+ DEFINE_PROP_UINT8("multifd-qatzip-level", MigrationState,
+ parameters.multifd_qatzip_level,
+ DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL),
DEFINE_PROP_UINT8("multifd-zstd-level", MigrationState,
parameters.multifd_zstd_level,
DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL),
@@ -787,6 +797,13 @@ int migrate_multifd_zlib_level(void)
return s->parameters.multifd_zlib_level;
}
+int migrate_multifd_qatzip_level(void)
+{
+ MigrationState *s = migrate_get_current();
+
+ return s->parameters.multifd_qatzip_level;
+}
+
int migrate_multifd_zstd_level(void)
{
MigrationState *s = migrate_get_current();
@@ -892,6 +909,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->multifd_compression = s->parameters.multifd_compression;
params->has_multifd_zlib_level = true;
params->multifd_zlib_level = s->parameters.multifd_zlib_level;
+ params->has_multifd_qatzip_level = true;
+ params->multifd_qatzip_level = s->parameters.multifd_qatzip_level;
params->has_multifd_zstd_level = true;
params->multifd_zstd_level = s->parameters.multifd_zstd_level;
params->has_xbzrle_cache_size = true;
@@ -946,6 +965,7 @@ void migrate_params_init(MigrationParameters *params)
params->has_multifd_channels = true;
params->has_multifd_compression = true;
params->has_multifd_zlib_level = true;
+ params->has_multifd_qatzip_level = true;
params->has_multifd_zstd_level = true;
params->has_xbzrle_cache_size = true;
params->has_max_postcopy_bandwidth = true;
@@ -1038,6 +1058,14 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
+ if (params->has_multifd_qatzip_level &&
+ ((params->multifd_qatzip_level > 9) ||
+ (params->multifd_qatzip_level < 1))) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_qatzip_level",
+ "a value between 1 and 9");
+ return false;
+ }
+
if (params->has_multifd_zstd_level &&
(params->multifd_zstd_level > 20)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
@@ -1195,6 +1223,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
if (params->has_multifd_compression) {
dest->multifd_compression = params->multifd_compression;
}
+ if (params->has_multifd_qatzip_level) {
+ dest->multifd_qatzip_level = params->multifd_qatzip_level;
+ }
if (params->has_multifd_zlib_level) {
dest->multifd_zlib_level = params->multifd_zlib_level;
}
@@ -1315,6 +1346,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
if (params->has_multifd_compression) {
s->parameters.multifd_compression = params->multifd_compression;
}
+ if (params->has_multifd_qatzip_level) {
+ s->parameters.multifd_qatzip_level = params->multifd_qatzip_level;
+ }
if (params->has_multifd_zlib_level) {
s->parameters.multifd_zlib_level = params->multifd_zlib_level;
}
diff --git a/migration/options.h b/migration/options.h
index a2397026db..a0bd6edc06 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -78,6 +78,7 @@ uint64_t migrate_max_postcopy_bandwidth(void);
int migrate_multifd_channels(void);
MultiFDCompression migrate_multifd_compression(void);
int migrate_multifd_zlib_level(void);
+int migrate_multifd_qatzip_level(void);
int migrate_multifd_zstd_level(void);
uint8_t migrate_throttle_trigger_threshold(void);
const char *migrate_tls_authz(void);
diff --git a/qapi/migration.json b/qapi/migration.json
index 7324571e92..f4c27426c8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -792,6 +792,11 @@
# speed, and 9 means best compression ratio which will consume
# more CPU. Defaults to 1. (Since 5.0)
#
+# @multifd-qatzip-level: Set the compression level to be used in live
+# migration. The level is an integer between 1 and 9, where 1 means
+# the best compression speed, and 9 means the best compression
+# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
+#
# @multifd-zstd-level: Set the compression level to be used in live
# migration, the compression level is an integer between 0 and 20,
# where 0 means no compression, 1 means the best compression
@@ -852,6 +857,7 @@
'xbzrle-cache-size', 'max-postcopy-bandwidth',
'max-cpu-throttle', 'multifd-compression',
'multifd-zlib-level', 'multifd-zstd-level',
+ 'multifd-qatzip-level',
'block-bitmap-mapping',
{ 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
'vcpu-dirty-limit',
@@ -967,6 +973,11 @@
# speed, and 9 means best compression ratio which will consume
# more CPU. Defaults to 1. (Since 5.0)
#
+# @multifd-qatzip-level: Set the compression level to be used in live
+# migration. The level is an integer between 1 and 9, where 1 means
+# the best compression speed, and 9 means the best compression
+# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
+#
# @multifd-zstd-level: Set the compression level to be used in live
# migration, the compression level is an integer between 0 and 20,
# where 0 means no compression, 1 means the best compression
@@ -1040,6 +1051,7 @@
'*max-cpu-throttle': 'uint8',
'*multifd-compression': 'MultiFDCompression',
'*multifd-zlib-level': 'uint8',
+ '*multifd-qatzip-level': 'uint8',
'*multifd-zstd-level': 'uint8',
'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
'*x-vcpu-dirty-limit-period': { 'type': 'uint64',
@@ -1171,6 +1183,11 @@
# speed, and 9 means best compression ratio which will consume
# more CPU. Defaults to 1. (Since 5.0)
#
+# @multifd-qatzip-level: Set the compression level to be used in live
+# migration. The level is an integer between 1 and 9, where 1 means
+# the best compression speed, and 9 means the best compression
+# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
+#
# @multifd-zstd-level: Set the compression level to be used in live
# migration, the compression level is an integer between 0 and 20,
# where 0 means no compression, 1 means the best compression
@@ -1241,6 +1258,7 @@
'*max-cpu-throttle': 'uint8',
'*multifd-compression': 'MultiFDCompression',
'*multifd-zlib-level': 'uint8',
+ '*multifd-qatzip-level': 'uint8',
'*multifd-zstd-level': 'uint8',
'*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
'*x-vcpu-dirty-limit-period': { 'type': 'uint64',
--
Yichen Wang
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v8 3/5] migration: Add migration parameters for QATzip
2024-08-20 17:09 ` [PATCH v8 3/5] migration: Add migration parameters for QATzip Yichen Wang
@ 2024-08-21 11:56 ` Prasad Pandit
2024-08-21 20:42 ` [External] " Yichen Wang
0 siblings, 1 reply; 13+ messages in thread
From: Prasad Pandit @ 2024-08-21 11:56 UTC (permalink / raw)
To: Yichen Wang
Cc: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel, Hao Xiang, Liu, Yuan1, Zou, Nanhai,
Ho-Ren (Jack) Chuang, Xiaoning Ding, Bryan Zhang
On Tue, 20 Aug 2024 at 22:40, Yichen Wang <yichen.wang@bytedance.com> wrote:
> Adds support for migration parameters to control QATzip compression
> level and to enable/disable software fallback when QAT hardware is
> unavailable. This is a preparatory commit for a subsequent commit that
> will actually use QATzip compression.
* Is the check whether "QAT hardware is available" happening in this
patch? (I couldn't spot it).
* The informatory notice "This is a preparatory commit for..." could
be moved to the cover-letter, instead of commit message. (Not sure how
it helps to log it in git history)
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
> Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
> Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
> ---
> migration/migration-hmp-cmds.c | 4 ++++
> migration/options.c | 34 ++++++++++++++++++++++++++++++++++
> migration/options.h | 1 +
> qapi/migration.json | 18 ++++++++++++++++++
> 4 files changed, 57 insertions(+)
>
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 7d608d26e1..28165cfc9e 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -576,6 +576,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
> p->has_multifd_zlib_level = true;
> visit_type_uint8(v, param, &p->multifd_zlib_level, &err);
> break;
> + case MIGRATION_PARAMETER_MULTIFD_QATZIP_LEVEL:
> + p->has_multifd_qatzip_level = true;
> + visit_type_uint8(v, param, &p->multifd_qatzip_level, &err);
> + break;
> case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
> p->has_multifd_zstd_level = true;
> visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
> diff --git a/migration/options.c b/migration/options.c
> index 645f55003d..147cd2b8fd 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -55,6 +55,13 @@
> #define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
> /* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
> #define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
> +/*
> + * 1: best speed, ... 9: best compress ratio
> + * There is some nuance here. Refer to QATzip documentation to understand
> + * the mapping of QATzip levels to standard deflate levels.
> + */
> +#define DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL 1
> +
> /* 0: means nocompress, 1: best speed, ... 20: best compress ratio */
> #define DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL 1
>
> @@ -123,6 +130,9 @@ Property migration_properties[] = {
> DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
> parameters.multifd_zlib_level,
> DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
> + DEFINE_PROP_UINT8("multifd-qatzip-level", MigrationState,
> + parameters.multifd_qatzip_level,
> + DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL),
> DEFINE_PROP_UINT8("multifd-zstd-level", MigrationState,
> parameters.multifd_zstd_level,
> DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL),
> @@ -787,6 +797,13 @@ int migrate_multifd_zlib_level(void)
> return s->parameters.multifd_zlib_level;
> }
>
> +int migrate_multifd_qatzip_level(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return s->parameters.multifd_qatzip_level;
> +}
> +
> int migrate_multifd_zstd_level(void)
> {
> MigrationState *s = migrate_get_current();
> @@ -892,6 +909,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> params->multifd_compression = s->parameters.multifd_compression;
> params->has_multifd_zlib_level = true;
> params->multifd_zlib_level = s->parameters.multifd_zlib_level;
> + params->has_multifd_qatzip_level = true;
> + params->multifd_qatzip_level = s->parameters.multifd_qatzip_level;
> params->has_multifd_zstd_level = true;
> params->multifd_zstd_level = s->parameters.multifd_zstd_level;
> params->has_xbzrle_cache_size = true;
> @@ -946,6 +965,7 @@ void migrate_params_init(MigrationParameters *params)
> params->has_multifd_channels = true;
> params->has_multifd_compression = true;
> params->has_multifd_zlib_level = true;
> + params->has_multifd_qatzip_level = true;
> params->has_multifd_zstd_level = true;
> params->has_xbzrle_cache_size = true;
> params->has_max_postcopy_bandwidth = true;
> @@ -1038,6 +1058,14 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
> return false;
> }
>
> + if (params->has_multifd_qatzip_level &&
> + ((params->multifd_qatzip_level > 9) ||
> + (params->multifd_qatzip_level < 1))) {
> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_qatzip_level",
> + "a value between 1 and 9");
> + return false;
> + }
> +
> if (params->has_multifd_zstd_level &&
> (params->multifd_zstd_level > 20)) {
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
> @@ -1195,6 +1223,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
> if (params->has_multifd_compression) {
> dest->multifd_compression = params->multifd_compression;
> }
> + if (params->has_multifd_qatzip_level) {
> + dest->multifd_qatzip_level = params->multifd_qatzip_level;
> + }
> if (params->has_multifd_zlib_level) {
> dest->multifd_zlib_level = params->multifd_zlib_level;
> }
> @@ -1315,6 +1346,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> if (params->has_multifd_compression) {
> s->parameters.multifd_compression = params->multifd_compression;
> }
> + if (params->has_multifd_qatzip_level) {
> + s->parameters.multifd_qatzip_level = params->multifd_qatzip_level;
> + }
> if (params->has_multifd_zlib_level) {
> s->parameters.multifd_zlib_level = params->multifd_zlib_level;
> }
> diff --git a/migration/options.h b/migration/options.h
> index a2397026db..a0bd6edc06 100644
> --- a/migration/options.h
> +++ b/migration/options.h
> @@ -78,6 +78,7 @@ uint64_t migrate_max_postcopy_bandwidth(void);
> int migrate_multifd_channels(void);
> MultiFDCompression migrate_multifd_compression(void);
> int migrate_multifd_zlib_level(void);
> +int migrate_multifd_qatzip_level(void);
> int migrate_multifd_zstd_level(void);
> uint8_t migrate_throttle_trigger_threshold(void);
> const char *migrate_tls_authz(void);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7324571e92..f4c27426c8 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -792,6 +792,11 @@
> # speed, and 9 means best compression ratio which will consume
> # more CPU. Defaults to 1. (Since 5.0)
> #
> +# @multifd-qatzip-level: Set the compression level to be used in live
> +# migration. The level is an integer between 1 and 9, where 1 means
> +# the best compression speed, and 9 means the best compression
> +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> +#
> # @multifd-zstd-level: Set the compression level to be used in live
> # migration, the compression level is an integer between 0 and 20,
> # where 0 means no compression, 1 means the best compression
> @@ -852,6 +857,7 @@
> 'xbzrle-cache-size', 'max-postcopy-bandwidth',
> 'max-cpu-throttle', 'multifd-compression',
> 'multifd-zlib-level', 'multifd-zstd-level',
> + 'multifd-qatzip-level',
> 'block-bitmap-mapping',
> { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
> 'vcpu-dirty-limit',
> @@ -967,6 +973,11 @@
> # speed, and 9 means best compression ratio which will consume
> # more CPU. Defaults to 1. (Since 5.0)
> #
> +# @multifd-qatzip-level: Set the compression level to be used in live
> +# migration. The level is an integer between 1 and 9, where 1 means
> +# the best compression speed, and 9 means the best compression
> +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> +#
> # @multifd-zstd-level: Set the compression level to be used in live
> # migration, the compression level is an integer between 0 and 20,
> # where 0 means no compression, 1 means the best compression
> @@ -1040,6 +1051,7 @@
> '*max-cpu-throttle': 'uint8',
> '*multifd-compression': 'MultiFDCompression',
> '*multifd-zlib-level': 'uint8',
> + '*multifd-qatzip-level': 'uint8',
> '*multifd-zstd-level': 'uint8',
> '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
> '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
> @@ -1171,6 +1183,11 @@
> # speed, and 9 means best compression ratio which will consume
> # more CPU. Defaults to 1. (Since 5.0)
> #
> +# @multifd-qatzip-level: Set the compression level to be used in live
> +# migration. The level is an integer between 1 and 9, where 1 means
> +# the best compression speed, and 9 means the best compression
> +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> +#
> # @multifd-zstd-level: Set the compression level to be used in live
> # migration, the compression level is an integer between 0 and 20,
> # where 0 means no compression, 1 means the best compression
> @@ -1241,6 +1258,7 @@
> '*max-cpu-throttle': 'uint8',
> '*multifd-compression': 'MultiFDCompression',
> '*multifd-zlib-level': 'uint8',
> + '*multifd-qatzip-level': 'uint8',
> '*multifd-zstd-level': 'uint8',
> '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
> '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
> --
> Yichen Wang
'multifd-qatzip-level' related changes look okay.
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Thank you.
---
- Prasad
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [External] Re: [PATCH v8 3/5] migration: Add migration parameters for QATzip
2024-08-21 11:56 ` Prasad Pandit
@ 2024-08-21 20:42 ` Yichen Wang
2024-08-22 10:23 ` Prasad Pandit
0 siblings, 1 reply; 13+ messages in thread
From: Yichen Wang @ 2024-08-21 20:42 UTC (permalink / raw)
To: Prasad Pandit
Cc: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel, Hao Xiang, Liu, Yuan1, Zou, Nanhai,
Ho-Ren (Jack) Chuang, Xiaoning Ding, Bryan Zhang
On Wed, Aug 21, 2024 at 4:56 AM Prasad Pandit <ppandit@redhat.com> wrote:
>
> On Tue, 20 Aug 2024 at 22:40, Yichen Wang <yichen.wang@bytedance.com> wrote:
> > Adds support for migration parameters to control QATzip compression
> > level and to enable/disable software fallback when QAT hardware is
> > unavailable. This is a preparatory commit for a subsequent commit that
> > will actually use QATzip compression.
>
> * Is the check whether "QAT hardware is available" happening in this
> patch? (I couldn't spot it).
After discussing with Intel folks, I decided to align to the existing
QPL behavior. In QPL, the code path of compression will always go
through regardless. When acceleration hardware is initialized
properly, use it. If failed, fallback to software path automatically.
So in this QAT case, I do the same. The line of "ret =
qzInit(&q->sess, true);" will do the auto software fallback.
> * The informatory notice "This is a preparatory commit for..." could
> be moved to the cover-letter, instead of commit message. (Not sure how
> it helps to log it in git history)
>
Oh, this line is purely for breaking the big patch into two commits.
This one plus the following commit [4/5] together implements the full
feature. I can remove this from the commit message if you prefer.
> > Acked-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
> > Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
> > Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
> > ---
> > migration/migration-hmp-cmds.c | 4 ++++
> > migration/options.c | 34 ++++++++++++++++++++++++++++++++++
> > migration/options.h | 1 +
> > qapi/migration.json | 18 ++++++++++++++++++
> > 4 files changed, 57 insertions(+)
> >
> > diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> > index 7d608d26e1..28165cfc9e 100644
> > --- a/migration/migration-hmp-cmds.c
> > +++ b/migration/migration-hmp-cmds.c
> > @@ -576,6 +576,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
> > p->has_multifd_zlib_level = true;
> > visit_type_uint8(v, param, &p->multifd_zlib_level, &err);
> > break;
> > + case MIGRATION_PARAMETER_MULTIFD_QATZIP_LEVEL:
> > + p->has_multifd_qatzip_level = true;
> > + visit_type_uint8(v, param, &p->multifd_qatzip_level, &err);
> > + break;
> > case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
> > p->has_multifd_zstd_level = true;
> > visit_type_uint8(v, param, &p->multifd_zstd_level, &err);
> > diff --git a/migration/options.c b/migration/options.c
> > index 645f55003d..147cd2b8fd 100644
> > --- a/migration/options.c
> > +++ b/migration/options.c
> > @@ -55,6 +55,13 @@
> > #define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
> > /* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
> > #define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
> > +/*
> > + * 1: best speed, ... 9: best compress ratio
> > + * There is some nuance here. Refer to QATzip documentation to understand
> > + * the mapping of QATzip levels to standard deflate levels.
> > + */
> > +#define DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL 1
> > +
> > /* 0: means nocompress, 1: best speed, ... 20: best compress ratio */
> > #define DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL 1
> >
> > @@ -123,6 +130,9 @@ Property migration_properties[] = {
> > DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
> > parameters.multifd_zlib_level,
> > DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
> > + DEFINE_PROP_UINT8("multifd-qatzip-level", MigrationState,
> > + parameters.multifd_qatzip_level,
> > + DEFAULT_MIGRATE_MULTIFD_QATZIP_LEVEL),
> > DEFINE_PROP_UINT8("multifd-zstd-level", MigrationState,
> > parameters.multifd_zstd_level,
> > DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL),
> > @@ -787,6 +797,13 @@ int migrate_multifd_zlib_level(void)
> > return s->parameters.multifd_zlib_level;
> > }
> >
> > +int migrate_multifd_qatzip_level(void)
> > +{
> > + MigrationState *s = migrate_get_current();
> > +
> > + return s->parameters.multifd_qatzip_level;
> > +}
> > +
> > int migrate_multifd_zstd_level(void)
> > {
> > MigrationState *s = migrate_get_current();
> > @@ -892,6 +909,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> > params->multifd_compression = s->parameters.multifd_compression;
> > params->has_multifd_zlib_level = true;
> > params->multifd_zlib_level = s->parameters.multifd_zlib_level;
> > + params->has_multifd_qatzip_level = true;
> > + params->multifd_qatzip_level = s->parameters.multifd_qatzip_level;
> > params->has_multifd_zstd_level = true;
> > params->multifd_zstd_level = s->parameters.multifd_zstd_level;
> > params->has_xbzrle_cache_size = true;
> > @@ -946,6 +965,7 @@ void migrate_params_init(MigrationParameters *params)
> > params->has_multifd_channels = true;
> > params->has_multifd_compression = true;
> > params->has_multifd_zlib_level = true;
> > + params->has_multifd_qatzip_level = true;
> > params->has_multifd_zstd_level = true;
> > params->has_xbzrle_cache_size = true;
> > params->has_max_postcopy_bandwidth = true;
> > @@ -1038,6 +1058,14 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
> > return false;
> > }
> >
> > + if (params->has_multifd_qatzip_level &&
> > + ((params->multifd_qatzip_level > 9) ||
> > + (params->multifd_qatzip_level < 1))) {
> > + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_qatzip_level",
> > + "a value between 1 and 9");
> > + return false;
> > + }
> > +
> > if (params->has_multifd_zstd_level &&
> > (params->multifd_zstd_level > 20)) {
> > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
> > @@ -1195,6 +1223,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
> > if (params->has_multifd_compression) {
> > dest->multifd_compression = params->multifd_compression;
> > }
> > + if (params->has_multifd_qatzip_level) {
> > + dest->multifd_qatzip_level = params->multifd_qatzip_level;
> > + }
> > if (params->has_multifd_zlib_level) {
> > dest->multifd_zlib_level = params->multifd_zlib_level;
> > }
> > @@ -1315,6 +1346,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> > if (params->has_multifd_compression) {
> > s->parameters.multifd_compression = params->multifd_compression;
> > }
> > + if (params->has_multifd_qatzip_level) {
> > + s->parameters.multifd_qatzip_level = params->multifd_qatzip_level;
> > + }
> > if (params->has_multifd_zlib_level) {
> > s->parameters.multifd_zlib_level = params->multifd_zlib_level;
> > }
> > diff --git a/migration/options.h b/migration/options.h
> > index a2397026db..a0bd6edc06 100644
> > --- a/migration/options.h
> > +++ b/migration/options.h
> > @@ -78,6 +78,7 @@ uint64_t migrate_max_postcopy_bandwidth(void);
> > int migrate_multifd_channels(void);
> > MultiFDCompression migrate_multifd_compression(void);
> > int migrate_multifd_zlib_level(void);
> > +int migrate_multifd_qatzip_level(void);
> > int migrate_multifd_zstd_level(void);
> > uint8_t migrate_throttle_trigger_threshold(void);
> > const char *migrate_tls_authz(void);
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 7324571e92..f4c27426c8 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -792,6 +792,11 @@
> > # speed, and 9 means best compression ratio which will consume
> > # more CPU. Defaults to 1. (Since 5.0)
> > #
> > +# @multifd-qatzip-level: Set the compression level to be used in live
> > +# migration. The level is an integer between 1 and 9, where 1 means
> > +# the best compression speed, and 9 means the best compression
> > +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> > +#
> > # @multifd-zstd-level: Set the compression level to be used in live
> > # migration, the compression level is an integer between 0 and 20,
> > # where 0 means no compression, 1 means the best compression
> > @@ -852,6 +857,7 @@
> > 'xbzrle-cache-size', 'max-postcopy-bandwidth',
> > 'max-cpu-throttle', 'multifd-compression',
> > 'multifd-zlib-level', 'multifd-zstd-level',
> > + 'multifd-qatzip-level',
> > 'block-bitmap-mapping',
> > { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] },
> > 'vcpu-dirty-limit',
> > @@ -967,6 +973,11 @@
> > # speed, and 9 means best compression ratio which will consume
> > # more CPU. Defaults to 1. (Since 5.0)
> > #
> > +# @multifd-qatzip-level: Set the compression level to be used in live
> > +# migration. The level is an integer between 1 and 9, where 1 means
> > +# the best compression speed, and 9 means the best compression
> > +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> > +#
> > # @multifd-zstd-level: Set the compression level to be used in live
> > # migration, the compression level is an integer between 0 and 20,
> > # where 0 means no compression, 1 means the best compression
> > @@ -1040,6 +1051,7 @@
> > '*max-cpu-throttle': 'uint8',
> > '*multifd-compression': 'MultiFDCompression',
> > '*multifd-zlib-level': 'uint8',
> > + '*multifd-qatzip-level': 'uint8',
> > '*multifd-zstd-level': 'uint8',
> > '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
> > '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
> > @@ -1171,6 +1183,11 @@
> > # speed, and 9 means best compression ratio which will consume
> > # more CPU. Defaults to 1. (Since 5.0)
> > #
> > +# @multifd-qatzip-level: Set the compression level to be used in live
> > +# migration. The level is an integer between 1 and 9, where 1 means
> > +# the best compression speed, and 9 means the best compression
> > +# ratio which will consume more CPU. Defaults to 1. (Since 9.2)
> > +#
> > # @multifd-zstd-level: Set the compression level to be used in live
> > # migration, the compression level is an integer between 0 and 20,
> > # where 0 means no compression, 1 means the best compression
> > @@ -1241,6 +1258,7 @@
> > '*max-cpu-throttle': 'uint8',
> > '*multifd-compression': 'MultiFDCompression',
> > '*multifd-zlib-level': 'uint8',
> > + '*multifd-qatzip-level': 'uint8',
> > '*multifd-zstd-level': 'uint8',
> > '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
> > '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
> > --
> > Yichen Wang
>
> 'multifd-qatzip-level' related changes look okay.
> Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
>
> Thank you.
> ---
> - Prasad
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [External] Re: [PATCH v8 3/5] migration: Add migration parameters for QATzip
2024-08-21 20:42 ` [External] " Yichen Wang
@ 2024-08-22 10:23 ` Prasad Pandit
0 siblings, 0 replies; 13+ messages in thread
From: Prasad Pandit @ 2024-08-22 10:23 UTC (permalink / raw)
To: Yichen Wang
Cc: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel, Hao Xiang, Liu, Yuan1, Zou, Nanhai,
Ho-Ren (Jack) Chuang, Xiaoning Ding, Bryan Zhang
Hi,
On Thu, 22 Aug 2024 at 02:13, Yichen Wang <yichen.wang@bytedance.com> wrote:
> After discussing with Intel folks, I decided to align to the existing
> QPL behavior. In QPL, the code path of compression will always go
> through regardless. When acceleration hardware is initialized
> properly, use it. If failed, fallback to software path automatically.
> So in this QAT case, I do the same. The line of "ret =
> qzInit(&q->sess, true);" will do the auto software fallback.
* I see.
> Oh, this line is purely for breaking the big patch into two commits.
> This one plus the following commit [4/5] together implements the full
> feature. I can remove this from the commit message if you prefer.
* Yes, that'll be nice.
Thank you.
---
- Prasad
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v8 4/5] migration: Introduce 'qatzip' compression method
2024-08-20 17:09 [PATCH v8 0/5] Implement QATzip compression method Yichen Wang
` (2 preceding siblings ...)
2024-08-20 17:09 ` [PATCH v8 3/5] migration: Add migration parameters for QATzip Yichen Wang
@ 2024-08-20 17:09 ` Yichen Wang
2024-08-22 11:06 ` Prasad Pandit
2024-08-20 17:09 ` [PATCH v8 5/5] tests/migration: Add integration test for " Yichen Wang
4 siblings, 1 reply; 13+ messages in thread
From: Yichen Wang @ 2024-08-20 17:09 UTC (permalink / raw)
To: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel
Cc: Hao Xiang, Liu, Yuan1, Zou, Nanhai, Ho-Ren (Jack) Chuang,
Yichen Wang, Xiaoning Ding, Bryan Zhang
From: Bryan Zhang <bryan.zhang@bytedance.com>
Adds support for 'qatzip' as an option for the multifd compression
method parameter, and implements using QAT for 'qatzip' compression and
decompression.
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
---
hw/core/qdev-properties-system.c | 2 +-
migration/meson.build | 1 +
migration/multifd-qatzip.c | 394 +++++++++++++++++++++++++++++++
migration/multifd.h | 5 +-
qapi/migration.json | 3 +
5 files changed, 402 insertions(+), 3 deletions(-)
create mode 100644 migration/multifd-qatzip.c
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index f13350b4fb..a56fbf728d 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -659,7 +659,7 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
const PropertyInfo qdev_prop_multifd_compression = {
.name = "MultiFDCompression",
.description = "multifd_compression values, "
- "none/zlib/zstd/qpl/uadk",
+ "none/zlib/zstd/qpl/uadk/qatzip",
.enum_table = &MultiFDCompression_lookup,
.get = qdev_propinfo_get_enum,
.set = qdev_propinfo_set_enum,
diff --git a/migration/meson.build b/migration/meson.build
index 5ce2acb41e..c9454c26ae 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -41,6 +41,7 @@ system_ss.add(when: rdma, if_true: files('rdma.c'))
system_ss.add(when: zstd, if_true: files('multifd-zstd.c'))
system_ss.add(when: qpl, if_true: files('multifd-qpl.c'))
system_ss.add(when: uadk, if_true: files('multifd-uadk.c'))
+system_ss.add(when: qatzip, if_true: files('multifd-qatzip.c'))
specific_ss.add(when: 'CONFIG_SYSTEM_ONLY',
if_true: files('ram.c',
diff --git a/migration/multifd-qatzip.c b/migration/multifd-qatzip.c
new file mode 100644
index 0000000000..3c787ed879
--- /dev/null
+++ b/migration/multifd-qatzip.c
@@ -0,0 +1,394 @@
+/*
+ * Multifd QATzip compression implementation
+ *
+ * Copyright (c) Bytedance
+ *
+ * Authors:
+ * Bryan Zhang <bryan.zhang@bytedance.com>
+ * Hao Xiang <hao.xiang@bytedance.com>
+ * Yichen Wang <yichen.wang@bytedance.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "exec/ramblock.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qapi/qapi-types-migration.h"
+#include "options.h"
+#include "multifd.h"
+#include <qatzip.h>
+
+typedef struct {
+ /*
+ * Unique session for use with QATzip API
+ */
+ QzSession_T sess;
+
+ /*
+ * For compression: Buffer for pages to compress
+ * For decompression: Buffer for data to decompress
+ */
+ uint8_t *in_buf;
+ uint32_t in_len;
+
+ /*
+ * For compression: Output buffer of compressed data
+ * For decompression: Output buffer of decompressed data
+ */
+ uint8_t *out_buf;
+ uint32_t out_len;
+} QatzipData;
+
+/**
+ * qatzip_send_setup: Set up QATzip session and private buffers.
+ *
+ * @param p Multifd channel params
+ * @param errp Pointer to error, which will be set in case of error
+ * @return 0 on success, -1 on error (and *errp will be set)
+ */
+static int qatzip_send_setup(MultiFDSendParams *p, Error **errp)
+{
+ QatzipData *q;
+ QzSessionParamsDeflate_T params;
+ const char *err_msg;
+ int ret;
+
+ q = g_new0(QatzipData, 1);
+ p->compress_data = q;
+ /* We need one extra place for the packet header */
+ p->iov = g_new0(struct iovec, 2);
+
+ /*
+ * Initialize QAT device with software fallback by default. This allows
+ * QATzip to use CPU path when QAT hardware reaches maximum throughput.
+ */
+ ret = qzInit(&q->sess, true);
+ if (ret != QZ_OK && ret != QZ_DUPLICATE) {
+ err_msg = "qzInit failed";
+ goto err;
+ }
+
+ ret = qzGetDefaultsDeflate(¶ms);
+ if (ret != QZ_OK) {
+ err_msg = "qzGetDefaultsDeflate failed";
+ goto err;
+ }
+
+ /* Make sure to use configured QATzip compression level. */
+ params.common_params.comp_lvl = migrate_multifd_qatzip_level();
+ ret = qzSetupSessionDeflate(&q->sess, ¶ms);
+ if (ret != QZ_OK && ret != QZ_DUPLICATE) {
+ err_msg = "qzSetupSessionDeflate failed";
+ goto err;
+ }
+
+ if (MULTIFD_PACKET_SIZE > UINT32_MAX) {
+ err_msg = "packet size too large for QAT";
+ goto err;
+ }
+
+ q->in_len = MULTIFD_PACKET_SIZE;
+ /*
+ * PINNED_MEM is an enum from qatzip headers, which means to use
+ * kzalloc_node() to allocate memory for QAT DMA purposes. When QAT device
+ * is not available or software fallback is used, the malloc flag needs to
+ * be set as COMMON_MEM.
+ */
+ q->in_buf = qzMalloc(q->in_len, 0, PINNED_MEM);
+ if (!q->in_buf) {
+ q->in_buf = qzMalloc(q->in_len, 0, COMMON_MEM);
+ if (!q->in_buf) {
+ err_msg = "qzMalloc failed";
+ goto err;
+ }
+ }
+
+ q->out_len = qzMaxCompressedLength(MULTIFD_PACKET_SIZE, &q->sess);
+ q->out_buf = qzMalloc(q->out_len, 0, PINNED_MEM);
+ if (!q->out_buf) {
+ q->out_buf = qzMalloc(q->out_len, 0, COMMON_MEM);
+ if (!q->out_buf) {
+ err_msg = "qzMalloc failed";
+ goto err;
+ }
+ }
+
+ return 0;
+
+err:
+ error_setg(errp, "multifd %u: [sender] %s", p->id, err_msg);
+ return -1;
+}
+
+/**
+ * qatzip_send_cleanup: Tear down QATzip session and release private buffers.
+ *
+ * @param p Multifd channel params
+ * @param errp Pointer to error, which will be set in case of error
+ * @return None
+ */
+static void qatzip_send_cleanup(MultiFDSendParams *p, Error **errp)
+{
+ QatzipData *q = p->compress_data;
+
+ if (q) {
+ if (q->in_buf) {
+ qzFree(q->in_buf);
+ }
+ if (q->out_buf) {
+ qzFree(q->out_buf);
+ }
+ (void)qzTeardownSession(&q->sess);
+ (void)qzClose(&q->sess);
+ g_free(q);
+ }
+
+ g_free(p->iov);
+ p->iov = NULL;
+ p->compress_data = NULL;
+}
+
+/**
+ * qatzip_send_prepare: Compress pages and update IO channel info.
+ *
+ * @param p Multifd channel params
+ * @param errp Pointer to error, which will be set in case of error
+ * @return 0 on success, -1 on error (and *errp will be set)
+ */
+static int qatzip_send_prepare(MultiFDSendParams *p, Error **errp)
+{
+ MultiFDPages_t *pages = p->pages;
+ QatzipData *q = p->compress_data;
+ int ret;
+ unsigned int in_len, out_len;
+
+ if (!multifd_send_prepare_common(p)) {
+ goto out;
+ }
+
+ /*
+ * Unlike other multifd compression implementations, we use a non-streaming
+ * API and place all the data into one buffer, rather than sending each
+ * page to the compression API at a time. Based on initial benchmarks, the
+ * non-streaming API outperforms the streaming API. Plus, the logic in QEMU
+ * is friendly to using the non-streaming API anyway. If either of these
+ * statements becomes no longer true, we can revisit adding a streaming
+ * implementation.
+ */
+ for (int i = 0; i < pages->normal_num; i++) {
+ memcpy(q->in_buf + (i * p->page_size),
+ pages->block->host + pages->offset[i],
+ p->page_size);
+ }
+
+ in_len = pages->normal_num * p->page_size;
+ if (in_len > q->in_len) {
+ error_setg(errp, "multifd %u: unexpectedly large input", p->id);
+ return -1;
+ }
+ out_len = q->out_len;
+
+ ret = qzCompress(&q->sess, q->in_buf, &in_len, q->out_buf, &out_len, 1);
+ if (ret != QZ_OK) {
+ error_setg(errp, "multifd %u: QATzip returned %d instead of QZ_OK",
+ p->id, ret);
+ return -1;
+ }
+ if (in_len != pages->normal_num * p->page_size) {
+ error_setg(errp, "multifd %u: QATzip failed to compress all input",
+ p->id);
+ return -1;
+ }
+
+ p->iov[p->iovs_num].iov_base = q->out_buf;
+ p->iov[p->iovs_num].iov_len = out_len;
+ p->iovs_num++;
+ p->next_packet_size = out_len;
+
+out:
+ p->flags |= MULTIFD_FLAG_QATZIP;
+ multifd_send_fill_packet(p);
+ return 0;
+}
+
+/**
+ * qatzip_recv_setup: Set up QATzip session and allocate private buffers.
+ *
+ * @param p Multifd channel params
+ * @param errp Pointer to error, which will be set in case of error
+ * @return 0 on success, -1 on error (and *errp will be set)
+ */
+static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp)
+{
+ QatzipData *q;
+ QzSessionParamsDeflate_T params;
+ const char *err_msg;
+ int ret;
+
+ q = g_new0(QatzipData, 1);
+ p->compress_data = q;
+
+ /*
+ * Initialize QAT device with software fallback by default. This allows
+ * QATzip to use CPU path when QAT hardware reaches maximum throughput.
+ */
+ ret = qzInit(&q->sess, true);
+ if (ret != QZ_OK && ret != QZ_DUPLICATE) {
+ err_msg = "qzInit failed";
+ goto err;
+ }
+
+ ret = qzGetDefaultsDeflate(¶ms);
+ if (ret != QZ_OK) {
+ err_msg = "qzGetDefaultsDeflate failed";
+ goto err;
+ }
+
+ ret = qzSetupSessionDeflate(&q->sess, ¶ms);
+ if (ret != QZ_OK && ret != QZ_DUPLICATE) {
+ err_msg = "qzSetupSessionDeflate failed";
+ goto err;
+ }
+
+ /*
+ * Reserve extra spaces for the incoming packets. Current implementation
+ * doesn't send uncompressed pages in case the compression gets too big.
+ */
+ q->in_len = MULTIFD_PACKET_SIZE * 2;
+ /*
+ * PINNED_MEM is an enum from qatzip headers, which means to use
+ * kzalloc_node() to allocate memory for QAT DMA purposes. When QAT device
+ * is not available or software fallback is used, the malloc flag needs to
+ * be set as COMMON_MEM.
+ */
+ q->in_buf = qzMalloc(q->in_len, 0, PINNED_MEM);
+ if (!q->in_buf) {
+ q->in_buf = qzMalloc(q->in_len, 0, COMMON_MEM);
+ if (!q->in_buf) {
+ err_msg = "qzMalloc failed";
+ goto err;
+ }
+ }
+
+ q->out_len = MULTIFD_PACKET_SIZE;
+ q->out_buf = qzMalloc(q->out_len, 0, PINNED_MEM);
+ if (!q->out_buf) {
+ q->out_buf = qzMalloc(q->out_len, 0, COMMON_MEM);
+ if (!q->out_buf) {
+ err_msg = "qzMalloc failed";
+ goto err;
+ }
+ }
+
+ return 0;
+
+err:
+ error_setg(errp, "multifd %u: [receiver] %s", p->id, err_msg);
+ return -1;
+}
+
+/**
+ * qatzip_recv_cleanup: Tear down QATzip session and release private buffers.
+ *
+ * @param p Multifd channel params
+ * @return None
+ */
+static void qatzip_recv_cleanup(MultiFDRecvParams *p)
+{
+ QatzipData *q = p->compress_data;
+
+ if (q) {
+ if (q->in_buf) {
+ qzFree(q->in_buf);
+ }
+ if (q->out_buf) {
+ qzFree(q->out_buf);
+ }
+ (void)qzTeardownSession(&q->sess);
+ (void)qzClose(&q->sess);
+ g_free(q);
+ }
+ p->compress_data = NULL;
+}
+
+
+/**
+ * qatzip_recv: Decompress pages and copy them to the appropriate
+ * locations.
+ *
+ * @param p Multifd channel params
+ * @param errp Pointer to error, which will be set in case of error
+ * @return 0 on success, -1 on error (and *errp will be set)
+ */
+static int qatzip_recv(MultiFDRecvParams *p, Error **errp)
+{
+ QatzipData *q = p->compress_data;
+ int ret;
+ unsigned int in_len, out_len;
+ uint32_t in_size = p->next_packet_size;
+ uint32_t expected_size = p->normal_num * p->page_size;
+ uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
+
+ if (in_size > q->in_len) {
+ error_setg(errp, "multifd %u: received unexpectedly large packet",
+ p->id);
+ return -1;
+ }
+
+ if (flags != MULTIFD_FLAG_QATZIP) {
+ error_setg(errp, "multifd %u: flags received %x flags expected %x",
+ p->id, flags, MULTIFD_FLAG_QATZIP);
+ return -1;
+ }
+
+ multifd_recv_zero_page_process(p);
+ if (!p->normal_num) {
+ assert(in_size == 0);
+ return 0;
+ }
+
+ ret = qio_channel_read_all(p->c, (void *)q->in_buf, in_size, errp);
+ if (ret != 0) {
+ return ret;
+ }
+
+ in_len = in_size;
+ out_len = q->out_len;
+ ret = qzDecompress(&q->sess, q->in_buf, &in_len, q->out_buf, &out_len);
+ if (ret != QZ_OK) {
+ error_setg(errp, "multifd %u: qzDecompress failed", p->id);
+ return -1;
+ }
+ if (out_len != expected_size) {
+ error_setg(errp, "multifd %u: packet size received %u size expected %u",
+ p->id, out_len, expected_size);
+ return -1;
+ }
+
+ /* Copy each page to its appropriate location. */
+ for (int i = 0; i < p->normal_num; i++) {
+ memcpy(p->host + p->normal[i],
+ q->out_buf + p->page_size * i,
+ p->page_size);
+ }
+ return 0;
+}
+
+static MultiFDMethods multifd_qatzip_ops = {
+ .send_setup = qatzip_send_setup,
+ .send_cleanup = qatzip_send_cleanup,
+ .send_prepare = qatzip_send_prepare,
+ .recv_setup = qatzip_recv_setup,
+ .recv_cleanup = qatzip_recv_cleanup,
+ .recv = qatzip_recv
+};
+
+static void multifd_qatzip_register(void)
+{
+ multifd_register_ops(MULTIFD_COMPRESSION_QATZIP, &multifd_qatzip_ops);
+}
+
+migration_init(multifd_qatzip_register);
diff --git a/migration/multifd.h b/migration/multifd.h
index 0ecd6f47d7..adceb65050 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -34,14 +34,15 @@ MultiFDRecvData *multifd_get_recv_data(void);
/* Multifd Compression flags */
#define MULTIFD_FLAG_SYNC (1 << 0)
-/* We reserve 4 bits for compression methods */
-#define MULTIFD_FLAG_COMPRESSION_MASK (0xf << 1)
+/* We reserve 5 bits for compression methods */
+#define MULTIFD_FLAG_COMPRESSION_MASK (0x1f << 1)
/* we need to be compatible. Before compression value was 0 */
#define MULTIFD_FLAG_NOCOMP (0 << 1)
#define MULTIFD_FLAG_ZLIB (1 << 1)
#define MULTIFD_FLAG_ZSTD (2 << 1)
#define MULTIFD_FLAG_QPL (4 << 1)
#define MULTIFD_FLAG_UADK (8 << 1)
+#define MULTIFD_FLAG_QATZIP (16 << 1)
/* This value needs to be a multiple of qemu_target_page_size() */
#define MULTIFD_PACKET_SIZE (512 * 1024)
diff --git a/qapi/migration.json b/qapi/migration.json
index f4c27426c8..f1b7103dc8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -561,6 +561,8 @@
#
# @zstd: use zstd compression method.
#
+# @qatzip: use qatzip compression method. (Since 9.2)
+#
# @qpl: use qpl compression method. Query Processing Library(qpl) is
# based on the deflate compression algorithm and use the Intel
# In-Memory Analytics Accelerator(IAA) accelerated compression and
@@ -573,6 +575,7 @@
{ 'enum': 'MultiFDCompression',
'data': [ 'none', 'zlib',
{ 'name': 'zstd', 'if': 'CONFIG_ZSTD' },
+ { 'name': 'qatzip', 'if': 'CONFIG_QATZIP'},
{ 'name': 'qpl', 'if': 'CONFIG_QPL' },
{ 'name': 'uadk', 'if': 'CONFIG_UADK' } ] }
--
Yichen Wang
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v8 4/5] migration: Introduce 'qatzip' compression method
2024-08-20 17:09 ` [PATCH v8 4/5] migration: Introduce 'qatzip' compression method Yichen Wang
@ 2024-08-22 11:06 ` Prasad Pandit
2024-08-23 22:51 ` [External] " Yichen Wang
0 siblings, 1 reply; 13+ messages in thread
From: Prasad Pandit @ 2024-08-22 11:06 UTC (permalink / raw)
To: Yichen Wang
Cc: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel, Hao Xiang, Liu, Yuan1, Zou, Nanhai,
Ho-Ren (Jack) Chuang, Xiaoning Ding, Bryan Zhang
Hello,
On Tue, 20 Aug 2024 at 22:40, Yichen Wang <yichen.wang@bytedance.com> wrote:
> +static int qatzip_send_setup(MultiFDSendParams *p, Error **errp)
> +{
> + QatzipData *q;
> + QzSessionParamsDeflate_T params;
> + const char *err_msg;
> + int ret;
> +
> + q = g_new0(QatzipData, 1);
> + p->compress_data = q;
> + /* We need one extra place for the packet header */
> + p->iov = g_new0(struct iovec, 2);
> +
> + /*
> + * Initialize QAT device with software fallback by default. This allows
> + * QATzip to use CPU path when QAT hardware reaches maximum throughput.
> + */
> + ret = qzInit(&q->sess, true);
> + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> + err_msg = "qzInit failed";
> + goto err;
> + }
> +
> + ret = qzGetDefaultsDeflate(¶ms);
> + if (ret != QZ_OK) {
> + err_msg = "qzGetDefaultsDeflate failed";
> + goto err;
> + }
> +
> + /* Make sure to use configured QATzip compression level. */
> + params.common_params.comp_lvl = migrate_multifd_qatzip_level();
> + ret = qzSetupSessionDeflate(&q->sess, ¶ms);
> + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> + err_msg = "qzSetupSessionDeflate failed";
> + goto err;
> + }
> +
> + if (MULTIFD_PACKET_SIZE > UINT32_MAX) {
> + err_msg = "packet size too large for QAT";
> + goto err;
> + }
> +
> + q->in_len = MULTIFD_PACKET_SIZE;
> + /*
> + * PINNED_MEM is an enum from qatzip headers, which means to use
> + * kzalloc_node() to allocate memory for QAT DMA purposes. When QAT device
> + * is not available or software fallback is used, the malloc flag needs to
> + * be set as COMMON_MEM.
> + */
> + q->in_buf = qzMalloc(q->in_len, 0, PINNED_MEM);
> + if (!q->in_buf) {
> + q->in_buf = qzMalloc(q->in_len, 0, COMMON_MEM);
> + if (!q->in_buf) {
> + err_msg = "qzMalloc failed";
> + goto err;
> + }
> + }
> +
> + q->out_len = qzMaxCompressedLength(MULTIFD_PACKET_SIZE, &q->sess);
> + q->out_buf = qzMalloc(q->out_len, 0, PINNED_MEM);
> + if (!q->out_buf) {
> + q->out_buf = qzMalloc(q->out_len, 0, COMMON_MEM);
> + if (!q->out_buf) {
> + err_msg = "qzMalloc failed";
> + goto err;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + error_setg(errp, "multifd %u: [sender] %s", p->id, err_msg);
> + return -1;
> +}
* Need to release (g_free OR qatzip_send_cleanup) allocated memory in
the error (err:) path.
> +static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp)
> +{
> + QatzipData *q;
> + QzSessionParamsDeflate_T params;
> + const char *err_msg;
> + int ret;
> +
> + q = g_new0(QatzipData, 1);
> + p->compress_data = q;
> +
> + /*
> + * Initialize QAT device with software fallback by default. This allows
> + * QATzip to use CPU path when QAT hardware reaches maximum throughput.
> + */
> + ret = qzInit(&q->sess, true);
> + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> + err_msg = "qzInit failed";
> + goto err;
> + }
> +
> + ret = qzGetDefaultsDeflate(¶ms);
> + if (ret != QZ_OK) {
> + err_msg = "qzGetDefaultsDeflate failed";
> + goto err;
> + }
> +
> + ret = qzSetupSessionDeflate(&q->sess, ¶ms);
> + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> + err_msg = "qzSetupSessionDeflate failed";
> + goto err;
> + }
> +
> + /*
> + * Reserve extra spaces for the incoming packets. Current implementation
> + * doesn't send uncompressed pages in case the compression gets too big.
> + */
> + q->in_len = MULTIFD_PACKET_SIZE * 2;
> + /*
> + * PINNED_MEM is an enum from qatzip headers, which means to use
> + * kzalloc_node() to allocate memory for QAT DMA purposes. When QAT device
> + * is not available or software fallback is used, the malloc flag needs to
> + * be set as COMMON_MEM.
> + */
> + q->in_buf = qzMalloc(q->in_len, 0, PINNED_MEM);
> + if (!q->in_buf) {
> + q->in_buf = qzMalloc(q->in_len, 0, COMMON_MEM);
> + if (!q->in_buf) {
> + err_msg = "qzMalloc failed";
> + goto err;
> + }
> + }
> +
> + q->out_len = MULTIFD_PACKET_SIZE;
> + q->out_buf = qzMalloc(q->out_len, 0, PINNED_MEM);
> + if (!q->out_buf) {
> + q->out_buf = qzMalloc(q->out_len, 0, COMMON_MEM);
> + if (!q->out_buf) {
> + err_msg = "qzMalloc failed";
> + goto err;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + error_setg(errp, "multifd %u: [receiver] %s", p->id, err_msg);
> + return -1;
> +}
* Need to release (g_free OR qatzip_recv_cleanup) allocated memory in
the error (err:) path.
Thank you.
---
- Prasad
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [External] Re: [PATCH v8 4/5] migration: Introduce 'qatzip' compression method
2024-08-22 11:06 ` Prasad Pandit
@ 2024-08-23 22:51 ` Yichen Wang
2024-08-26 7:14 ` Prasad Pandit
0 siblings, 1 reply; 13+ messages in thread
From: Yichen Wang @ 2024-08-23 22:51 UTC (permalink / raw)
To: Prasad Pandit
Cc: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel, Hao Xiang, Liu, Yuan1, Zou, Nanhai,
Ho-Ren (Jack) Chuang, Xiaoning Ding, Bryan Zhang
On Thu, Aug 22, 2024 at 4:06 AM Prasad Pandit <ppandit@redhat.com> wrote:
>
> Hello,
>
> On Tue, 20 Aug 2024 at 22:40, Yichen Wang <yichen.wang@bytedance.com> wrote:
> > +static int qatzip_send_setup(MultiFDSendParams *p, Error **errp)
> > +{
> > + QatzipData *q;
> > + QzSessionParamsDeflate_T params;
> > + const char *err_msg;
> > + int ret;
> > +
> > + q = g_new0(QatzipData, 1);
> > + p->compress_data = q;
> > + /* We need one extra place for the packet header */
> > + p->iov = g_new0(struct iovec, 2);
> > +
> > + /*
> > + * Initialize QAT device with software fallback by default. This allows
> > + * QATzip to use CPU path when QAT hardware reaches maximum throughput.
> > + */
> > + ret = qzInit(&q->sess, true);
> > + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> > + err_msg = "qzInit failed";
> > + goto err;
> > + }
> > +
> > + ret = qzGetDefaultsDeflate(¶ms);
> > + if (ret != QZ_OK) {
> > + err_msg = "qzGetDefaultsDeflate failed";
> > + goto err;
> > + }
> > +
> > + /* Make sure to use configured QATzip compression level. */
> > + params.common_params.comp_lvl = migrate_multifd_qatzip_level();
> > + ret = qzSetupSessionDeflate(&q->sess, ¶ms);
> > + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> > + err_msg = "qzSetupSessionDeflate failed";
> > + goto err;
> > + }
> > +
> > + if (MULTIFD_PACKET_SIZE > UINT32_MAX) {
> > + err_msg = "packet size too large for QAT";
> > + goto err;
> > + }
> > +
> > + q->in_len = MULTIFD_PACKET_SIZE;
> > + /*
> > + * PINNED_MEM is an enum from qatzip headers, which means to use
> > + * kzalloc_node() to allocate memory for QAT DMA purposes. When QAT device
> > + * is not available or software fallback is used, the malloc flag needs to
> > + * be set as COMMON_MEM.
> > + */
> > + q->in_buf = qzMalloc(q->in_len, 0, PINNED_MEM);
> > + if (!q->in_buf) {
> > + q->in_buf = qzMalloc(q->in_len, 0, COMMON_MEM);
> > + if (!q->in_buf) {
> > + err_msg = "qzMalloc failed";
> > + goto err;
> > + }
> > + }
> > +
> > + q->out_len = qzMaxCompressedLength(MULTIFD_PACKET_SIZE, &q->sess);
> > + q->out_buf = qzMalloc(q->out_len, 0, PINNED_MEM);
> > + if (!q->out_buf) {
> > + q->out_buf = qzMalloc(q->out_len, 0, COMMON_MEM);
> > + if (!q->out_buf) {
> > + err_msg = "qzMalloc failed";
> > + goto err;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +err:
> > + error_setg(errp, "multifd %u: [sender] %s", p->id, err_msg);
> > + return -1;
> > +}
>
> * Need to release (g_free OR qatzip_send_cleanup) allocated memory in
> the error (err:) path.
>
The patch was originally written exactly like what you suggest,
cleanup in the error path of the same function. However, later I
realized in gdb that I was wrong. The qatzip_send_cleanup() function
will be called later in another thread in both normal and error paths.
So I revised the patch to this behavior, otherwise I will run into
double free in the error path.
>
> > +static int qatzip_recv_setup(MultiFDRecvParams *p, Error **errp)
> > +{
> > + QatzipData *q;
> > + QzSessionParamsDeflate_T params;
> > + const char *err_msg;
> > + int ret;
> > +
> > + q = g_new0(QatzipData, 1);
> > + p->compress_data = q;
> > +
> > + /*
> > + * Initialize QAT device with software fallback by default. This allows
> > + * QATzip to use CPU path when QAT hardware reaches maximum throughput.
> > + */
> > + ret = qzInit(&q->sess, true);
> > + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> > + err_msg = "qzInit failed";
> > + goto err;
> > + }
> > +
> > + ret = qzGetDefaultsDeflate(¶ms);
> > + if (ret != QZ_OK) {
> > + err_msg = "qzGetDefaultsDeflate failed";
> > + goto err;
> > + }
> > +
> > + ret = qzSetupSessionDeflate(&q->sess, ¶ms);
> > + if (ret != QZ_OK && ret != QZ_DUPLICATE) {
> > + err_msg = "qzSetupSessionDeflate failed";
> > + goto err;
> > + }
> > +
> > + /*
> > + * Reserve extra spaces for the incoming packets. Current implementation
> > + * doesn't send uncompressed pages in case the compression gets too big.
> > + */
> > + q->in_len = MULTIFD_PACKET_SIZE * 2;
> > + /*
> > + * PINNED_MEM is an enum from qatzip headers, which means to use
> > + * kzalloc_node() to allocate memory for QAT DMA purposes. When QAT device
> > + * is not available or software fallback is used, the malloc flag needs to
> > + * be set as COMMON_MEM.
> > + */
> > + q->in_buf = qzMalloc(q->in_len, 0, PINNED_MEM);
> > + if (!q->in_buf) {
> > + q->in_buf = qzMalloc(q->in_len, 0, COMMON_MEM);
> > + if (!q->in_buf) {
> > + err_msg = "qzMalloc failed";
> > + goto err;
> > + }
> > + }
> > +
> > + q->out_len = MULTIFD_PACKET_SIZE;
> > + q->out_buf = qzMalloc(q->out_len, 0, PINNED_MEM);
> > + if (!q->out_buf) {
> > + q->out_buf = qzMalloc(q->out_len, 0, COMMON_MEM);
> > + if (!q->out_buf) {
> > + err_msg = "qzMalloc failed";
> > + goto err;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +err:
> > + error_setg(errp, "multifd %u: [receiver] %s", p->id, err_msg);
> > + return -1;
> > +}
>
> * Need to release (g_free OR qatzip_recv_cleanup) allocated memory in
> the error (err:) path.
>
> Thank you.
> ---
> - Prasad
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [External] Re: [PATCH v8 4/5] migration: Introduce 'qatzip' compression method
2024-08-23 22:51 ` [External] " Yichen Wang
@ 2024-08-26 7:14 ` Prasad Pandit
0 siblings, 0 replies; 13+ messages in thread
From: Prasad Pandit @ 2024-08-26 7:14 UTC (permalink / raw)
To: Yichen Wang
Cc: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel, Hao Xiang, Liu, Yuan1, Zou, Nanhai,
Ho-Ren (Jack) Chuang, Xiaoning Ding, Bryan Zhang
On Sat, 24 Aug 2024 at 04:22, Yichen Wang <yichen.wang@bytedance.com> wrote:
> The patch was originally written exactly like what you suggest,
> cleanup in the error path of the same function. However, later I
> realized in gdb that I was wrong. The qatzip_send_cleanup() function
> will be called later in another thread in both normal and error paths.
> So I revised the patch to this behavior, otherwise I will run into
> double free in the error path.
>
* I see, okay, in that case:
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Thank you.
---
- Prasad
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v8 5/5] tests/migration: Add integration test for 'qatzip' compression method
2024-08-20 17:09 [PATCH v8 0/5] Implement QATzip compression method Yichen Wang
` (3 preceding siblings ...)
2024-08-20 17:09 ` [PATCH v8 4/5] migration: Introduce 'qatzip' compression method Yichen Wang
@ 2024-08-20 17:09 ` Yichen Wang
4 siblings, 0 replies; 13+ messages in thread
From: Yichen Wang @ 2024-08-20 17:09 UTC (permalink / raw)
To: Peter Xu, Fabiano Rosas, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost, Marc-André Lureau, Thomas Huth,
Philippe Mathieu-Daudé, Eric Blake, Markus Armbruster,
qemu-devel
Cc: Hao Xiang, Liu, Yuan1, Zou, Nanhai, Ho-Ren (Jack) Chuang,
Yichen Wang, Xiaoning Ding, Bryan Zhang
From: Bryan Zhang <bryan.zhang@bytedance.com>
Adds an integration test for 'qatzip'.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
Signed-off-by: Hao Xiang <hao.xiang@linux.dev>
Signed-off-by: Yichen Wang <yichen.wang@bytedance.com>
---
tests/qtest/migration-test.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 70b606b888..3aed5441f7 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2992,6 +2992,18 @@ test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
}
#endif /* CONFIG_ZSTD */
+#ifdef CONFIG_QATZIP
+static void *
+test_migrate_precopy_tcp_multifd_qatzip_start(QTestState *from,
+ QTestState *to)
+{
+ migrate_set_parameter_int(from, "multifd-qatzip-level", 2);
+ migrate_set_parameter_int(to, "multifd-qatzip-level", 2);
+
+ return test_migrate_precopy_tcp_multifd_start_common(from, to, "qatzip");
+}
+#endif
+
#ifdef CONFIG_QPL
static void *
test_migrate_precopy_tcp_multifd_qpl_start(QTestState *from,
@@ -3089,6 +3101,17 @@ static void test_multifd_tcp_zstd(void)
}
#endif
+#ifdef CONFIG_QATZIP
+static void test_multifd_tcp_qatzip(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_precopy_tcp_multifd_qatzip_start,
+ };
+ test_precopy_common(&args);
+}
+#endif
+
#ifdef CONFIG_QPL
static void test_multifd_tcp_qpl(void)
{
@@ -3992,6 +4015,10 @@ int main(int argc, char **argv)
migration_test_add("/migration/multifd/tcp/plain/zstd",
test_multifd_tcp_zstd);
#endif
+#ifdef CONFIG_QATZIP
+ migration_test_add("/migration/multifd/tcp/plain/qatzip",
+ test_multifd_tcp_qatzip);
+#endif
#ifdef CONFIG_QPL
migration_test_add("/migration/multifd/tcp/plain/qpl",
test_multifd_tcp_qpl);
--
Yichen Wang
^ permalink raw reply related [flat|nested] 13+ messages in thread