Re: [RFC v3 2/4] cpu-throttle: new module, extracted from cpus.c

From: Claudio Fontana <cfontana@suse.de>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Peter Maydell" <peter.maydell@linaro.org>
Cc: Laurent Vivier <lvivier@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Colin Xu <colin.xu@intel.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	qemu-devel@nongnu.org, Roman Bolshakov <r.bolshakov@yadro.com>,
	haxm-team@intel.com, Wenchao Wang <wenchao.wang@intel.com>,
	Sunil Muthuswamy <sunilmut@microsoft.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [RFC v3 2/4] cpu-throttle: new module, extracted from cpus.c
Date: Mon, 25 May 2020 17:42:27 +0200	[thread overview]
Message-ID: <c1387681-fe93-d42b-c2b6-40b91c6801f1@suse.de> (raw)
In-Reply-To: <99995c3f-68a3-ce07-72bb-37a9b2263e52@redhat.com>

On 5/25/20 5:14 PM, Philippe Mathieu-Daudé wrote:
> On 5/25/20 4:54 PM, 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.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  MAINTAINERS                   |   3 +-
>>  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 +
>>  softmmu/Makefile.objs         |   1 +
>>  softmmu/cpu-throttle.c        | 122 ++++++++++++++++++++++++++++++++++++++++++
>>  softmmu/cpus.c                |  95 +++-----------------------------
>>  9 files changed, 190 insertions(+), 125 deletions(-)
>>  create mode 100644 include/sysemu/cpu-throttle.h
>>  create mode 100644 softmmu/cpu-throttle.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 0288ffbc50..708768f120 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2147,13 +2147,14 @@ F: ui/cocoa.m
>>  Main loop
>>  M: Paolo Bonzini <pbonzini@redhat.com>
>>  S: Maintained
>> -F: softmmu/cpus.c
>>  F: include/qemu/main-loop.h
>>  F: include/sysemu/runstate.h
>>  F: util/main-loop.c
>>  F: util/qemu-timer.c
>>  F: softmmu/vl.c
>>  F: softmmu/main.c
>> +F: softmmu/cpus.c
> 
> This line belong the the previous patch (#1).

right

> 
>> +F: softmmu/cpu-throttle.c
>>  F: qapi/run-state.json
> 
> Can you reorder patches #1/#2 to avoid moving cpu-throttle code twice?

I have no preference, will wait for more comments then, as you suggested elsewhere.

> 
> Otherwise this patch looks good.
> 
> [...]
> 
> 