| Message ID | 20230425163114.2609-2-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Eliminate multifd flush | expand |
On Tue, Apr 25, 2023 at 06:31:12PM +0200, Juan Quintela wrote: > We used to flush all channels at the end of each RAM section > sent. That is not needed, so preparing to only flush after a full > iteration through all the RAM. > > Default value of the property is false. But we return "true" in > migrate_multifd_flush_after_each_section() until we implement the code > in following patches. > > 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> PS: Normally I'll just keep the last Sign-off-by for each person. :) > > --- > > Rename each-iteration to after-each-section > Rename multifd-sync-after-each-section to > multifd-flush-after-each-section > --- > hw/core/machine.c | 1 + > migration/migration.c | 13 +++++++++++++ > migration/migration.h | 13 +++++++++++++ > 3 files changed, 27 insertions(+) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 2ce97a5d3b..32bd9277b3 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -60,6 +60,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-flush-after-each-section", "on"}, > }; Here we need hw_compat_8_0 instead? Thanks,
Peter Xu <peterx@redhat.com> wrote: > On Tue, Apr 25, 2023 at 06:31:12PM +0200, Juan Quintela wrote: >> We used to flush all channels at the end of each RAM section >> sent. That is not needed, so preparing to only flush after a full >> iteration through all the RAM. >> >> Default value of the property is false. But we return "true" in >> migrate_multifd_flush_after_each_section() until we implement the code >> in following patches. >> >> 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> > > PS: Normally I'll just keep the last Sign-off-by for each person. :) And here we are again O:-) I have a hook to put that in. And at some point it did the wrong thing (i.e. this), and I always forgot to look into old series for this error. Sorry, fixed. >> >> --- >> >> Rename each-iteration to after-each-section >> Rename multifd-sync-after-each-section to >> multifd-flush-after-each-section >> --- >> hw/core/machine.c | 1 + >> migration/migration.c | 13 +++++++++++++ >> migration/migration.h | 13 +++++++++++++ >> 3 files changed, 27 insertions(+) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index 2ce97a5d3b..32bd9277b3 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -60,6 +60,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-flush-after-each-section", "on"}, >> }; > > Here we need hw_compat_8_0 instead? Good catch. Changed.
diff --git a/hw/core/machine.c b/hw/core/machine.c index 2ce97a5d3b..32bd9277b3 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -60,6 +60,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-flush-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 d91fe9fd86..4b94360f05 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2697,6 +2697,17 @@ bool migrate_use_multifd(void) return s->capabilities[MIGRATION_CAPABILITY_MULTIFD]; } +bool migrate_multifd_flush_after_each_section(void) +{ + MigrationState *s = migrate_get_current(); + + /* + * Until the patch that remove this comment, we always return that + * the property is enabled. + */ + return true || s->multifd_flush_after_each_section; +} + bool migrate_pause_before_switchover(void) { MigrationState *s; @@ -4448,6 +4459,8 @@ static Property migration_properties[] = { send_section_footer, true), DEFINE_PROP_BOOL("decompress-error-check", MigrationState, decompress_error_check, true), + DEFINE_PROP_BOOL("multifd-flush-after-each-section", MigrationState, + multifd_flush_after_each_section, true), DEFINE_PROP_UINT8("x-clear-bitmap-shift", MigrationState, clear_bitmap_shift, CLEAR_BITMAP_SHIFT_DEFAULT), DEFINE_PROP_BOOL("x-preempt-pre-7-2", MigrationState, diff --git a/migration/migration.h b/migration/migration.h index 04e0860b4e..50b4006e93 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -404,6 +404,18 @@ struct MigrationState { */ bool preempt_pre_7_2; + /* + * flush every channel after each section sent. + * + * This assures that we can't mix pages from one iteration through + * ram pages with pages for the following iteration. We really + * only need to do this flush after we have go through all the + * dirty pages. For historical reasons, we do that after each + * section. This is suboptimal (we flush too many times). + * Default value is false. Setting this property has no effect + * until the patch that removes this comment. (since 8.1) + */ + bool multifd_flush_after_each_section; /* * This decides the size of guest memory chunk that will be used * to track dirty bitmap clearing. The size of memory chunk will @@ -463,6 +475,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_flush_after_each_section(void); #ifdef CONFIG_LINUX bool migrate_use_zero_copy_send(void);