[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 1/3] multifd: Create property multifd-sync-after-each-sect
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v5 1/3] multifd: Create property multifd-sync-after-each-section |
Date: |
Tue, 14 Feb 2023 09:45:02 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Juan Quintela <quintela@redhat.com> writes:
> We used to synchronize all channels at the end of each RAM section
> sent. That is not needed, so preparing to only synchronize once every
> full round in latests patches.
>
> Notice that we initialize the property as true. We will change the
> default when we introduce the new mechanism.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> ---
>
> Rename each-iteration to after-each-section
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> qapi/migration.json | 10 +++++++++-
> migration/migration.h | 1 +
> hw/core/machine.c | 1 +
> migration/migration.c | 15 +++++++++++++--
> 4 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index c84fa10e86..2907241b9c 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -478,6 +478,13 @@
> # should not affect the correctness of postcopy migration.
> # (since 7.1)
> #
> +# @multifd-sync-after-each-section: Synchronize channels after each
> +# section is sent.
What does it mean to synchronize channels?
When would I want to, and why?
> +# We used to do
> +# that in the past, but it is
> +# suboptimal.
This isn't particularly helpful, I'm afraid.
> +# Default value is true until all code is
> in.
As far as I can tell, it's actually *unused* for now, and a later patch
will put it to use ...
> +# (since 8.0)
> +#
> # Features:
> # @unstable: Members @x-colo and @x-ignore-shared are experimental.
> #
> @@ -492,7 +499,8 @@
> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
> 'validate-uuid', 'background-snapshot',
> - 'zero-copy-send', 'postcopy-preempt'] }
> + 'zero-copy-send', 'postcopy-preempt',
> + 'multifd-sync-after-each-section'] }
>
> ##
> # @MigrationCapabilityStatus:
> diff --git a/migration/migration.h b/migration/migration.h
> index 2da2f8a164..cf84520196 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -424,6 +424,7 @@ int migrate_multifd_channels(void);
> MultiFDCompression migrate_multifd_compression(void);
> int migrate_multifd_zlib_level(void);
> int migrate_multifd_zstd_level(void);
> +bool migrate_multifd_sync_after_each_section(void);
>
> #ifdef CONFIG_LINUX
> bool migrate_use_zero_copy_send(void);
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f73fc4c45c..dc86849402 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -54,6 +54,7 @@ const size_t hw_compat_7_1_len =
> G_N_ELEMENTS(hw_compat_7_1);
> GlobalProperty hw_compat_7_0[] = {
> { "arm-gicv3-common", "force-8-bit-prio", "on" },
> { "nvme-ns", "eui64-default", "on"},
> + { "migration", "multifd-sync-after-each-section", "on"},
> };
> const size_t hw_compat_7_0_len = G_N_ELEMENTS(hw_compat_7_0);
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 90fca70cb7..406c27bc82 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -167,7 +167,8 @@
> INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
> MIGRATION_CAPABILITY_XBZRLE,
> MIGRATION_CAPABILITY_X_COLO,
> MIGRATION_CAPABILITY_VALIDATE_UUID,
> - MIGRATION_CAPABILITY_ZERO_COPY_SEND);
> + MIGRATION_CAPABILITY_ZERO_COPY_SEND,
> + MIGRATION_CAPABILITY_MULTIFD_SYNC_AFTER_EACH_SECTION);
>
> /* When we add fault tolerance, we could have several
> migrations at once. For now we don't need to add
> @@ -2701,6 +2702,15 @@ bool migrate_use_multifd(void)
> return s->enabled_capabilities[MIGRATION_CAPABILITY_MULTIFD];
> }
>
> +bool migrate_multifd_sync_after_each_section(void)
> +{
> + MigrationState *s = migrate_get_current();
> +
> + return true;
> + // We will change this when code gets in.
> + return
> s->enabled_capabilities[MIGRATION_CAPABILITY_MULTIFD_SYNC_AFTER_EACH_SECTION];
... here.
No warning about unreachable code? Checking... nope, gcc seems to not
to care.
> +}
> +
> bool migrate_pause_before_switchover(void)
> {
> MigrationState *s;
> @@ -4535,7 +4545,8 @@ static Property migration_properties[] = {
> DEFINE_PROP_MIG_CAP("x-zero-copy-send",
> MIGRATION_CAPABILITY_ZERO_COPY_SEND),
> #endif
> -
> + DEFINE_PROP_MIG_CAP("multifd-sync-after-each-section",
> +
> MIGRATION_CAPABILITY_MULTIFD_SYNC_AFTER_EACH_SECTION),
> DEFINE_PROP_END_OF_LIST(),
> };