| Message ID | e41bdd8dde51403a25b817ec49e860f9b515b793.1729064919.git.yong.huang@smartx.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | migration: auto-converge refinements for huge VM | expand |
On Wed, Oct 16, 2024 at 03:56:42PM +0800, yong.huang@smartx.com wrote: > From: Hyman Huang <yong.huang@smartx.com> > > Move cpu-throttle.c from system to migration since it's > only used for migration; this makes us avoid exporting the > util functions and variables in misc.h but export them in > migration.h when implementing the background ramblock dirty > sync feature in the upcoming commits. > > Additionally, make the two modifications below: > > 1. Delay the timer registering of CPU throttle until > migration starts since it is only used in migration. > > 2. Stop CPU throttle if auto converge capability is > enabled since it only happens with auto converge. > > 3. Remove the unused header file reference in > accel/tcg/icount-common.c. Please consider split the things into smaller patches, especially when it involves file movements. > > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > --- > accel/tcg/icount-common.c | 1 - > {system => migration}/cpu-throttle.c | 2 +- > {include/sysemu => migration}/cpu-throttle.h | 0 > migration/meson.build | 1 + > migration/migration.c | 11 +++++++++-- > migration/ram.c | 2 +- > migration/trace-events | 3 +++ > system/cpu-timers.c | 3 --- > system/meson.build | 1 - > system/trace-events | 3 --- > 10 files changed, 15 insertions(+), 12 deletions(-) > rename {system => migration}/cpu-throttle.c (99%) > rename {include/sysemu => migration}/cpu-throttle.h (100%) > > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c > index 8d3d3a7e9d..30bf8500dc 100644 > --- a/accel/tcg/icount-common.c > +++ b/accel/tcg/icount-common.c > @@ -36,7 +36,6 @@ > #include "sysemu/runstate.h" > #include "hw/core/cpu.h" > #include "sysemu/cpu-timers.h" > -#include "sysemu/cpu-throttle.h" > #include "sysemu/cpu-timers-internal.h" > > /* > diff --git a/system/cpu-throttle.c b/migration/cpu-throttle.c > similarity index 99% > rename from system/cpu-throttle.c > rename to migration/cpu-throttle.c > index 7632dc6143..fa47ee2e21 100644 > --- a/system/cpu-throttle.c > +++ b/migration/cpu-throttle.c > @@ -27,7 +27,7 @@ > #include "hw/core/cpu.h" > #include "qemu/main-loop.h" > #include "sysemu/cpus.h" > -#include "sysemu/cpu-throttle.h" > +#include "cpu-throttle.h" > #include "trace.h" > > /* vcpu throttling controls */ > diff --git a/include/sysemu/cpu-throttle.h b/migration/cpu-throttle.h > similarity index 100% > rename from include/sysemu/cpu-throttle.h > rename to migration/cpu-throttle.h > diff --git a/migration/meson.build b/migration/meson.build > index 66d3de86f0..d53cf3417a 100644 > --- a/migration/meson.build > +++ b/migration/meson.build > @@ -13,6 +13,7 @@ system_ss.add(files( > 'block-dirty-bitmap.c', > 'channel.c', > 'channel-block.c', > + 'cpu-throttle.c', > 'dirtyrate.c', > 'exec.c', > 'fd.c', > diff --git a/migration/migration.c b/migration/migration.c > index 021faee2f3..7e71184257 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -24,7 +24,7 @@ > #include "socket.h" > #include "sysemu/runstate.h" > #include "sysemu/sysemu.h" > -#include "sysemu/cpu-throttle.h" > +#include "cpu-throttle.h" > #include "rdma.h" > #include "ram.h" > #include "migration/global_state.h" > @@ -3289,7 +3289,9 @@ static MigIterateState migration_iteration_run(MigrationState *s) > static void migration_iteration_finish(MigrationState *s) > { > /* If we enabled cpu throttling for auto-converge, turn it off. */ > - cpu_throttle_stop(); > + if (migrate_auto_converge()) { > + cpu_throttle_stop(); > + } > > bql_lock(); > switch (s->state) { > @@ -3508,6 +3510,11 @@ static void *migration_thread(void *opaque) > qemu_savevm_send_colo_enable(s->to_dst_file); > } > > + if (migrate_auto_converge()) { > + /* Start cpu throttle timers */ > + cpu_throttle_init(); > + } Might this leak the timer object? I think it perhaps needs to be moved to migration_object_init(). > + > bql_lock(); > ret = qemu_savevm_state_setup(s->to_dst_file, &local_err); > bql_unlock(); > diff --git a/migration/ram.c b/migration/ram.c > index 326ce7eb79..54d352b152 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -52,7 +52,7 @@ > #include "exec/target_page.h" > #include "qemu/rcu_queue.h" > #include "migration/colo.h" > -#include "sysemu/cpu-throttle.h" > +#include "cpu-throttle.h" > #include "savevm.h" > #include "qemu/iov.h" > #include "multifd.h" > diff --git a/migration/trace-events b/migration/trace-events > index c65902f042..9a19599804 100644 > --- a/migration/trace-events > +++ b/migration/trace-events > @@ -378,3 +378,6 @@ migration_block_progression(unsigned percent) "Completed %u%%" > # page_cache.c > migration_pagecache_init(int64_t max_num_items) "Setting cache buckets to %" PRId64 > migration_pagecache_insert(void) "Error allocating page" > + > +# cpu-throttle.c > +cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%" > diff --git a/system/cpu-timers.c b/system/cpu-timers.c > index 0b31c9a1b6..856e502e34 100644 > --- a/system/cpu-timers.c > +++ b/system/cpu-timers.c > @@ -35,7 +35,6 @@ > #include "sysemu/runstate.h" > #include "hw/core/cpu.h" > #include "sysemu/cpu-timers.h" > -#include "sysemu/cpu-throttle.h" > #include "sysemu/cpu-timers-internal.h" > > /* clock and ticks */ > @@ -272,6 +271,4 @@ void cpu_timers_init(void) > seqlock_init(&timers_state.vm_clock_seqlock); > qemu_spin_init(&timers_state.vm_clock_lock); > vmstate_register(NULL, 0, &vmstate_timers, &timers_state); > - > - cpu_throttle_init(); > } > diff --git a/system/meson.build b/system/meson.build > index a296270cb0..4952f4b2c7 100644 > --- a/system/meson.build > +++ b/system/meson.build > @@ -10,7 +10,6 @@ system_ss.add(files( > 'balloon.c', > 'bootdevice.c', > 'cpus.c', > - 'cpu-throttle.c', > 'cpu-timers.c', > 'datadir.c', > 'dirtylimit.c', > diff --git a/system/trace-events b/system/trace-events > index 074d001e90..2ed1d59b1f 100644 > --- a/system/trace-events > +++ b/system/trace-events > @@ -44,6 +44,3 @@ dirtylimit_state_finalize(void) > dirtylimit_throttle_pct(int cpu_index, uint64_t pct, int64_t time_us) "CPU[%d] throttle percent: %" PRIu64 ", throttle adjust time %"PRIi64 " us" > dirtylimit_set_vcpu(int cpu_index, uint64_t quota) "CPU[%d] set dirty page rate limit %"PRIu64 > dirtylimit_vcpu_execute(int cpu_index, int64_t sleep_time_us) "CPU[%d] sleep %"PRIi64 " us" > - > -# cpu-throttle.c > -cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%" > -- > 2.27.0 >
On Wed, Oct 16, 2024 at 11:50 PM Peter Xu <peterx@redhat.com> wrote: > On Wed, Oct 16, 2024 at 03:56:42PM +0800, yong.huang@smartx.com wrote: > > From: Hyman Huang <yong.huang@smartx.com> > > > > Move cpu-throttle.c from system to migration since it's > > only used for migration; this makes us avoid exporting the > > util functions and variables in misc.h but export them in > > migration.h when implementing the background ramblock dirty > > sync feature in the upcoming commits. > > > > Additionally, make the two modifications below: > > > > 1. Delay the timer registering of CPU throttle until > > migration starts since it is only used in migration. > > > > 2. Stop CPU throttle if auto converge capability is > > enabled since it only happens with auto converge. > > > > 3. Remove the unused header file reference in > > accel/tcg/icount-common.c. > > Please consider split the things into smaller patches, especially when it > involves file movements. > OK. > > > > Signed-off-by: Hyman Huang <yong.huang@smartx.com> > > --- > > accel/tcg/icount-common.c | 1 - > > {system => migration}/cpu-throttle.c | 2 +- > > {include/sysemu => migration}/cpu-throttle.h | 0 > > migration/meson.build | 1 + > > migration/migration.c | 11 +++++++++-- > > migration/ram.c | 2 +- > > migration/trace-events | 3 +++ > > system/cpu-timers.c | 3 --- > > system/meson.build | 1 - > > system/trace-events | 3 --- > > 10 files changed, 15 insertions(+), 12 deletions(-) > > rename {system => migration}/cpu-throttle.c (99%) > > rename {include/sysemu => migration}/cpu-throttle.h (100%) > > > > diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c > > index 8d3d3a7e9d..30bf8500dc 100644 > > --- a/accel/tcg/icount-common.c > > +++ b/accel/tcg/icount-common.c > > @@ -36,7 +36,6 @@ > > #include "sysemu/runstate.h" > > #include "hw/core/cpu.h" > > #include "sysemu/cpu-timers.h" > > -#include "sysemu/cpu-throttle.h" > > #include "sysemu/cpu-timers-internal.h" > > > > /* > > diff --git a/system/cpu-throttle.c b/migration/cpu-throttle.c > > similarity index 99% > > rename from system/cpu-throttle.c > > rename to migration/cpu-throttle.c > > index 7632dc6143..fa47ee2e21 100644 > > --- a/system/cpu-throttle.c > > +++ b/migration/cpu-throttle.c > > @@ -27,7 +27,7 @@ > > #include "hw/core/cpu.h" > > #include "qemu/main-loop.h" > > #include "sysemu/cpus.h" > > -#include "sysemu/cpu-throttle.h" > > +#include "cpu-throttle.h" > > #include "trace.h" > > > > /* vcpu throttling controls */ > > diff --git a/include/sysemu/cpu-throttle.h b/migration/cpu-throttle.h > > similarity index 100% > > rename from include/sysemu/cpu-throttle.h > > rename to migration/cpu-throttle.h > > diff --git a/migration/meson.build b/migration/meson.build > > index 66d3de86f0..d53cf3417a 100644 > > --- a/migration/meson.build > > +++ b/migration/meson.build > > @@ -13,6 +13,7 @@ system_ss.add(files( > > 'block-dirty-bitmap.c', > > 'channel.c', > > 'channel-block.c', > > + 'cpu-throttle.c', > > 'dirtyrate.c', > > 'exec.c', > > 'fd.c', > > diff --git a/migration/migration.c b/migration/migration.c > > index 021faee2f3..7e71184257 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -24,7 +24,7 @@ > > #include "socket.h" > > #include "sysemu/runstate.h" > > #include "sysemu/sysemu.h" > > -#include "sysemu/cpu-throttle.h" > > +#include "cpu-throttle.h" > > #include "rdma.h" > > #include "ram.h" > > #include "migration/global_state.h" > > @@ -3289,7 +3289,9 @@ static MigIterateState > migration_iteration_run(MigrationState *s) > > static void migration_iteration_finish(MigrationState *s) > > { > > /* If we enabled cpu throttling for auto-converge, turn it off. */ > > - cpu_throttle_stop(); > > + if (migrate_auto_converge()) { > > + cpu_throttle_stop(); > > + } > > > > bql_lock(); > > switch (s->state) { > > @@ -3508,6 +3510,11 @@ static void *migration_thread(void *opaque) > > qemu_savevm_send_colo_enable(s->to_dst_file); > > } > > > > + if (migrate_auto_converge()) { > > + /* Start cpu throttle timers */ > > + cpu_throttle_init(); > > + } > > Might this leak the timer object? > I think it perhaps needs to be moved to migration_object_init(). > Ok, I'll fix it in the next version. > > > + > > bql_lock(); > > ret = qemu_savevm_state_setup(s->to_dst_file, &local_err); > > bql_unlock(); > > diff --git a/migration/ram.c b/migration/ram.c > > index 326ce7eb79..54d352b152 100644 > > --- a/migration/ram.c > > +++ b/migration/ram.c > > @@ -52,7 +52,7 @@ > > #include "exec/target_page.h" > > #include "qemu/rcu_queue.h" > > #include "migration/colo.h" > > -#include "sysemu/cpu-throttle.h" > > +#include "cpu-throttle.h" > > #include "savevm.h" > > #include "qemu/iov.h" > > #include "multifd.h" > > diff --git a/migration/trace-events b/migration/trace-events > > index c65902f042..9a19599804 100644 > > --- a/migration/trace-events > > +++ b/migration/trace-events > > @@ -378,3 +378,6 @@ migration_block_progression(unsigned percent) > "Completed %u%%" > > # page_cache.c > > migration_pagecache_init(int64_t max_num_items) "Setting cache buckets > to %" PRId64 > > migration_pagecache_insert(void) "Error allocating page" > > + > > +# cpu-throttle.c > > +cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by > %d%%" > > diff --git a/system/cpu-timers.c b/system/cpu-timers.c > > index 0b31c9a1b6..856e502e34 100644 > > --- a/system/cpu-timers.c > > +++ b/system/cpu-timers.c > > @@ -35,7 +35,6 @@ > > #include "sysemu/runstate.h" > > #include "hw/core/cpu.h" > > #include "sysemu/cpu-timers.h" > > -#include "sysemu/cpu-throttle.h" > > #include "sysemu/cpu-timers-internal.h" > > > > /* clock and ticks */ > > @@ -272,6 +271,4 @@ void cpu_timers_init(void) > > seqlock_init(&timers_state.vm_clock_seqlock); > > qemu_spin_init(&timers_state.vm_clock_lock); > > vmstate_register(NULL, 0, &vmstate_timers, &timers_state); > > - > > - cpu_throttle_init(); > > } > > diff --git a/system/meson.build b/system/meson.build > > index a296270cb0..4952f4b2c7 100644 > > --- a/system/meson.build > > +++ b/system/meson.build > > @@ -10,7 +10,6 @@ system_ss.add(files( > > 'balloon.c', > > 'bootdevice.c', > > 'cpus.c', > > - 'cpu-throttle.c', > > 'cpu-timers.c', > > 'datadir.c', > > 'dirtylimit.c', > > diff --git a/system/trace-events b/system/trace-events > > index 074d001e90..2ed1d59b1f 100644 > > --- a/system/trace-events > > +++ b/system/trace-events > > @@ -44,6 +44,3 @@ dirtylimit_state_finalize(void) > > dirtylimit_throttle_pct(int cpu_index, uint64_t pct, int64_t time_us) > "CPU[%d] throttle percent: %" PRIu64 ", throttle adjust time %"PRIi64 " us" > > dirtylimit_set_vcpu(int cpu_index, uint64_t quota) "CPU[%d] set dirty > page rate limit %"PRIu64 > > dirtylimit_vcpu_execute(int cpu_index, int64_t sleep_time_us) "CPU[%d] > sleep %"PRIi64 " us" > > - > > -# cpu-throttle.c > > -cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by > %d%%" > > -- > > 2.27.0 > > > > -- > Peter Xu > >
diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c index 8d3d3a7e9d..30bf8500dc 100644 --- a/accel/tcg/icount-common.c +++ b/accel/tcg/icount-common.c @@ -36,7 +36,6 @@ #include "sysemu/runstate.h" #include "hw/core/cpu.h" #include "sysemu/cpu-timers.h" -#include "sysemu/cpu-throttle.h" #include "sysemu/cpu-timers-internal.h" /* diff --git a/system/cpu-throttle.c b/migration/cpu-throttle.c similarity index 99% rename from system/cpu-throttle.c rename to migration/cpu-throttle.c index 7632dc6143..fa47ee2e21 100644 --- a/system/cpu-throttle.c +++ b/migration/cpu-throttle.c @@ -27,7 +27,7 @@ #include "hw/core/cpu.h" #include "qemu/main-loop.h" #include "sysemu/cpus.h" -#include "sysemu/cpu-throttle.h" +#include "cpu-throttle.h" #include "trace.h" /* vcpu throttling controls */ diff --git a/include/sysemu/cpu-throttle.h b/migration/cpu-throttle.h similarity index 100% rename from include/sysemu/cpu-throttle.h rename to migration/cpu-throttle.h diff --git a/migration/meson.build b/migration/meson.build index 66d3de86f0..d53cf3417a 100644 --- a/migration/meson.build +++ b/migration/meson.build @@ -13,6 +13,7 @@ system_ss.add(files( 'block-dirty-bitmap.c', 'channel.c', 'channel-block.c', + 'cpu-throttle.c', 'dirtyrate.c', 'exec.c', 'fd.c', diff --git a/migration/migration.c b/migration/migration.c index 021faee2f3..7e71184257 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -24,7 +24,7 @@ #include "socket.h" #include "sysemu/runstate.h" #include "sysemu/sysemu.h" -#include "sysemu/cpu-throttle.h" +#include "cpu-throttle.h" #include "rdma.h" #include "ram.h" #include "migration/global_state.h" @@ -3289,7 +3289,9 @@ static MigIterateState migration_iteration_run(MigrationState *s) static void migration_iteration_finish(MigrationState *s) { /* If we enabled cpu throttling for auto-converge, turn it off. */ - cpu_throttle_stop(); + if (migrate_auto_converge()) { + cpu_throttle_stop(); + } bql_lock(); switch (s->state) { @@ -3508,6 +3510,11 @@ static void *migration_thread(void *opaque) qemu_savevm_send_colo_enable(s->to_dst_file); } + if (migrate_auto_converge()) { + /* Start cpu throttle timers */ + cpu_throttle_init(); + } + bql_lock(); ret = qemu_savevm_state_setup(s->to_dst_file, &local_err); bql_unlock(); diff --git a/migration/ram.c b/migration/ram.c index 326ce7eb79..54d352b152 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -52,7 +52,7 @@ #include "exec/target_page.h" #include "qemu/rcu_queue.h" #include "migration/colo.h" -#include "sysemu/cpu-throttle.h" +#include "cpu-throttle.h" #include "savevm.h" #include "qemu/iov.h" #include "multifd.h" diff --git a/migration/trace-events b/migration/trace-events index c65902f042..9a19599804 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -378,3 +378,6 @@ migration_block_progression(unsigned percent) "Completed %u%%" # page_cache.c migration_pagecache_init(int64_t max_num_items) "Setting cache buckets to %" PRId64 migration_pagecache_insert(void) "Error allocating page" + +# cpu-throttle.c +cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%" diff --git a/system/cpu-timers.c b/system/cpu-timers.c index 0b31c9a1b6..856e502e34 100644 --- a/system/cpu-timers.c +++ b/system/cpu-timers.c @@ -35,7 +35,6 @@ #include "sysemu/runstate.h" #include "hw/core/cpu.h" #include "sysemu/cpu-timers.h" -#include "sysemu/cpu-throttle.h" #include "sysemu/cpu-timers-internal.h" /* clock and ticks */ @@ -272,6 +271,4 @@ void cpu_timers_init(void) seqlock_init(&timers_state.vm_clock_seqlock); qemu_spin_init(&timers_state.vm_clock_lock); vmstate_register(NULL, 0, &vmstate_timers, &timers_state); - - cpu_throttle_init(); } diff --git a/system/meson.build b/system/meson.build index a296270cb0..4952f4b2c7 100644 --- a/system/meson.build +++ b/system/meson.build @@ -10,7 +10,6 @@ system_ss.add(files( 'balloon.c', 'bootdevice.c', 'cpus.c', - 'cpu-throttle.c', 'cpu-timers.c', 'datadir.c', 'dirtylimit.c', diff --git a/system/trace-events b/system/trace-events index 074d001e90..2ed1d59b1f 100644 --- a/system/trace-events +++ b/system/trace-events @@ -44,6 +44,3 @@ dirtylimit_state_finalize(void) dirtylimit_throttle_pct(int cpu_index, uint64_t pct, int64_t time_us) "CPU[%d] throttle percent: %" PRIu64 ", throttle adjust time %"PRIi64 " us" dirtylimit_set_vcpu(int cpu_index, uint64_t quota) "CPU[%d] set dirty page rate limit %"PRIu64 dirtylimit_vcpu_execute(int cpu_index, int64_t sleep_time_us) "CPU[%d] sleep %"PRIi64 " us" - -# cpu-throttle.c -cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%"