| Message ID | fb2a51fcf30d927a2512b397cd2dc6f34c3936c4.1662052189.git.huangy81@chinatelecom.cn |
|---|---|
| State | New |
| Headers | show |
| Series | migration: introduce dirtylimit capability | expand |
huangy81@chinatelecom.cn writes: > From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> > > Introduce migration dirty-limit capability, which can > be turned on before live migration and limit dirty > page rate durty live migration. > > Introduce migrate_dirty_limit function to help check > if dirty-limit capability enabled during live migration. > > Meanwhile, refactor vcpu_dirty_rate_stat_collect > so that period can be configured instead of hardcoded. > > dirty-limit capability is kind of like auto-converge > but using dirty limit instead of traditional cpu-throttle > to throttle guest down. To enable this feature, turn on > the dirty-limit capability before live migration using > migratioin-set-capabilities, and set the parameters migrate-set-capabilities > "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably "x-vcpu-dirty-limit" > to speed up convergence. > > Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> Hmm. You make dirty-limiting as a whole a stable interface (evidence: capability "dirty-limit" is stable), but keep its two parameters unstable. Rationale behind that?
在 2022/9/2 16:07, Markus Armbruster 写道: > huangy81@chinatelecom.cn writes: > >> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >> >> Introduce migration dirty-limit capability, which can >> be turned on before live migration and limit dirty >> page rate durty live migration. >> >> Introduce migrate_dirty_limit function to help check >> if dirty-limit capability enabled during live migration. >> >> Meanwhile, refactor vcpu_dirty_rate_stat_collect >> so that period can be configured instead of hardcoded. >> >> dirty-limit capability is kind of like auto-converge >> but using dirty limit instead of traditional cpu-throttle >> to throttle guest down. To enable this feature, turn on >> the dirty-limit capability before live migration using >> migratioin-set-capabilities, and set the parameters > > migrate-set-capabilities > >> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably > > "x-vcpu-dirty-limit" > >> to speed up convergence. >> >> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> > > Hmm. You make dirty-limiting as a whole a stable interface (evidence: > capability "dirty-limit" is stable), but keep its two parameters > unstable. Rationale behind that? > Thanks Markus's comments. :) x-vcpu-dirty-limit-period is an experimental parameter indeed, as to x-vcpu-dirty-limit, i think it's resonable to be a stable parameter. These 2 parameters are introduced first time and none of them suffer heavily tests, so i also made vcpu-dirty-limit experimental last version. For dirty-limit interface, it improves the vCPU computational performance during migration indeed(see the test results in cover letter), so it sounds ok to be an stable interface. The 'x-vcpu-dirty-limit-period' parameter can be dropped, IMHO, after being proved insignificant for migration in the future, and meanwhile, x-vcpu-dirty-limit be made stable. Since I don't have much experience to introducing fresh new interface, any suggestions are welcome.
Hyman Huang <huangy81@chinatelecom.cn> writes: > 在 2022/9/2 16:07, Markus Armbruster 写道: >> huangy81@chinatelecom.cn writes: >> >>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >>> >>> Introduce migration dirty-limit capability, which can >>> be turned on before live migration and limit dirty >>> page rate durty live migration. >>> >>> Introduce migrate_dirty_limit function to help check >>> if dirty-limit capability enabled during live migration. >>> >>> Meanwhile, refactor vcpu_dirty_rate_stat_collect >>> so that period can be configured instead of hardcoded. >>> >>> dirty-limit capability is kind of like auto-converge >>> but using dirty limit instead of traditional cpu-throttle >>> to throttle guest down. To enable this feature, turn on >>> the dirty-limit capability before live migration using >>> migratioin-set-capabilities, and set the parameters >> >> migrate-set-capabilities >> >>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably >> >> "x-vcpu-dirty-limit" >> >>> to speed up convergence. >>> >>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >> >> Hmm. You make dirty-limiting as a whole a stable interface (evidence: >> capability "dirty-limit" is stable), but keep its two parameters >> unstable. Rationale behind that? >> > Thanks Markus's comments. :) > > x-vcpu-dirty-limit-period is an experimental parameter indeed, as to x-vcpu-dirty-limit, i think it's resonable to be a stable parameter. > These 2 parameters are introduced first time and none of them suffer heavily tests, so i also made vcpu-dirty-limit experimental last version. > > For dirty-limit interface, it improves the vCPU computational performance during migration indeed(see the test results in cover > letter), so it sounds ok to be an stable interface. > > The 'x-vcpu-dirty-limit-period' parameter can be dropped, IMHO, after being proved insignificant for migration in the future, and meanwhile, > x-vcpu-dirty-limit be made stable. > > Since I don't have much experience to introducing fresh new interface, > any suggestions are welcome. Is the new interface fit for purpose without use of any experimental parameter? If the answer is something like "command dirty-limit improves things even without use of experimental parameters, but using them may well improve things more (but we need more testing to know for sure)", then your current use of 'unstable' may make sense.
在 2022/9/5 17:32, Markus Armbruster 写道: > Hyman Huang <huangy81@chinatelecom.cn> writes: > >> 在 2022/9/2 16:07, Markus Armbruster 写道: >>> huangy81@chinatelecom.cn writes: >>> >>>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >>>> >>>> Introduce migration dirty-limit capability, which can >>>> be turned on before live migration and limit dirty >>>> page rate durty live migration. >>>> >>>> Introduce migrate_dirty_limit function to help check >>>> if dirty-limit capability enabled during live migration. >>>> >>>> Meanwhile, refactor vcpu_dirty_rate_stat_collect >>>> so that period can be configured instead of hardcoded. >>>> >>>> dirty-limit capability is kind of like auto-converge >>>> but using dirty limit instead of traditional cpu-throttle >>>> to throttle guest down. To enable this feature, turn on >>>> the dirty-limit capability before live migration using >>>> migratioin-set-capabilities, and set the parameters >>> >>> migrate-set-capabilities >>> >>>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably >>> >>> "x-vcpu-dirty-limit" >>> >>>> to speed up convergence. >>>> >>>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >>> >>> Hmm. You make dirty-limiting as a whole a stable interface (evidence: >>> capability "dirty-limit" is stable), but keep its two parameters >>> unstable. Rationale behind that? >>> >> Thanks Markus's comments. :) >> >> x-vcpu-dirty-limit-period is an experimental parameter indeed, as to x-vcpu-dirty-limit, i think it's resonable to be a stable parameter. >> These 2 parameters are introduced first time and none of them suffer heavily tests, so i also made vcpu-dirty-limit experimental last version. >> >> For dirty-limit interface, it improves the vCPU computational performance during migration indeed(see the test results in cover >> letter), so it sounds ok to be an stable interface. >> >> The 'x-vcpu-dirty-limit-period' parameter can be dropped, IMHO, after being proved insignificant for migration in the future, and meanwhile, >> x-vcpu-dirty-limit be made stable. >> >> Since I don't have much experience to introducing fresh new interface, >> any suggestions are welcome. > > Is the new interface fit for purpose without use of any experimental > parameter? > > If the answer is something like "command dirty-limit improves things > even without use of experimental parameters, but using them may well > improve things more (but we need more testing to know for sure)", then > your current use of 'unstable' may make sense. > Yes, with the default value of parameter，the new interface works ok and improve performance. For x-vcpu-dirty-limit, we provide it because user may not want virtual CPU throttle heavily, x-vcpu-dirty-limit is kind of like cpu-throttle-percentage, which is used to setup the threshold when making guest down. For x-vcpu-dirty-limit-period, it is just as you said: "command dirty-limit improves things even without use of experimental parameters, but using them may wellimprove things more (but we need more testing to know for sure)" So, should i make x-vcpu-dirty-limit non-experimental next version?
Hyman Huang <huangy81@chinatelecom.cn> writes: > 在 2022/9/5 17:32, Markus Armbruster 写道: >> Hyman Huang <huangy81@chinatelecom.cn> writes: >> >>> 在 2022/9/2 16:07, Markus Armbruster 写道: >>>> huangy81@chinatelecom.cn writes: >>>> >>>>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >>>>> >>>>> Introduce migration dirty-limit capability, which can >>>>> be turned on before live migration and limit dirty >>>>> page rate durty live migration. >>>>> >>>>> Introduce migrate_dirty_limit function to help check >>>>> if dirty-limit capability enabled during live migration. >>>>> >>>>> Meanwhile, refactor vcpu_dirty_rate_stat_collect >>>>> so that period can be configured instead of hardcoded. >>>>> >>>>> dirty-limit capability is kind of like auto-converge >>>>> but using dirty limit instead of traditional cpu-throttle >>>>> to throttle guest down. To enable this feature, turn on >>>>> the dirty-limit capability before live migration using >>>>> migratioin-set-capabilities, and set the parameters >>>> >>>> migrate-set-capabilities >>>> >>>>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably >>>> >>>> "x-vcpu-dirty-limit" >>>> >>>>> to speed up convergence. >>>>> >>>>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> >>>> >>>> Hmm. You make dirty-limiting as a whole a stable interface (evidence: >>>> capability "dirty-limit" is stable), but keep its two parameters >>>> unstable. Rationale behind that? >>>> >>> Thanks Markus's comments. :) >>> >>> x-vcpu-dirty-limit-period is an experimental parameter indeed, as to x-vcpu-dirty-limit, i think it's resonable to be a stable parameter. >>> These 2 parameters are introduced first time and none of them suffer heavily tests, so i also made vcpu-dirty-limit experimental last version. >>> >>> For dirty-limit interface, it improves the vCPU computational performance during migration indeed(see the test results in cover >>> letter), so it sounds ok to be an stable interface. >>> >>> The 'x-vcpu-dirty-limit-period' parameter can be dropped, IMHO, after being proved insignificant for migration in the future, and meanwhile, >>> x-vcpu-dirty-limit be made stable. >>> >>> Since I don't have much experience to introducing fresh new interface, >>> any suggestions are welcome. >> Is the new interface fit for purpose without use of any experimental >> parameter? >> > If the answer is something like "command dirty-limit improves things >> even without use of experimental parameters, but using them may well >> improve things more (but we need more testing to know for sure)", then >> your current use of 'unstable' may make sense. >> > Yes, with the default value of parameter，the new interface works ok and improve performance. > > For x-vcpu-dirty-limit, we provide it because user may not want virtual CPU throttle heavily, x-vcpu-dirty-limit is kind of like > cpu-throttle-percentage, which is used to setup the threshold when making guest down. > > For x-vcpu-dirty-limit-period, it is just as you said: "command dirty-limit improves things even without use of experimental parameters, > but using them may wellimprove things more (but we need more testing to know for sure)" > > So, should i make x-vcpu-dirty-limit non-experimental next version? I think this depends on what exactly you want to signal to users. Your current patch has command dirty-limit stable and the parameters unstable. This signals "go ahead and use dirty-limit, don't worry about the parameters; even if they become stable later, using just dirty-limit will remain okay." If you keep the command unstable as well, you signal the entire interface isn't quite baked, yet. That's a much weaker proposition. So weak in fact that you cannot go wrong :) In short, it boils down to whether you want to encourage use of a part of the evolving interface *now*. Make that part stable. Requires confidence in that part, obviously.
diff --git a/migration/migration.c b/migration/migration.c index a748fe5..d117bb4 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2508,6 +2508,15 @@ bool migrate_auto_converge(void) return s->enabled_capabilities[MIGRATION_CAPABILITY_AUTO_CONVERGE]; } +bool migrate_dirty_limit(void) +{ + MigrationState *s; + + s = migrate_get_current(); + + return s->enabled_capabilities[MIGRATION_CAPABILITY_DIRTY_LIMIT]; +} + bool migrate_zero_blocks(void) { MigrationState *s; @@ -4437,6 +4446,7 @@ static Property migration_properties[] = { DEFINE_PROP_MIG_CAP("x-zero-copy-send", MIGRATION_CAPABILITY_ZERO_COPY_SEND), #endif + DEFINE_PROP_MIG_CAP("x-dirty-limit", MIGRATION_CAPABILITY_DIRTY_LIMIT), DEFINE_PROP_END_OF_LIST(), }; diff --git a/migration/migration.h b/migration/migration.h index cdad8ac..7fbb9f8 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -409,6 +409,7 @@ bool migrate_ignore_shared(void); bool migrate_validate_uuid(void); bool migrate_auto_converge(void); +bool migrate_dirty_limit(void); bool migrate_use_multifd(void); bool migrate_pause_before_switchover(void); int migrate_multifd_channels(void); diff --git a/qapi/migration.json b/qapi/migration.json index 8554d33..bc4bc96 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -477,6 +477,8 @@ # will be handled faster. This is a performance feature and # should not affect the correctness of postcopy migration. # (since 7.1) +# @dirty-limit: Use dirty-limit to throttle down guest if enabled. +# (since 7.1) # # Features: # @unstable: Members @x-colo and @x-ignore-shared are experimental. @@ -492,7 +494,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', 'dirty-limit'] } ## # @MigrationCapabilityStatus: diff --git a/softmmu/dirtylimit.c b/softmmu/dirtylimit.c index 8d98cb7..1fdd8c6 100644 --- a/softmmu/dirtylimit.c +++ b/softmmu/dirtylimit.c @@ -23,6 +23,8 @@ #include "exec/memory.h" #include "hw/boards.h" #include "sysemu/kvm.h" +#include "migration/misc.h" +#include "migration/migration.h" #include "trace.h" /* @@ -75,11 +77,18 @@ static bool dirtylimit_quit; static void vcpu_dirty_rate_stat_collect(void) { + MigrationState *s = migrate_get_current(); VcpuStat stat; int i = 0; + int64_t period = DIRTYLIMIT_CALC_TIME_MS; + + if (migrate_dirty_limit() && + migration_is_active(s)) { + period = s->parameters.x_vcpu_dirty_limit_period; + } /* calculate vcpu dirtyrate */ - vcpu_calculate_dirtyrate(DIRTYLIMIT_CALC_TIME_MS, + vcpu_calculate_dirtyrate(period, &stat, GLOBAL_DIRTY_LIMIT, false);