On Fri, Feb 16, 2024 at 2:42 PM Hao Xiang <hao.xi...@bytedance.com> wrote:
> This change adds a dedicated handler for
> MigrationOps::ram_save_target_page in
> multifd live migration. Now zero page checking can be done in the multifd
> threads
> and this becomes the default configuration. We still provide backward
> compatibility
> where zero page checking is done from the migration main thread.
>
> Signed-off-by: Hao Xiang <hao.xi...@bytedance.com>
> ---
> migration/multifd.c | 1 +
> migration/options.c | 2 +-
> migration/ram.c | 53 ++++++++++++++++++++++++++++++++++-----------
> 3 files changed, 42 insertions(+), 14 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index fbb40ea10b..ef5dad1019 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -13,6 +13,7 @@
> #include "qemu/osdep.h"
> #include "qemu/cutils.h"
> #include "qemu/rcu.h"
> +#include "qemu/cutils.h"
> #include "exec/target_page.h"
> #include "sysemu/sysemu.h"
> #include "exec/ramblock.h"
> diff --git a/migration/options.c b/migration/options.c
> index 3c603391b0..3c79b6ccd4 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -181,7 +181,7 @@ Property migration_properties[] = {
> MIG_MODE_NORMAL),
> DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState,
> parameters.zero_page_detection,
> - ZERO_PAGE_DETECTION_LEGACY),
> + ZERO_PAGE_DETECTION_MULTIFD),
>
> /* Migration capabilities */
> DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
> diff --git a/migration/ram.c b/migration/ram.c
> index 5ece9f042e..b088c5a98c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1123,10 +1123,6 @@ static int save_zero_page(RAMState *rs,
> PageSearchStatus *pss,
> QEMUFile *file = pss->pss_channel;
> int len = 0;
>
> - if (migrate_zero_page_detection() != ZERO_PAGE_DETECTION_LEGACY) {
> - return 0;
> - }
> -
> if (!buffer_is_zero(p, TARGET_PAGE_SIZE)) {
> return 0;
> }
> @@ -1256,6 +1252,10 @@ static int ram_save_page(RAMState *rs,
> PageSearchStatus *pss)
>
> static int ram_save_multifd_page(RAMBlock *block, ram_addr_t offset)
> {
> + assert(migrate_multifd());
>
We only call ram_save_multifd_page() if:
if (migrate_multifd()) {
migration_ops->ram_save_target_page = ram_save_target_page_multifd;
So this assert is not needed.
+ assert(!migrate_compress());
>
+ assert(!migration_in_postcopy());
>
These two are redundant and done before we call in here.
+
> if (!multifd_queue_page(block, offset)) {
> return -1;
> }
> @@ -2046,7 +2046,6 @@ static bool save_compress_page(RAMState *rs,
> PageSearchStatus *pss,
> */
> static int ram_save_target_page_legacy(RAMState *rs, PageSearchStatus
> *pss)
> {
> - RAMBlock *block = pss->block;
> ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
> int res;
>
> @@ -2062,17 +2061,40 @@ static int ram_save_target_page_legacy(RAMState
> *rs, PageSearchStatus *pss)
> return 1;
> }
>
> + return ram_save_page(rs, pss);
> +}
> +
> +/**
> + * ram_save_target_page_multifd: save one target page
> + *
> + * Returns the number of pages written
> + *
> + * @rs: current RAM state
> + * @pss: data about the page we want to send
> + */
> +static int ram_save_target_page_multifd(RAMState *rs, PageSearchStatus
> *pss)
> +{
> + RAMBlock *block = pss->block;
> + ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
> +
> + /* Multifd is not compatible with old compression. */
> + assert(!migrate_compress());
>
Do we need to check this for every page?
> + /* Multifd is not compabible with postcopy. */
> + assert(!migration_in_postcopy());
> +
> /*
> - * Do not use multifd in postcopy as one whole host page should be
> - * placed. Meanwhile postcopy requires atomic update of pages, so
> even
> - * if host page size == guest page size the dest guest during run may
> - * still see partially copied pages which is data corruption.
> + * Backward compatibility support. While using multifd live
> + * migration, we still need to handle zero page checking on the
> + * migration main thread.
> */
> - if (migrate_multifd() && !migration_in_postcopy()) {
> - return ram_save_multifd_page(block, offset);
> + if (migrate_zero_page_detection() == ZERO_PAGE_DETECTION_LEGACY) {
> + if (save_zero_page(rs, pss, offset)) {
> + return 1;
> + }
> }
>
> - return ram_save_page(rs, pss);
> + return ram_save_multifd_page(block, offset);
> }
>
> /* Should be called before sending a host page */
> @@ -2984,7 +3006,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> }
>
> migration_ops = g_malloc0(sizeof(MigrationOps));
> - migration_ops->ram_save_target_page = ram_save_target_page_legacy;
> +
> + if (migrate_multifd()) {
> + migration_ops->ram_save_target_page =
> ram_save_target_page_multifd;
> + } else {
> + migration_ops->ram_save_target_page = ram_save_target_page_legacy;
> + }
>
> bql_unlock();
> ret = multifd_send_sync_main();
> --
> 2.30.2
>
>
>
--
Elena
