| Message ID | 20240216224002.1476890-2-hao.xiang@bytedance.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce multifd zero page checking. | expand |
Hao Xiang <hao.xiang@bytedance.com> writes: > This new parameter controls where the zero page checking is running. > 1. If this parameter is set to 'legacy', zero page checking is > done in the migration main thread. > 2. If this parameter is set to 'none', zero page checking is disabled. > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> [...] > diff --git a/qapi/migration.json b/qapi/migration.json > index 5a565d9b8d..99843a8e95 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -653,6 +653,17 @@ > { 'enum': 'MigMode', > 'data': [ 'normal', 'cpr-reboot' ] } > > +## > +# @ZeroPageDetection: > +# > +# @legacy: Perform zero page checking from main migration thread. (since 9.0) > +# > +# @none: Do not perform zero page checking. > +# > +## The entire type is since 9.0. Thus: ## # @ZeroPageDetection: # # @legacy: Perform zero page checking from main migration thread. # # @none: Do not perform zero page checking. # # Since: 9.0 ## > +{ 'enum': 'ZeroPageDetection', > + 'data': [ 'legacy', 'none' ] } > + > ## > # @BitmapMigrationBitmapAliasTransform: > # > @@ -874,6 +885,9 @@ > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > # (Since 8.2) > # > +# @zero-page-detection: See description in @ZeroPageDetection. > +# Default is 'legacy'. (Since 9.0) The description feels a bit lazy :) Suggest # @zero-page-detection: Whether and how to detect zero pages. Default # is 'legacy'. (since 9.0) Same for the other two copies. > +# > # Features: > # > # @deprecated: Member @block-incremental is deprecated. Use > @@ -907,7 +921,8 @@ > 'block-bitmap-mapping', > { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] }, > 'vcpu-dirty-limit', > - 'mode'] } > + 'mode', > + 'zero-page-detection'] } > > ## > # @MigrateSetParameters: > @@ -1066,6 +1081,10 @@ > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > # (Since 8.2) > # > +# @zero-page-detection: See description in @ZeroPageDetection. > +# Default is 'legacy'. (Since 9.0) > +# > +# > # Features: > # > # @deprecated: Member @block-incremental is deprecated. Use > @@ -1119,7 +1138,8 @@ > '*x-vcpu-dirty-limit-period': { 'type': 'uint64', > 'features': [ 'unstable' ] }, > '*vcpu-dirty-limit': 'uint64', > - '*mode': 'MigMode'} } > + '*mode': 'MigMode', > + '*zero-page-detection': 'ZeroPageDetection'} } > > ## > # @migrate-set-parameters: > @@ -1294,6 +1314,9 @@ > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > # (Since 8.2) > # > +# @zero-page-detection: See description in @ZeroPageDetection. > +# Default is 'legacy'. (Since 9.0) > +# > # Features: > # > # @deprecated: Member @block-incremental is deprecated. Use > @@ -1344,7 +1367,8 @@ > '*x-vcpu-dirty-limit-period': { 'type': 'uint64', > 'features': [ 'unstable' ] }, > '*vcpu-dirty-limit': 'uint64', > - '*mode': 'MigMode'} } > + '*mode': 'MigMode', > + '*zero-page-detection': 'ZeroPageDetection'} } > > ## > # @query-migrate-parameters:
On Fri, Feb 16, 2024 at 2:41 PM Hao Xiang <hao.xiang@bytedance.com> wrote: > This new parameter controls where the zero page checking is running. > 1. If this parameter is set to 'legacy', zero page checking is > done in the migration main thread. > 2. If this parameter is set to 'none', zero page checking is disabled. > > Hello Hao Few questions and comments. First the commit message states that the parameter control where the checking is done, but it also controls if sending of zero pages is done by multifd threads or not. > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > --- > hw/core/qdev-properties-system.c | 10 ++++++++++ > include/hw/qdev-properties-system.h | 4 ++++ > migration/migration-hmp-cmds.c | 9 +++++++++ > migration/options.c | 21 ++++++++++++++++++++ > migration/options.h | 1 + > migration/ram.c | 4 ++++ > qapi/migration.json | 30 ++++++++++++++++++++++++++--- > 7 files changed, 76 insertions(+), 3 deletions(-) > > diff --git a/hw/core/qdev-properties-system.c > b/hw/core/qdev-properties-system.c > index 1a396521d5..63843f18b5 100644 > --- a/hw/core/qdev-properties-system.c > +++ b/hw/core/qdev-properties-system.c > @@ -679,6 +679,16 @@ const PropertyInfo qdev_prop_mig_mode = { > .set_default_value = qdev_propinfo_set_default_value_enum, > }; > > +const PropertyInfo qdev_prop_zero_page_detection = { > + .name = "ZeroPageDetection", > + .description = "zero_page_detection values, " > + "multifd,legacy,none", > + .enum_table = &ZeroPageDetection_lookup, > + .get = qdev_propinfo_get_enum, > + .set = qdev_propinfo_set_enum, > + .set_default_value = qdev_propinfo_set_default_value_enum, > +}; > + > /* --- Reserved Region --- */ > > /* > diff --git a/include/hw/qdev-properties-system.h > b/include/hw/qdev-properties-system.h > index 06c359c190..839b170235 100644 > --- a/include/hw/qdev-properties-system.h > +++ b/include/hw/qdev-properties-system.h > @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr; > extern const PropertyInfo qdev_prop_reserved_region; > extern const PropertyInfo qdev_prop_multifd_compression; > extern const PropertyInfo qdev_prop_mig_mode; > +extern const PropertyInfo qdev_prop_zero_page_detection; > extern const PropertyInfo qdev_prop_losttickpolicy; > extern const PropertyInfo qdev_prop_blockdev_on_error; > extern const PropertyInfo qdev_prop_bios_chs_trans; > @@ -47,6 +48,9 @@ extern const PropertyInfo > qdev_prop_iothread_vq_mapping_list; > #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \ > DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \ > MigMode) > +#define DEFINE_PROP_ZERO_PAGE_DETECTION(_n, _s, _f, _d) \ > + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_zero_page_detection, \ > + ZeroPageDetection) > #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ > DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ > LostTickPolicy) > diff --git a/migration/migration-hmp-cmds.c > b/migration/migration-hmp-cmds.c > index 99b49df5dd..7e96ae6ffd 100644 > --- a/migration/migration-hmp-cmds.c > +++ b/migration/migration-hmp-cmds.c > @@ -344,6 +344,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const > QDict *qdict) > monitor_printf(mon, "%s: %s\n", > > MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION), > MultiFDCompression_str(params->multifd_compression)); > + assert(params->has_zero_page_detection); > What is the reason to have assert here? > + monitor_printf(mon, "%s: %s\n", > + > MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION), > + qapi_enum_lookup(&ZeroPageDetection_lookup, > + params->zero_page_detection)); > monitor_printf(mon, "%s: %" PRIu64 " bytes\n", > MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE), > params->xbzrle_cache_size); > @@ -634,6 +639,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const > QDict *qdict) > p->has_multifd_zstd_level = true; > visit_type_uint8(v, param, &p->multifd_zstd_level, &err); > break; > + case MIGRATION_PARAMETER_ZERO_PAGE_DETECTION: > + p->has_zero_page_detection = true; > + visit_type_ZeroPageDetection(v, param, &p->zero_page_detection, > &err); > + break; > case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE: > p->has_xbzrle_cache_size = true; > if (!visit_type_size(v, param, &cache_size, &err)) { > diff --git a/migration/options.c b/migration/options.c > index 3e3e0b93b4..3c603391b0 100644 > --- a/migration/options.c > +++ b/migration/options.c > @@ -179,6 +179,9 @@ Property migration_properties[] = { > DEFINE_PROP_MIG_MODE("mode", MigrationState, > parameters.mode, > MIG_MODE_NORMAL), > + DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState, > + parameters.zero_page_detection, > + ZERO_PAGE_DETECTION_LEGACY), > > /* Migration capabilities */ > DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), > @@ -903,6 +906,13 @@ uint64_t migrate_xbzrle_cache_size(void) > return s->parameters.xbzrle_cache_size; > } > > +ZeroPageDetection migrate_zero_page_detection(void) > +{ > + MigrationState *s = migrate_get_current(); > + > + return s->parameters.zero_page_detection; > +} > + > /* parameter setters */ > > void migrate_set_block_incremental(bool value) > @@ -1013,6 +1023,8 @@ MigrationParameters > *qmp_query_migrate_parameters(Error **errp) > params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit; > params->has_mode = true; > params->mode = s->parameters.mode; > + params->has_zero_page_detection = true; > + params->zero_page_detection = s->parameters.zero_page_detection; > > return params; > } > @@ -1049,6 +1061,7 @@ void migrate_params_init(MigrationParameters *params) > params->has_x_vcpu_dirty_limit_period = true; > params->has_vcpu_dirty_limit = true; > params->has_mode = true; > + params->has_zero_page_detection = true; > } > > /* > @@ -1350,6 +1363,10 @@ static void > migrate_params_test_apply(MigrateSetParameters *params, > if (params->has_mode) { > dest->mode = params->mode; > } > + > + if (params->has_zero_page_detection) { > + dest->zero_page_detection = params->zero_page_detection; > + } > } > > static void migrate_params_apply(MigrateSetParameters *params, Error > **errp) > @@ -1494,6 +1511,10 @@ static void > migrate_params_apply(MigrateSetParameters *params, Error **errp) > if (params->has_mode) { > s->parameters.mode = params->mode; > } > + > + if (params->has_zero_page_detection) { > + s->parameters.zero_page_detection = params->zero_page_detection; > + } > } > > void qmp_migrate_set_parameters(MigrateSetParameters *params, Error > **errp) > diff --git a/migration/options.h b/migration/options.h > index 246c160aee..b7c4fb3861 100644 > --- a/migration/options.h > +++ b/migration/options.h > @@ -93,6 +93,7 @@ const char *migrate_tls_authz(void); > const char *migrate_tls_creds(void); > const char *migrate_tls_hostname(void); > uint64_t migrate_xbzrle_cache_size(void); > +ZeroPageDetection migrate_zero_page_detection(void); > > /* parameters setters */ > > diff --git a/migration/ram.c b/migration/ram.c > index 4649a81204..556725c30f 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -1123,6 +1123,10 @@ 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; > } > diff --git a/qapi/migration.json b/qapi/migration.json > index 5a565d9b8d..99843a8e95 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -653,6 +653,17 @@ > { 'enum': 'MigMode', > 'data': [ 'normal', 'cpr-reboot' ] } > > +## > +# @ZeroPageDetection: > +# > +# @legacy: Perform zero page checking from main migration thread. (since > 9.0) > +# > +# @none: Do not perform zero page checking. > +# > +## > +{ 'enum': 'ZeroPageDetection', > + 'data': [ 'legacy', 'none' ] } > + > Above you have introduced the qdev property qdev_prop_zero_page_detection with multifd, but it is not present in the scheme. Perhaps 'mulitfd' in qdev_prop_zero_page_detection belongs to another patch? > ## > # @BitmapMigrationBitmapAliasTransform: > # > @@ -874,6 +885,9 @@ > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > # (Since 8.2) > # > +# @zero-page-detection: See description in @ZeroPageDetection. > +# Default is 'legacy'. (Since 9.0) > +# > # Features: > # > # @deprecated: Member @block-incremental is deprecated. Use > @@ -907,7 +921,8 @@ > 'block-bitmap-mapping', > { 'name': 'x-vcpu-dirty-limit-period', 'features': > ['unstable'] }, > 'vcpu-dirty-limit', > - 'mode'] } > + 'mode', > + 'zero-page-detection'] } > > ## > # @MigrateSetParameters: > @@ -1066,6 +1081,10 @@ > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > # (Since 8.2) > # > +# @zero-page-detection: See description in @ZeroPageDetection. > +# Default is 'legacy'. (Since 9.0) > +# > +# > # Features: > # > # @deprecated: Member @block-incremental is deprecated. Use > @@ -1119,7 +1138,8 @@ > '*x-vcpu-dirty-limit-period': { 'type': 'uint64', > 'features': [ 'unstable' ] }, > '*vcpu-dirty-limit': 'uint64', > - '*mode': 'MigMode'} } > + '*mode': 'MigMode', > + '*zero-page-detection': 'ZeroPageDetection'} } > > ## > # @migrate-set-parameters: > @@ -1294,6 +1314,9 @@ > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > # (Since 8.2) > # > +# @zero-page-detection: See description in @ZeroPageDetection. > +# Default is 'legacy'. (Since 9.0) > +# > # Features: > # > # @deprecated: Member @block-incremental is deprecated. Use > @@ -1344,7 +1367,8 @@ > '*x-vcpu-dirty-limit-period': { 'type': 'uint64', > 'features': [ 'unstable' ] }, > '*vcpu-dirty-limit': 'uint64', > - '*mode': 'MigMode'} } > + '*mode': 'MigMode', > + '*zero-page-detection': 'ZeroPageDetection'} } > > ## > # @query-migrate-parameters: > -- > 2.30.2 > > >
On Fri, Feb 16, 2024 at 10:39:56PM +0000, Hao Xiang wrote: > @@ -1123,6 +1123,10 @@ 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; > + } Nitpick: use "== NONE" here seems clearer. > + > if (!buffer_is_zero(p, TARGET_PAGE_SIZE)) { > return 0; > }
On Wed, Feb 21, 2024 at 4:03 AM Markus Armbruster <armbru@redhat.com> wrote: > > Hao Xiang <hao.xiang@bytedance.com> writes: > > > This new parameter controls where the zero page checking is running. > > 1. If this parameter is set to 'legacy', zero page checking is > > done in the migration main thread. > > 2. If this parameter is set to 'none', zero page checking is disabled. > > > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > > [...] > > > diff --git a/qapi/migration.json b/qapi/migration.json > > index 5a565d9b8d..99843a8e95 100644 > > --- a/qapi/migration.json > > +++ b/qapi/migration.json > > @@ -653,6 +653,17 @@ > > { 'enum': 'MigMode', > > 'data': [ 'normal', 'cpr-reboot' ] } > > > > +## > > +# @ZeroPageDetection: > > +# > > +# @legacy: Perform zero page checking from main migration thread. (since 9.0) > > +# > > +# @none: Do not perform zero page checking. > > +# > > +## > > The entire type is since 9.0. Thus: > > ## > # @ZeroPageDetection: > # > # @legacy: Perform zero page checking from main migration thread. > # > # @none: Do not perform zero page checking. > # > # Since: 9.0 > ## > > > +{ 'enum': 'ZeroPageDetection', > > + 'data': [ 'legacy', 'none' ] } > > + > > ## > > # @BitmapMigrationBitmapAliasTransform: > > # > > @@ -874,6 +885,9 @@ > > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > > # (Since 8.2) > > # > > +# @zero-page-detection: See description in @ZeroPageDetection. > > +# Default is 'legacy'. (Since 9.0) > > The description feels a bit lazy :) > > Suggest > > # @zero-page-detection: Whether and how to detect zero pages. Default > # is 'legacy'. (since 9.0) > > Same for the other two copies. I will fix these in the next version. > > > +# > > # Features: > > # > > # @deprecated: Member @block-incremental is deprecated. Use > > @@ -907,7 +921,8 @@ > > 'block-bitmap-mapping', > > { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] }, > > 'vcpu-dirty-limit', > > - 'mode'] } > > + 'mode', > > + 'zero-page-detection'] } > > > > ## > > # @MigrateSetParameters: > > @@ -1066,6 +1081,10 @@ > > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > > # (Since 8.2) > > # > > +# @zero-page-detection: See description in @ZeroPageDetection. > > +# Default is 'legacy'. (Since 9.0) > > +# > > +# > > # Features: > > # > > # @deprecated: Member @block-incremental is deprecated. Use > > @@ -1119,7 +1138,8 @@ > > '*x-vcpu-dirty-limit-period': { 'type': 'uint64', > > 'features': [ 'unstable' ] }, > > '*vcpu-dirty-limit': 'uint64', > > - '*mode': 'MigMode'} } > > + '*mode': 'MigMode', > > + '*zero-page-detection': 'ZeroPageDetection'} } > > > > ## > > # @migrate-set-parameters: > > @@ -1294,6 +1314,9 @@ > > # @mode: Migration mode. See description in @MigMode. Default is 'normal'. > > # (Since 8.2) > > # > > +# @zero-page-detection: See description in @ZeroPageDetection. > > +# Default is 'legacy'. (Since 9.0) > > +# > > # Features: > > # > > # @deprecated: Member @block-incremental is deprecated. Use > > @@ -1344,7 +1367,8 @@ > > '*x-vcpu-dirty-limit-period': { 'type': 'uint64', > > 'features': [ 'unstable' ] }, > > '*vcpu-dirty-limit': 'uint64', > > - '*mode': 'MigMode'} } > > + '*mode': 'MigMode', > > + '*zero-page-detection': 'ZeroPageDetection'} } > > > > ## > > # @query-migrate-parameters: >
On Wed, Feb 21, 2024 at 5:58 AM Elena Ufimtseva <ufimtseva@gmail.com> wrote: > > > > On Fri, Feb 16, 2024 at 2:41 PM Hao Xiang <hao.xiang@bytedance.com> wrote: >> >> This new parameter controls where the zero page checking is running. >> 1. If this parameter is set to 'legacy', zero page checking is >> done in the migration main thread. >> 2. If this parameter is set to 'none', zero page checking is disabled. >> > > Hello Hao > > Few questions and comments. > > First the commit message states that the parameter control where the checking is done, but it also controls > if sending of zero pages is done by multifd threads or not. > > >> >> Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> >> --- >> hw/core/qdev-properties-system.c | 10 ++++++++++ >> include/hw/qdev-properties-system.h | 4 ++++ >> migration/migration-hmp-cmds.c | 9 +++++++++ >> migration/options.c | 21 ++++++++++++++++++++ >> migration/options.h | 1 + >> migration/ram.c | 4 ++++ >> qapi/migration.json | 30 ++++++++++++++++++++++++++--- >> 7 files changed, 76 insertions(+), 3 deletions(-) >> >> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c >> index 1a396521d5..63843f18b5 100644 >> --- a/hw/core/qdev-properties-system.c >> +++ b/hw/core/qdev-properties-system.c >> @@ -679,6 +679,16 @@ const PropertyInfo qdev_prop_mig_mode = { >> .set_default_value = qdev_propinfo_set_default_value_enum, >> }; >> >> +const PropertyInfo qdev_prop_zero_page_detection = { >> + .name = "ZeroPageDetection", >> + .description = "zero_page_detection values, " >> + "multifd,legacy,none", >> + .enum_table = &ZeroPageDetection_lookup, >> + .get = qdev_propinfo_get_enum, >> + .set = qdev_propinfo_set_enum, >> + .set_default_value = qdev_propinfo_set_default_value_enum, >> +}; >> + >> /* --- Reserved Region --- */ >> >> /* >> diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h >> index 06c359c190..839b170235 100644 >> --- a/include/hw/qdev-properties-system.h >> +++ b/include/hw/qdev-properties-system.h >> @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr; >> extern const PropertyInfo qdev_prop_reserved_region; >> extern const PropertyInfo qdev_prop_multifd_compression; >> extern const PropertyInfo qdev_prop_mig_mode; >> +extern const PropertyInfo qdev_prop_zero_page_detection; >> extern const PropertyInfo qdev_prop_losttickpolicy; >> extern const PropertyInfo qdev_prop_blockdev_on_error; >> extern const PropertyInfo qdev_prop_bios_chs_trans; >> @@ -47,6 +48,9 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; >> #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \ >> DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \ >> MigMode) >> +#define DEFINE_PROP_ZERO_PAGE_DETECTION(_n, _s, _f, _d) \ >> + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_zero_page_detection, \ >> + ZeroPageDetection) >> #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ >> DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ >> LostTickPolicy) >> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c >> index 99b49df5dd..7e96ae6ffd 100644 >> --- a/migration/migration-hmp-cmds.c >> +++ b/migration/migration-hmp-cmds.c >> @@ -344,6 +344,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) >> monitor_printf(mon, "%s: %s\n", >> MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION), >> MultiFDCompression_str(params->multifd_compression)); >> + assert(params->has_zero_page_detection); > > > What is the reason to have assert here? It's just to verify that the option is initialized properly before we reach here. Same things are done for other options. > >> >> + monitor_printf(mon, "%s: %s\n", >> + MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION), >> + qapi_enum_lookup(&ZeroPageDetection_lookup, >> + params->zero_page_detection)); >> monitor_printf(mon, "%s: %" PRIu64 " bytes\n", >> MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE), >> params->xbzrle_cache_size); >> @@ -634,6 +639,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> p->has_multifd_zstd_level = true; >> visit_type_uint8(v, param, &p->multifd_zstd_level, &err); >> break; >> + case MIGRATION_PARAMETER_ZERO_PAGE_DETECTION: >> + p->has_zero_page_detection = true; >> + visit_type_ZeroPageDetection(v, param, &p->zero_page_detection, &err); >> + break; >> case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE: >> p->has_xbzrle_cache_size = true; >> if (!visit_type_size(v, param, &cache_size, &err)) { >> diff --git a/migration/options.c b/migration/options.c >> index 3e3e0b93b4..3c603391b0 100644 >> --- a/migration/options.c >> +++ b/migration/options.c >> @@ -179,6 +179,9 @@ Property migration_properties[] = { >> DEFINE_PROP_MIG_MODE("mode", MigrationState, >> parameters.mode, >> MIG_MODE_NORMAL), >> + DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState, >> + parameters.zero_page_detection, >> + ZERO_PAGE_DETECTION_LEGACY), >> >> /* Migration capabilities */ >> DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), >> @@ -903,6 +906,13 @@ uint64_t migrate_xbzrle_cache_size(void) >> return s->parameters.xbzrle_cache_size; >> } >> >> +ZeroPageDetection migrate_zero_page_detection(void) >> +{ >> + MigrationState *s = migrate_get_current(); >> + >> + return s->parameters.zero_page_detection; >> +} >> + >> /* parameter setters */ >> >> void migrate_set_block_incremental(bool value) >> @@ -1013,6 +1023,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) >> params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit; >> params->has_mode = true; >> params->mode = s->parameters.mode; >> + params->has_zero_page_detection = true; >> + params->zero_page_detection = s->parameters.zero_page_detection; >> >> return params; >> } >> @@ -1049,6 +1061,7 @@ void migrate_params_init(MigrationParameters *params) >> params->has_x_vcpu_dirty_limit_period = true; >> params->has_vcpu_dirty_limit = true; >> params->has_mode = true; >> + params->has_zero_page_detection = true; >> } >> >> /* >> @@ -1350,6 +1363,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params, >> if (params->has_mode) { >> dest->mode = params->mode; >> } >> + >> + if (params->has_zero_page_detection) { >> + dest->zero_page_detection = params->zero_page_detection; >> + } >> } >> >> static void migrate_params_apply(MigrateSetParameters *params, Error **errp) >> @@ -1494,6 +1511,10 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp) >> if (params->has_mode) { >> s->parameters.mode = params->mode; >> } >> + >> + if (params->has_zero_page_detection) { >> + s->parameters.zero_page_detection = params->zero_page_detection; >> + } >> } >> >> void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp) >> diff --git a/migration/options.h b/migration/options.h >> index 246c160aee..b7c4fb3861 100644 >> --- a/migration/options.h >> +++ b/migration/options.h >> @@ -93,6 +93,7 @@ const char *migrate_tls_authz(void); >> const char *migrate_tls_creds(void); >> const char *migrate_tls_hostname(void); >> uint64_t migrate_xbzrle_cache_size(void); >> +ZeroPageDetection migrate_zero_page_detection(void); >> >> /* parameters setters */ >> >> diff --git a/migration/ram.c b/migration/ram.c >> index 4649a81204..556725c30f 100644 >> --- a/migration/ram.c >> +++ b/migration/ram.c >> @@ -1123,6 +1123,10 @@ 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; >> } >> diff --git a/qapi/migration.json b/qapi/migration.json >> index 5a565d9b8d..99843a8e95 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json >> @@ -653,6 +653,17 @@ >> { 'enum': 'MigMode', >> 'data': [ 'normal', 'cpr-reboot' ] } >> >> +## >> +# @ZeroPageDetection: >> +# >> +# @legacy: Perform zero page checking from main migration thread. (since 9.0) >> +# >> +# @none: Do not perform zero page checking. >> +# >> +## >> +{ 'enum': 'ZeroPageDetection', >> + 'data': [ 'legacy', 'none' ] } >> + > > > Above you have introduced the qdev property qdev_prop_zero_page_detection with multifd, but it is not present in the scheme. > Perhaps 'mulitfd' in qdev_prop_zero_page_detection belongs to another patch? You are right. I will fix that. > > >> >> ## >> # @BitmapMigrationBitmapAliasTransform: >> # >> @@ -874,6 +885,9 @@ >> # @mode: Migration mode. See description in @MigMode. Default is 'normal'. >> # (Since 8.2) >> # >> +# @zero-page-detection: See description in @ZeroPageDetection. >> +# Default is 'legacy'. (Since 9.0) >> +# >> # Features: >> # >> # @deprecated: Member @block-incremental is deprecated. Use >> @@ -907,7 +921,8 @@ >> 'block-bitmap-mapping', >> { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] }, >> 'vcpu-dirty-limit', >> - 'mode'] } >> + 'mode', >> + 'zero-page-detection'] } >> >> ## >> # @MigrateSetParameters: >> @@ -1066,6 +1081,10 @@ >> # @mode: Migration mode. See description in @MigMode. Default is 'normal'. >> # (Since 8.2) >> # >> +# @zero-page-detection: See description in @ZeroPageDetection. >> +# Default is 'legacy'. (Since 9.0) >> +# >> +# >> # Features: >> # >> # @deprecated: Member @block-incremental is deprecated. Use >> @@ -1119,7 +1138,8 @@ >> '*x-vcpu-dirty-limit-period': { 'type': 'uint64', >> 'features': [ 'unstable' ] }, >> '*vcpu-dirty-limit': 'uint64', >> - '*mode': 'MigMode'} } >> + '*mode': 'MigMode', >> + '*zero-page-detection': 'ZeroPageDetection'} } >> >> ## >> # @migrate-set-parameters: >> @@ -1294,6 +1314,9 @@ >> # @mode: Migration mode. See description in @MigMode. Default is 'normal'. >> # (Since 8.2) >> # >> +# @zero-page-detection: See description in @ZeroPageDetection. >> +# Default is 'legacy'. (Since 9.0) >> +# >> # Features: >> # >> # @deprecated: Member @block-incremental is deprecated. Use >> @@ -1344,7 +1367,8 @@ >> '*x-vcpu-dirty-limit-period': { 'type': 'uint64', >> 'features': [ 'unstable' ] }, >> '*vcpu-dirty-limit': 'uint64', >> - '*mode': 'MigMode'} } >> + '*mode': 'MigMode', >> + '*zero-page-detection': 'ZeroPageDetection'} } >> >> ## >> # @query-migrate-parameters: >> -- >> 2.30.2 >> >> > > > -- > Elena
On 2/17/2024 6:39, Hao Xiang wrote: > This new parameter controls where the zero page checking is running. > 1. If this parameter is set to 'legacy', zero page checking is > done in the migration main thread. > 2. If this parameter is set to 'none', zero page checking is disabled. > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > --- > hw/core/qdev-properties-system.c | 10 ++++++++++ > include/hw/qdev-properties-system.h | 4 ++++ > migration/migration-hmp-cmds.c | 9 +++++++++ > migration/options.c | 21 ++++++++++++++++++++ > migration/options.h | 1 + > migration/ram.c | 4 ++++ > qapi/migration.json | 30 ++++++++++++++++++++++++++--- > 7 files changed, 76 insertions(+), 3 deletions(-) > > diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c > index 1a396521d5..63843f18b5 100644 > --- a/hw/core/qdev-properties-system.c > +++ b/hw/core/qdev-properties-system.c > @@ -679,6 +679,16 @@ const PropertyInfo qdev_prop_mig_mode = { > .set_default_value = qdev_propinfo_set_default_value_enum, > }; > > +const PropertyInfo qdev_prop_zero_page_detection = { > + .name = "ZeroPageDetection", > + .description = "zero_page_detection values, " > + "multifd,legacy,none", Nit: Maybe multifd/legacy/none?
On Sun, Feb 25, 2024 at 11:19 PM Wang, Lei <lei4.wang@intel.com> wrote: > > On 2/17/2024 6:39, Hao Xiang wrote: > > This new parameter controls where the zero page checking is running. > > 1. If this parameter is set to 'legacy', zero page checking is > > done in the migration main thread. > > 2. If this parameter is set to 'none', zero page checking is disabled. > > > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > > --- > > hw/core/qdev-properties-system.c | 10 ++++++++++ > > include/hw/qdev-properties-system.h | 4 ++++ > > migration/migration-hmp-cmds.c | 9 +++++++++ > > migration/options.c | 21 ++++++++++++++++++++ > > migration/options.h | 1 + > > migration/ram.c | 4 ++++ > > qapi/migration.json | 30 ++++++++++++++++++++++++++--- > > 7 files changed, 76 insertions(+), 3 deletions(-) > > > > diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c > > index 1a396521d5..63843f18b5 100644 > > --- a/hw/core/qdev-properties-system.c > > +++ b/hw/core/qdev-properties-system.c > > @@ -679,6 +679,16 @@ const PropertyInfo qdev_prop_mig_mode = { > > .set_default_value = qdev_propinfo_set_default_value_enum, > > }; > > > > +const PropertyInfo qdev_prop_zero_page_detection = { > > + .name = "ZeroPageDetection", > > + .description = "zero_page_detection values, " > > + "multifd,legacy,none", > > Nit: Maybe multifd/legacy/none? I changed it to .description = "zero_page_detection values, " "none,legacy,multifd", Since both "," and "/" are used in existing code, I think it would be fine either way.
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 1a396521d5..63843f18b5 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -679,6 +679,16 @@ const PropertyInfo qdev_prop_mig_mode = { .set_default_value = qdev_propinfo_set_default_value_enum, }; +const PropertyInfo qdev_prop_zero_page_detection = { + .name = "ZeroPageDetection", + .description = "zero_page_detection values, " + "multifd,legacy,none", + .enum_table = &ZeroPageDetection_lookup, + .get = qdev_propinfo_get_enum, + .set = qdev_propinfo_set_enum, + .set_default_value = qdev_propinfo_set_default_value_enum, +}; + /* --- Reserved Region --- */ /* diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h index 06c359c190..839b170235 100644 --- a/include/hw/qdev-properties-system.h +++ b/include/hw/qdev-properties-system.h @@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr; extern const PropertyInfo qdev_prop_reserved_region; extern const PropertyInfo qdev_prop_multifd_compression; extern const PropertyInfo qdev_prop_mig_mode; +extern const PropertyInfo qdev_prop_zero_page_detection; extern const PropertyInfo qdev_prop_losttickpolicy; extern const PropertyInfo qdev_prop_blockdev_on_error; extern const PropertyInfo qdev_prop_bios_chs_trans; @@ -47,6 +48,9 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \ MigMode) +#define DEFINE_PROP_ZERO_PAGE_DETECTION(_n, _s, _f, _d) \ + DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_zero_page_detection, \ + ZeroPageDetection) #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \ LostTickPolicy) diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index 99b49df5dd..7e96ae6ffd 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -344,6 +344,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, "%s: %s\n", MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION), MultiFDCompression_str(params->multifd_compression)); + assert(params->has_zero_page_detection); + monitor_printf(mon, "%s: %s\n", + MigrationParameter_str(MIGRATION_PARAMETER_ZERO_PAGE_DETECTION), + qapi_enum_lookup(&ZeroPageDetection_lookup, + params->zero_page_detection)); monitor_printf(mon, "%s: %" PRIu64 " bytes\n", MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE), params->xbzrle_cache_size); @@ -634,6 +639,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) p->has_multifd_zstd_level = true; visit_type_uint8(v, param, &p->multifd_zstd_level, &err); break; + case MIGRATION_PARAMETER_ZERO_PAGE_DETECTION: + p->has_zero_page_detection = true; + visit_type_ZeroPageDetection(v, param, &p->zero_page_detection, &err); + break; case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE: p->has_xbzrle_cache_size = true; if (!visit_type_size(v, param, &cache_size, &err)) { diff --git a/migration/options.c b/migration/options.c index 3e3e0b93b4..3c603391b0 100644 --- a/migration/options.c +++ b/migration/options.c @@ -179,6 +179,9 @@ Property migration_properties[] = { DEFINE_PROP_MIG_MODE("mode", MigrationState, parameters.mode, MIG_MODE_NORMAL), + DEFINE_PROP_ZERO_PAGE_DETECTION("zero-page-detection", MigrationState, + parameters.zero_page_detection, + ZERO_PAGE_DETECTION_LEGACY), /* Migration capabilities */ DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), @@ -903,6 +906,13 @@ uint64_t migrate_xbzrle_cache_size(void) return s->parameters.xbzrle_cache_size; } +ZeroPageDetection migrate_zero_page_detection(void) +{ + MigrationState *s = migrate_get_current(); + + return s->parameters.zero_page_detection; +} + /* parameter setters */ void migrate_set_block_incremental(bool value) @@ -1013,6 +1023,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp) params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit; params->has_mode = true; params->mode = s->parameters.mode; + params->has_zero_page_detection = true; + params->zero_page_detection = s->parameters.zero_page_detection; return params; } @@ -1049,6 +1061,7 @@ void migrate_params_init(MigrationParameters *params) params->has_x_vcpu_dirty_limit_period = true; params->has_vcpu_dirty_limit = true; params->has_mode = true; + params->has_zero_page_detection = true; } /* @@ -1350,6 +1363,10 @@ static void migrate_params_test_apply(MigrateSetParameters *params, if (params->has_mode) { dest->mode = params->mode; } + + if (params->has_zero_page_detection) { + dest->zero_page_detection = params->zero_page_detection; + } } static void migrate_params_apply(MigrateSetParameters *params, Error **errp) @@ -1494,6 +1511,10 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp) if (params->has_mode) { s->parameters.mode = params->mode; } + + if (params->has_zero_page_detection) { + s->parameters.zero_page_detection = params->zero_page_detection; + } } void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp) diff --git a/migration/options.h b/migration/options.h index 246c160aee..b7c4fb3861 100644 --- a/migration/options.h +++ b/migration/options.h @@ -93,6 +93,7 @@ const char *migrate_tls_authz(void); const char *migrate_tls_creds(void); const char *migrate_tls_hostname(void); uint64_t migrate_xbzrle_cache_size(void); +ZeroPageDetection migrate_zero_page_detection(void); /* parameters setters */ diff --git a/migration/ram.c b/migration/ram.c index 4649a81204..556725c30f 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1123,6 +1123,10 @@ 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; } diff --git a/qapi/migration.json b/qapi/migration.json index 5a565d9b8d..99843a8e95 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -653,6 +653,17 @@ { 'enum': 'MigMode', 'data': [ 'normal', 'cpr-reboot' ] } +## +# @ZeroPageDetection: +# +# @legacy: Perform zero page checking from main migration thread. (since 9.0) +# +# @none: Do not perform zero page checking. +# +## +{ 'enum': 'ZeroPageDetection', + 'data': [ 'legacy', 'none' ] } + ## # @BitmapMigrationBitmapAliasTransform: # @@ -874,6 +885,9 @@ # @mode: Migration mode. See description in @MigMode. Default is 'normal'. # (Since 8.2) # +# @zero-page-detection: See description in @ZeroPageDetection. +# Default is 'legacy'. (Since 9.0) +# # Features: # # @deprecated: Member @block-incremental is deprecated. Use @@ -907,7 +921,8 @@ 'block-bitmap-mapping', { 'name': 'x-vcpu-dirty-limit-period', 'features': ['unstable'] }, 'vcpu-dirty-limit', - 'mode'] } + 'mode', + 'zero-page-detection'] } ## # @MigrateSetParameters: @@ -1066,6 +1081,10 @@ # @mode: Migration mode. See description in @MigMode. Default is 'normal'. # (Since 8.2) # +# @zero-page-detection: See description in @ZeroPageDetection. +# Default is 'legacy'. (Since 9.0) +# +# # Features: # # @deprecated: Member @block-incremental is deprecated. Use @@ -1119,7 +1138,8 @@ '*x-vcpu-dirty-limit-period': { 'type': 'uint64', 'features': [ 'unstable' ] }, '*vcpu-dirty-limit': 'uint64', - '*mode': 'MigMode'} } + '*mode': 'MigMode', + '*zero-page-detection': 'ZeroPageDetection'} } ## # @migrate-set-parameters: @@ -1294,6 +1314,9 @@ # @mode: Migration mode. See description in @MigMode. Default is 'normal'. # (Since 8.2) # +# @zero-page-detection: See description in @ZeroPageDetection. +# Default is 'legacy'. (Since 9.0) +# # Features: # # @deprecated: Member @block-incremental is deprecated. Use @@ -1344,7 +1367,8 @@ '*x-vcpu-dirty-limit-period': { 'type': 'uint64', 'features': [ 'unstable' ] }, '*vcpu-dirty-limit': 'uint64', - '*mode': 'MigMode'} } + '*mode': 'MigMode', + '*zero-page-detection': 'ZeroPageDetection'} } ## # @query-migrate-parameters:
This new parameter controls where the zero page checking is running. 1. If this parameter is set to 'legacy', zero page checking is done in the migration main thread. 2. If this parameter is set to 'none', zero page checking is disabled. Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> --- hw/core/qdev-properties-system.c | 10 ++++++++++ include/hw/qdev-properties-system.h | 4 ++++ migration/migration-hmp-cmds.c | 9 +++++++++ migration/options.c | 21 ++++++++++++++++++++ migration/options.h | 1 + migration/ram.c | 4 ++++ qapi/migration.json | 30 ++++++++++++++++++++++++++--- 7 files changed, 76 insertions(+), 3 deletions(-)