| Message ID | 20230130080956.3047-9-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Multifd zero page support | expand |
Juan Quintela <quintela@redhat.com> writes: > We have to enable it by default until we introduce the new code. > > Signed-off-by: Juan Quintela <quintela@redhat.com> The subject doesn't quite match the patch to the QAPI schema. It claims "capability to enable/disable zero_page", but ... > --- > > Change it to a capability. As capabilities are off by default, have > to change MULTIFD_ZERO_PAGE to MAIN_ZERO_PAGE, so it is false for > default, and true for older versions. > --- > qapi/migration.json | 8 +++++++- > migration/migration.h | 1 + > hw/core/machine.c | 1 + > migration/migration.c | 13 ++++++++++++- > 4 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/qapi/migration.json b/qapi/migration.json > index 88ecf86ac8..ac5bc071a9 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -472,12 +472,18 @@ > # Requires that QEMU be permitted to use locked memory > # for guest RAM pages. > # (since 7.1) > +# > # @postcopy-preempt: If enabled, the migration process will allow postcopy > # requests to preempt precopy stream, so postcopy requests > # will be handled faster. This is a performance feature and > # should not affect the correctness of postcopy migration. > # (since 7.1) > # > +# @main-zero-page: If enabled, the detection of zero pages will be > +# done on the main thread. Otherwise it is done on > +# the multifd threads. ... here, we add a capability to shift certain work to another thread. No "enable/disable" as far as I can tell. Which one is right? What's the default? Not this patch's fault, but needs fixing: we neglect to document the default for several other parameters. Wordsmithing nitpick: suggest "done by the thread" or maybe "done in the thread". @main-zero-page suggests this is about a special zero page. Perhaps I can think of a clearer name, but first I need to be sure what the thing is about. > +# (since 8.0) > +# > # Features: > # @unstable: Members @x-colo and @x-ignore-shared are experimental. > # > @@ -492,7 +498,7 @@ > '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', 'main-zero-page'] } > > ## > # @MigrationCapabilityStatus:
Markus Armbruster <armbru@redhat.com> wrote: > Juan Quintela <quintela@redhat.com> writes: > >> We have to enable it by default until we introduce the new code. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > The subject doesn't quite match the patch to the QAPI schema. It claims > "capability to enable/disable zero_page", but ... And here I am, making a full of myself (again). Will change the documentation/commit informatioon, what is right is the code. Thanks a lot. Later, Juan. > >> --- >> >> Change it to a capability. As capabilities are off by default, have >> to change MULTIFD_ZERO_PAGE to MAIN_ZERO_PAGE, so it is false for >> default, and true for older versions. >> --- >> qapi/migration.json | 8 +++++++- >> migration/migration.h | 1 + >> hw/core/machine.c | 1 + >> migration/migration.c | 13 ++++++++++++- >> 4 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/qapi/migration.json b/qapi/migration.json >> index 88ecf86ac8..ac5bc071a9 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json >> @@ -472,12 +472,18 @@ >> # Requires that QEMU be permitted to use locked memory >> # for guest RAM pages. >> # (since 7.1) >> +# >> # @postcopy-preempt: If enabled, the migration process will allow postcopy >> # requests to preempt precopy stream, so postcopy requests >> # will be handled faster. This is a performance feature and >> # should not affect the correctness of postcopy migration. >> # (since 7.1) >> # >> +# @main-zero-page: If enabled, the detection of zero pages will be >> +# done on the main thread. Otherwise it is done on >> +# the multifd threads. > > ... here, we add a capability to shift certain work to another thread. > No "enable/disable" as far as I can tell. Which one is right? > > What's the default? > > Not this patch's fault, but needs fixing: we neglect to document the > default for several other parameters. > > Wordsmithing nitpick: suggest "done by the thread" or maybe "done in the > thread". > > @main-zero-page suggests this is about a special zero page. Perhaps I > can think of a clearer name, but first I need to be sure what the thing > is about. > >> +# (since 8.0) >> +# >> # Features: >> # @unstable: Members @x-colo and @x-ignore-shared are experimental. >> # >> @@ -492,7 +498,7 @@ >> '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', 'main-zero-page'] } >> >> ## >> # @MigrationCapabilityStatus:
Markus Armbruster <armbru@redhat.com> wrote: > Juan Quintela <quintela@redhat.com> writes: > >> We have to enable it by default until we introduce the new code. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > The subject doesn't quite match the patch to the QAPI schema. It claims > "capability to enable/disable zero_page", but ... And here I am, making a full of myself (again). Will change the documentation/commit information, what is right is the code. Thanks a lot. Later, Juan. > >> --- >> >> Change it to a capability. As capabilities are off by default, have >> to change MULTIFD_ZERO_PAGE to MAIN_ZERO_PAGE, so it is false for >> default, and true for older versions. >> --- >> qapi/migration.json | 8 +++++++- >> migration/migration.h | 1 + >> hw/core/machine.c | 1 + >> migration/migration.c | 13 ++++++++++++- >> 4 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/qapi/migration.json b/qapi/migration.json >> index 88ecf86ac8..ac5bc071a9 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json >> @@ -472,12 +472,18 @@ >> # Requires that QEMU be permitted to use locked memory >> # for guest RAM pages. >> # (since 7.1) >> +# >> # @postcopy-preempt: If enabled, the migration process will allow postcopy >> # requests to preempt precopy stream, so postcopy requests >> # will be handled faster. This is a performance feature and >> # should not affect the correctness of postcopy migration. >> # (since 7.1) >> # >> +# @main-zero-page: If enabled, the detection of zero pages will be >> +# done on the main thread. Otherwise it is done on >> +# the multifd threads. > > ... here, we add a capability to shift certain work to another thread. > No "enable/disable" as far as I can tell. Which one is right? > > What's the default? > > Not this patch's fault, but needs fixing: we neglect to document the > default for several other parameters. > > Wordsmithing nitpick: suggest "done by the thread" or maybe "done in the > thread". > > @main-zero-page suggests this is about a special zero page. Perhaps I > can think of a clearer name, but first I need to be sure what the thing > is about. > >> +# (since 8.0) >> +# >> # Features: >> # @unstable: Members @x-colo and @x-ignore-shared are experimental. >> # >> @@ -492,7 +498,7 @@ >> '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', 'main-zero-page'] } >> >> ## >> # @MigrationCapabilityStatus:
diff --git a/qapi/migration.json b/qapi/migration.json index 88ecf86ac8..ac5bc071a9 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -472,12 +472,18 @@ # Requires that QEMU be permitted to use locked memory # for guest RAM pages. # (since 7.1) +# # @postcopy-preempt: If enabled, the migration process will allow postcopy # requests to preempt precopy stream, so postcopy requests # will be handled faster. This is a performance feature and # should not affect the correctness of postcopy migration. # (since 7.1) # +# @main-zero-page: If enabled, the detection of zero pages will be +# done on the main thread. Otherwise it is done on +# the multifd threads. +# (since 8.0) +# # Features: # @unstable: Members @x-colo and @x-ignore-shared are experimental. # @@ -492,7 +498,7 @@ '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', 'main-zero-page'] } ## # @MigrationCapabilityStatus: diff --git a/migration/migration.h b/migration/migration.h index ae4ffd3454..c38a0baf10 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -408,6 +408,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_use_main_zero_page(void); #ifdef CONFIG_LINUX bool migrate_use_zero_copy_send(void); diff --git a/hw/core/machine.c b/hw/core/machine.c index 616f3a207c..97149e2de3 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -52,6 +52,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", "main-zero-page", "true" }, }; 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 d261c7d16d..ab86c6601d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -164,7 +164,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_MAIN_ZERO_PAGE); /* When we add fault tolerance, we could have several migrations at once. For now we don't need to add @@ -2603,6 +2604,14 @@ bool migrate_use_multifd(void) return s->enabled_capabilities[MIGRATION_CAPABILITY_MULTIFD]; } +bool migrate_use_main_zero_page(void) +{ + MigrationState *s = migrate_get_current(); + + /* We will enable this when we add the right code. */ + return true || s->enabled_capabilities[MIGRATION_CAPABILITY_MAIN_ZERO_PAGE]; +} + bool migrate_pause_before_switchover(void) { MigrationState *s; @@ -4425,6 +4434,8 @@ static Property migration_properties[] = { DEFINE_PROP_MIG_CAP("x-zero-copy-send", MIGRATION_CAPABILITY_ZERO_COPY_SEND), #endif + DEFINE_PROP_MIG_CAP("main-zero-page", + MIGRATION_CAPABILITY_MAIN_ZERO_PAGE), DEFINE_PROP_END_OF_LIST(), };
We have to enable it by default until we introduce the new code. Signed-off-by: Juan Quintela <quintela@redhat.com> --- Change it to a capability. As capabilities are off by default, have to change MULTIFD_ZERO_PAGE to MAIN_ZERO_PAGE, so it is false for default, and true for older versions. --- qapi/migration.json | 8 +++++++- migration/migration.h | 1 + hw/core/machine.c | 1 + migration/migration.c | 13 ++++++++++++- 4 files changed, 21 insertions(+), 2 deletions(-)