On 22/05/2020 19.10, Claudio Fontana wrote: > move the vcpu throttling functionality into its own module. > > This functionality is not specific to any accelerator, > and it is used currently by migration to slow down guests to try to > have migrations converge, and by the cocoa MacOS UI to throttle speed. > > cpu-throttle contains the controls to adjust and inspect throttle > settings, start (set) and stop vcpu throttling, and the throttling > function itself that is run periodically on vcpus to make them take a nap. > > Execution of the throttling function on all vcpus is triggered by a timer, > registered at module initialization. > > No functionality change.
Thanks for the update, sounds better now! > Signed-off-by: Claudio Fontana <cfont...@suse.de> > --- > MAINTAINERS | 1 + > Makefile.target | 8 ++- > cpu-throttle.c | 122 > ++++++++++++++++++++++++++++++++++++++++++ > cpus.c | 95 +++----------------------------- > include/hw/core/cpu.h | 37 ------------- > include/qemu/main-loop.h | 5 ++ > include/sysemu/cpu-throttle.h | 50 +++++++++++++++++ > migration/migration.c | 1 + > migration/ram.c | 1 + > 9 files changed, 195 insertions(+), 125 deletions(-) > create mode 100644 cpu-throttle.c > create mode 100644 include/sysemu/cpu-throttle.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3690f313c3..95be18c0b5 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2148,6 +2148,7 @@ Main loop > M: Paolo Bonzini <pbonz...@redhat.com> > S: Maintained > F: cpus.c > +F: cpu-throttle.c > F: include/qemu/main-loop.h > F: include/sysemu/runstate.h > F: util/main-loop.c > diff --git a/Makefile.target b/Makefile.target > index 8ed1eba95b..60cfa2a78b 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -152,7 +152,13 @@ endif #CONFIG_BSD_USER > ######################################################### > # System emulator target > ifdef CONFIG_SOFTMMU > -obj-y += arch_init.o cpus.o gdbstub.o balloon.o ioport.o > +obj-y += arch_init.o > +obj-y += cpus.o > +obj-y += cpu-throttle.o Hmm, maybe the new files should rather be created in the softmmu/ folder (now that we've got it)? And cpus.c could finally be moved there, too... well, it's just an idea, I've got no strong opinion about this. > +obj-y += gdbstub.o > +obj-y += balloon.o > +obj-y += ioport.o > + > obj-y += qtest.o > obj-y += dump/ > obj-y += hw/ > diff --git a/cpu-throttle.c b/cpu-throttle.c > new file mode 100644 > index 0000000000..4e6b2818ca > --- /dev/null > +++ b/cpu-throttle.c > @@ -0,0 +1,122 @@ > +/* > + * QEMU System Emulator > + * > + * Copyright (c) 2003-2008 Fabrice Bellard > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > deal > + * in the Software without restriction, including without limitation the > rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > + > +#include "qemu/osdep.h" > +#include "qemu-common.h" > +#include "qemu/thread.h" > +#include "hw/core/cpu.h" > +#include "qemu/main-loop.h" > +#include "sysemu/cpus.h" > +#include "sysemu/cpu-throttle.h" > + > +/* vcpu throttling controls */ > +static QEMUTimer *throttle_timer; > +static unsigned int throttle_percentage; > + > +#define CPU_THROTTLE_PCT_MIN 1 > +#define CPU_THROTTLE_PCT_MAX 99 > +#define CPU_THROTTLE_TIMESLICE_NS 10000000 > + > +static void cpu_throttle_thread(CPUState *cpu, run_on_cpu_data opaque) > +{ > + double pct; > + double throttle_ratio; > + int64_t sleeptime_ns, endtime_ns; > + > + if (!cpu_throttle_get_percentage()) { > + return; > + } > + > + pct = (double)cpu_throttle_get_percentage() / 100; > + throttle_ratio = pct / (1 - pct); > + /* Add 1ns to fix double's rounding error (like 0.9999999...) */ > + sleeptime_ns = (int64_t)(throttle_ratio * CPU_THROTTLE_TIMESLICE_NS + 1); > + endtime_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + sleeptime_ns; > + while (sleeptime_ns > 0 && !cpu->stop) { > + if (sleeptime_ns > SCALE_MS) { > + qemu_cond_timedwait_iothread(cpu->halt_cond, > + sleeptime_ns / SCALE_MS); > + } else { > + qemu_mutex_unlock_iothread(); > + g_usleep(sleeptime_ns / SCALE_US); > + qemu_mutex_lock_iothread(); > + } > + sleeptime_ns = endtime_ns - qemu_clock_get_ns(QEMU_CLOCK_REALTIME); > + } > + atomic_set(&cpu->throttle_thread_scheduled, 0); > +} > + > +static void cpu_throttle_timer_tick(void *opaque) > +{ > + CPUState *cpu; > + double pct; > + > + /* Stop the timer if needed */ > + if (!cpu_throttle_get_percentage()) { > + return; > + } > + CPU_FOREACH(cpu) { > + if (!atomic_xchg(&cpu->throttle_thread_scheduled, 1)) { > + async_run_on_cpu(cpu, cpu_throttle_thread, > + RUN_ON_CPU_NULL); > + } > + } > + > + pct = (double)cpu_throttle_get_percentage() / 100; > + timer_mod(throttle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + > + CPU_THROTTLE_TIMESLICE_NS / (1 - pct)); > +} > + > +void cpu_throttle_set(int new_throttle_pct) > +{ > + /* Ensure throttle percentage is within valid range */ > + new_throttle_pct = MIN(new_throttle_pct, CPU_THROTTLE_PCT_MAX); > + new_throttle_pct = MAX(new_throttle_pct, CPU_THROTTLE_PCT_MIN); > + > + atomic_set(&throttle_percentage, new_throttle_pct); > + > + timer_mod(throttle_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + > + CPU_THROTTLE_TIMESLICE_NS); > +} > + > +void cpu_throttle_stop(void) > +{ > + atomic_set(&throttle_percentage, 0); > +} > + > +bool cpu_throttle_active(void) > +{ > + return (cpu_throttle_get_percentage() != 0); > +} > + > +int cpu_throttle_get_percentage(void) > +{ > + return atomic_read(&throttle_percentage); > +} > + > +void cpu_throttle_init(void) > +{ > + throttle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT, > + cpu_throttle_timer_tick, NULL); > +} > diff --git a/cpus.c b/cpus.c > index 5670c96bcf..3a46a4fc2b 100644 > --- a/cpus.c > +++ b/cpus.c [...] > > +void qemu_cond_timedwait_iothread(QemuCond *cond, int ms) > +{ > + qemu_cond_timedwait(cond, &qemu_global_mutex, ms); > +} So the new function is in cpus.c ... > diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h > index a6d20b0719..2fa3d90ad6 100644 > --- a/include/qemu/main-loop.h > +++ b/include/qemu/main-loop.h > @@ -263,6 +263,11 @@ int qemu_add_child_watch(pid_t pid); > */ > bool qemu_mutex_iothread_locked(void); > > +/* > + * qemu_cond_timedwait_iothread: like the previous, but with timeout > + */ > +void qemu_cond_timedwait_iothread(QemuCond *cond, int ms); ... but it's prototype is in main-loop.h ? That's a little bit confusing... I'd rather expect it in include/sysemu/cpus.h instead? Or should the new function rather be moved to softmmu/vl.c ? Thomas