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.
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();
+ }
+
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.0On 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
>
--
Peter Xu
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
>
>
--
Best regards
© 2016 - 2024 Red Hat, Inc.