| Message ID | 20231231205804.2366509-6-bryan.zhang@bytedance.com |
|---|---|
| State | New |
| Headers | show |
| Series | *** Implement using Intel QAT to offload ZLIB | expand |
On Sun, Dec 31, 2023 at 08:58:04PM +0000, Bryan Zhang wrote: > Adds an integration test for 'qatzip'. Please use "tests" as prefix of this patch. It can be "tests/migration:", "tests/migration-test:", etc. > > Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com> > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> [...] > test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from, > @@ -3480,6 +3504,19 @@ int main(int argc, char **argv) > qtest_add_func("/migration/multifd/tcp/plain/zstd", > test_multifd_tcp_zstd); > #endif > +#ifdef CONFIG_QATZIP > + /* > + * Use QATzip's qzInit() function as a runtime hardware check. > + * Ideally there might be a cleaner way to probe for the presence of QAT. > + */ > + QzSession_T sess; > + memset(&sess, 0, sizeof(QzSession_T)); > + if (qzInit(&sess, 0) == QZ_OK) { Does "0" means it'll fail if no hardware is available, no matter whether due to processor too old, or limited resources? Would it make sense to test it even if only software fallbacks are available? IIUC the migration path will still be covered in software fallbacks, so it may still makes sense to me. It can be very likely that most CIs will not cover the qatzip paths otherwise, because of either no control over hardwares, or limited privileges of the CI user (does qat HWs need special privilege normally? I have no idea how QAT resource management is done if there're only limited HW resources). Besides, I believe all the data points you provided in the cover letter are collected with 2 QAT HWs enabled? I'm curious how's the performance of the software fallback of qatzip comparing to existing algorithm: iiuc zstd is mostly always preferred if sololy comparing to zlib? where does qatzip soft-mode stand in the picture? Thanks,
On Mon, Jan 29, 2024 at 12:53 AM Peter Xu <peterx@redhat.com> wrote: > On Sun, Dec 31, 2023 at 08:58:04PM +0000, Bryan Zhang wrote: > > Adds an integration test for 'qatzip'. > > Please use "tests" as prefix of this patch. It can be "tests/migration:", > "tests/migration-test:", etc. > > Will do. > > > > Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com> > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > > [...] > > > test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from, > > @@ -3480,6 +3504,19 @@ int main(int argc, char **argv) > > qtest_add_func("/migration/multifd/tcp/plain/zstd", > > test_multifd_tcp_zstd); > > #endif > > +#ifdef CONFIG_QATZIP > > + /* > > + * Use QATzip's qzInit() function as a runtime hardware check. > > + * Ideally there might be a cleaner way to probe for the presence > of QAT. > > + */ > > + QzSession_T sess; > > + memset(&sess, 0, sizeof(QzSession_T)); > > + if (qzInit(&sess, 0) == QZ_OK) { > > Does "0" means it'll fail if no hardware is available, no matter whether > due to processor too old, or limited resources? > > Would it make sense to test it even if only software fallbacks are > available? IIUC the migration path will still be covered in software > fallbacks, so it may still makes sense to me. It can be very likely that > most CIs will not cover the qatzip paths otherwise, because of either no > control over hardwares, or limited privileges of the CI user (does qat HWs > need special privilege normally? I have no idea how QAT resource > management is done if there're only limited HW resources). > > Besides, I believe all the data points you provided in the cover letter are > collected with 2 QAT HWs enabled? I'm curious how's the performance of the > software fallback of qatzip comparing to existing algorithm: iiuc zstd is > mostly always preferred if sololy comparing to zlib? where does qatzip > soft-mode stand in the picture? > Yes, as I understand it, 0 means the code will error if the hardware is unavailable for whatever reason. We can enable software fallback in the live migration path, which will also enable using software to run the QATzip tests (and will conveniently allow us to remove the awkward `qzInit` check in the test code). It also might be a good idea since it would also avoid failure in case of, e.g., a transient hardware problem? Performance: The software fallback seems prohibitively slow. QATzip fallback uses the built-in zlib implementation, but I ran a migration test that normally takes zlib about 150 seconds and the QATzip fallback took about 30 minutes before my SSH session disconnected :P Also, a note: When enabling software fallback, QATzip and the QAT driver will both repeatedly print a complaint to the QEMU monitor when trying to compress without hardware. Is it bad form to introduce seemingly-unsuppressable prints from dependencies, or is this acceptable? > > Thanks, > > -- > Peter Xu > > Thanks for your feedback. -- Bryan Zhang
On Wed, Feb 28, 2024 at 06:55:37PM -0800, Bryan Zhang . wrote: > We can enable software fallback in the live migration path, which will also > enable using software to run the QATzip tests (and will conveniently allow > us to remove the awkward `qzInit` check in the test code). It also might be > a good idea since it would also avoid failure in case of, e.g., a transient > hardware problem? > > Performance: The software fallback seems prohibitively slow. QATzip > fallback uses the built-in zlib implementation, but I ran a migration test > that normally takes zlib about 150 seconds and the QATzip fallback took > about 30 minutes before my SSH session disconnected :P When preparing the qatzip documentataion file, please mention this. This is definitely not expected by myself, we should suggest mostly never use qatzip compatible (software) mode unless the admin is sure to have the hardware support. It should then only be used in qtest to cover the qatzip paths only. > > Also, a note: When enabling software fallback, QATzip and the QAT driver > will both repeatedly print a complaint to the QEMU monitor when trying to > compress without hardware. Is it bad form to introduce > seemingly-unsuppressable prints from dependencies, or is this acceptable? It is definitely bad to keep printing the same message, no matter from where. Why it repeatedly do so? Do you know why the library cannot make it print_once()?
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 47dabf91d0..5931bd6418 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -302,6 +302,10 @@ if gnutls.found() endif endif +if qatzip.found() + migration_files += [qatzip] +endif + qtests = { 'bios-tables-test': [io, 'boot-sector.c', 'acpi-utils.c', 'tpm-emu.c'], 'cdrom-test': files('boot-sector.c'), diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index d520c587f7..f51bc4056f 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -35,6 +35,10 @@ # endif /* CONFIG_TASN1 */ #endif /* CONFIG_GNUTLS */ +#ifdef CONFIG_QATZIP +#include <qatzip.h> +#endif /* CONFIG_QATZIP */ + /* For dirty ring test; so far only x86_64 is supported */ #if defined(__linux__) && defined(HOST_X86_64) #include "linux/kvm.h" @@ -2572,6 +2576,15 @@ 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) +{ + return test_migrate_precopy_tcp_multifd_start_common(from, to, "qatzip"); +} +#endif + static void test_multifd_tcp_none(void) { MigrateCommon args = { @@ -2607,6 +2620,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_GNUTLS static void * test_migrate_multifd_tcp_tls_psk_start_match(QTestState *from, @@ -3480,6 +3504,19 @@ int main(int argc, char **argv) qtest_add_func("/migration/multifd/tcp/plain/zstd", test_multifd_tcp_zstd); #endif +#ifdef CONFIG_QATZIP + /* + * Use QATzip's qzInit() function as a runtime hardware check. + * Ideally there might be a cleaner way to probe for the presence of QAT. + */ + QzSession_T sess; + memset(&sess, 0, sizeof(QzSession_T)); + if (qzInit(&sess, 0) == QZ_OK) { + qzClose(&sess); + qtest_add_func("/migration/multifd/tcp/plain/qatzip", + test_multifd_tcp_qatzip); + } +#endif #ifdef CONFIG_GNUTLS qtest_add_func("/migration/multifd/tcp/tls/psk/match", test_multifd_tcp_tls_psk_match);