| Message ID | 20220621140507.1246-2-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Eliminate multifd flush | expand |
* Juan Quintela (quintela@redhat.com) wrote: > 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. I don't understand why this is a property - does it break the actual stream format? Dave > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/migration.h | 6 ++++++ > hw/core/machine.c | 1 + > migration/migration.c | 10 ++++++++++ > 3 files changed, 17 insertions(+) > > diff --git a/migration/migration.h b/migration/migration.h > index 485d58b95f..70dae24516 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -332,6 +332,11 @@ struct MigrationState { > * This save hostname when out-going migration starts > */ > char *hostname; > + /* > + * Synchronize channels after each iteration. > + * We used to do that on the past, but it is suboptimal. > + */ > + bool multifd_sync_each_iteration; > }; > > void migrate_set_state(int *state, int old_state, int new_state); > @@ -374,6 +379,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_each_iteration(void); > > #ifdef CONFIG_LINUX > bool migrate_use_zero_copy_send(void); > diff --git a/hw/core/machine.c b/hw/core/machine.c > index a673302cce..c8a60917e0 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -43,6 +43,7 @@ > GlobalProperty hw_compat_7_0[] = { > { "arm-gicv3-common", "force-8-bit-prio", "on" }, > { "nvme-ns", "eui64-default", "on"}, > + { "migration", "multifd-sync-each-iteration", "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 31739b2af9..3f79df0b70 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2540,6 +2540,13 @@ bool migrate_use_multifd(void) > return s->enabled_capabilities[MIGRATION_CAPABILITY_MULTIFD]; > } > > +bool migrate_multifd_sync_each_iteration(void) > +{ > + MigrationState *s = migrate_get_current(); > + > + return s->multifd_sync_each_iteration; > +} > + > bool migrate_pause_before_switchover(void) > { > MigrationState *s; > @@ -4274,6 +4281,9 @@ static Property migration_properties[] = { > DEFINE_PROP_SIZE("announce-step", MigrationState, > parameters.announce_step, > DEFAULT_MIGRATE_ANNOUNCE_STEP), > + /* We will change to false when we introduce the new mechanism */ > + DEFINE_PROP_BOOL("multifd-sync-each-iteration", MigrationState, > + multifd_sync_each_iteration, true), > > /* Migration capabilities */ > DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), > -- > 2.34.1 >
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> 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. > > I don't understand why this is a property - does it break the actual > stream format? Yeap. You can see on following patches. The problem is that we synchronize each time that we sent/receive a RAM_SAVE_FLAG_EOS And that is way too much. As Leo showed, it can be as much as 20 times a second. Later, Juan.
* Juan Quintela (quintela@redhat.com) wrote: > 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> > --- > migration/migration.h | 6 ++++++ > hw/core/machine.c | 1 + > migration/migration.c | 10 ++++++++++ > 3 files changed, 17 insertions(+) > > diff --git a/migration/migration.h b/migration/migration.h > index 485d58b95f..70dae24516 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -332,6 +332,11 @@ struct MigrationState { > * This save hostname when out-going migration starts > */ > char *hostname; > + /* > + * Synchronize channels after each iteration. > + * We used to do that on the past, but it is suboptimal. > + */ > + bool multifd_sync_each_iteration; > }; > > void migrate_set_state(int *state, int old_state, int new_state); > @@ -374,6 +379,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_each_iteration(void); > > #ifdef CONFIG_LINUX > bool migrate_use_zero_copy_send(void); > diff --git a/hw/core/machine.c b/hw/core/machine.c > index a673302cce..c8a60917e0 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -43,6 +43,7 @@ > GlobalProperty hw_compat_7_0[] = { > { "arm-gicv3-common", "force-8-bit-prio", "on" }, > { "nvme-ns", "eui64-default", "on"}, > + { "migration", "multifd-sync-each-iteration", "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 31739b2af9..3f79df0b70 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2540,6 +2540,13 @@ bool migrate_use_multifd(void) > return s->enabled_capabilities[MIGRATION_CAPABILITY_MULTIFD]; > } > > +bool migrate_multifd_sync_each_iteration(void) > +{ > + MigrationState *s = migrate_get_current(); > + > + return s->multifd_sync_each_iteration; > +} > + > bool migrate_pause_before_switchover(void) > { > MigrationState *s; > @@ -4274,6 +4281,9 @@ static Property migration_properties[] = { > DEFINE_PROP_SIZE("announce-step", MigrationState, > parameters.announce_step, > DEFAULT_MIGRATE_ANNOUNCE_STEP), > + /* We will change to false when we introduce the new mechanism */ > + DEFINE_PROP_BOOL("multifd-sync-each-iteration", MigrationState, > + multifd_sync_each_iteration, true), > > /* Migration capabilities */ > DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), > -- > 2.34.1 >
diff --git a/migration/migration.h b/migration/migration.h index 485d58b95f..70dae24516 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -332,6 +332,11 @@ struct MigrationState { * This save hostname when out-going migration starts */ char *hostname; + /* + * Synchronize channels after each iteration. + * We used to do that on the past, but it is suboptimal. + */ + bool multifd_sync_each_iteration; }; void migrate_set_state(int *state, int old_state, int new_state); @@ -374,6 +379,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_each_iteration(void); #ifdef CONFIG_LINUX bool migrate_use_zero_copy_send(void); diff --git a/hw/core/machine.c b/hw/core/machine.c index a673302cce..c8a60917e0 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -43,6 +43,7 @@ GlobalProperty hw_compat_7_0[] = { { "arm-gicv3-common", "force-8-bit-prio", "on" }, { "nvme-ns", "eui64-default", "on"}, + { "migration", "multifd-sync-each-iteration", "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 31739b2af9..3f79df0b70 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2540,6 +2540,13 @@ bool migrate_use_multifd(void) return s->enabled_capabilities[MIGRATION_CAPABILITY_MULTIFD]; } +bool migrate_multifd_sync_each_iteration(void) +{ + MigrationState *s = migrate_get_current(); + + return s->multifd_sync_each_iteration; +} + bool migrate_pause_before_switchover(void) { MigrationState *s; @@ -4274,6 +4281,9 @@ static Property migration_properties[] = { DEFINE_PROP_SIZE("announce-step", MigrationState, parameters.announce_step, DEFAULT_MIGRATE_ANNOUNCE_STEP), + /* We will change to false when we introduce the new mechanism */ + DEFINE_PROP_BOOL("multifd-sync-each-iteration", MigrationState, + multifd_sync_each_iteration, true), /* Migration capabilities */ DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
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> --- migration/migration.h | 6 ++++++ hw/core/machine.c | 1 + migration/migration.c | 10 ++++++++++ 3 files changed, 17 insertions(+)