From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48865) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4EfM-00009D-IP for qemu-devel@nongnu.org; Tue, 30 Jul 2013 14:29:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V4EfF-0006bk-CZ for qemu-devel@nongnu.org; Tue, 30 Jul 2013 14:29:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14901) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V4EfF-0006bH-4b for qemu-devel@nongnu.org; Tue, 30 Jul 2013 14:28:53 -0400 Date: Tue, 30 Jul 2013 21:30:15 +0300 From: "Michael S. Tsirkin" Message-ID: <20130730183015.GA24653@redhat.com> References: <1375203359-17562-1-git-send-email-afaerber@suse.de> <1375203359-17562-3-git-send-email-afaerber@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1375203359-17562-3-git-send-email-afaerber@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC qom-cpu for-next 2/2] cpu: Replace qemu_for_each_cpu() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: qemu-devel@nongnu.org On Tue, Jul 30, 2013 at 06:55:59PM +0200, Andreas F=E4rber wrote: > It was introduced to loop over CPUs from target-independent code, but > since commit 182735efaf956ccab50b6d74a4fed163e0f35660 target-independen= t > CPUState is used. >=20 > A loop can be considered more efficient than function calls in a loop, > and CPU_FOREACH() hides implementation details just as well, so use tha= t > instead. >=20 > Suggested-by: Markus Armbruster > Signed-off-by: Andreas F=E4rber Efficiency is unlikely to be stellar seeing as there are tons of string matching going on in QOM. But I don't think it matters in any of this code, so Acked-by: Michael S. Tsirkin > --- > arch_init.c | 11 +++++------ > cpus.c | 11 ++++------- > exec.c | 9 --------- > hw/acpi/piix4.c | 20 +++++++++----------- > include/qom/cpu.h | 9 --------- > qom/cpu.c | 30 +++++++++--------------------- > 6 files changed, 27 insertions(+), 63 deletions(-) >=20 > diff --git a/arch_init.c b/arch_init.c > index 68a7ab7..148f76e 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -1195,15 +1195,14 @@ static void mig_sleep_cpu(void *opq) > much time in the VM. The migration thread will try to catchup. > Workload will experience a performance drop. > */ > -static void mig_throttle_cpu_down(CPUState *cpu, void *data) > -{ > - async_run_on_cpu(cpu, mig_sleep_cpu, NULL); > -} > - > static void mig_throttle_guest_down(void) > { > + CPUState *cpu; > + > qemu_mutex_lock_iothread(); > - qemu_for_each_cpu(mig_throttle_cpu_down, NULL); > + CPU_FOREACH(cpu) { > + async_run_on_cpu(cpu, mig_sleep_cpu, NULL); > + } > qemu_mutex_unlock_iothread(); > } > =20 > diff --git a/cpus.c b/cpus.c > index 1e2fd8a..c1d0f18 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -837,12 +837,6 @@ static void *qemu_dummy_cpu_thread_fn(void *arg) > =20 > static void tcg_exec_all(void); > =20 > -static void tcg_signal_cpu_creation(CPUState *cpu, void *data) > -{ > - cpu->thread_id =3D qemu_get_thread_id(); > - cpu->created =3D true; > -} > - > static void *qemu_tcg_cpu_thread_fn(void *arg) > { > CPUState *cpu =3D arg; > @@ -851,7 +845,10 @@ static void *qemu_tcg_cpu_thread_fn(void *arg) > qemu_thread_get_self(cpu->thread); > =20 > qemu_mutex_lock(&qemu_global_mutex); > - qemu_for_each_cpu(tcg_signal_cpu_creation, NULL); > + CPU_FOREACH(cpu) { > + cpu->thread_id =3D qemu_get_thread_id(); > + cpu->created =3D true; > + } > qemu_cond_signal(&qemu_cpu_cond); > =20 > /* wait for initial kick-off after machine start */ > diff --git a/exec.c b/exec.c > index 4ee61a6..7d90119 100644 > --- a/exec.c > +++ b/exec.c > @@ -362,15 +362,6 @@ CPUState *qemu_get_cpu(int index) > return NULL; > } > =20 > -void qemu_for_each_cpu(void (*func)(CPUState *cpu, void *data), void *= data) > -{ > - CPUState *cpu; > - > - CPU_FOREACH(cpu) { > - func(cpu, data); > - } > -} > - > void cpu_exec_init(CPUArchState *env) > { > CPUState *cpu =3D ENV_GET_CPU(env); > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index c885690..12e40b0 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -667,22 +667,14 @@ static void piix4_cpu_added_req(Notifier *n, void= *opaque) > piix4_cpu_hotplug_req(s, CPU(opaque), PLUG); > } > =20 > -static void piix4_init_cpu_status(CPUState *cpu, void *data) > -{ > - CPUStatus *g =3D (CPUStatus *)data; > - CPUClass *k =3D CPU_GET_CLASS(cpu); > - int64_t id =3D k->get_arch_id(cpu); > - > - g_assert((id / 8) < PIIX4_PROC_LEN); > - g->sts[id / 8] |=3D (1 << (id % 8)); > -} > - > static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev, > PCIHotplugState state); > =20 > static void piix4_acpi_system_hot_add_init(MemoryRegion *parent, > PCIBus *bus, PIIX4PMState *= s) > { > + CPUState *cpu; > + > memory_region_init_io(&s->io_gpe, OBJECT(s), &piix4_gpe_ops, s, > "acpi-gpe0", GPE_LEN); > memory_region_add_subregion(parent, GPE_BASE, &s->io_gpe); > @@ -693,7 +685,13 @@ static void piix4_acpi_system_hot_add_init(MemoryR= egion *parent, > &s->io_pci); > pci_bus_hotplug(bus, piix4_device_hotplug, DEVICE(s)); > =20 > - qemu_for_each_cpu(piix4_init_cpu_status, &s->gpe_cpu); > + CPU_FOREACH(cpu) { > + CPUClass *cc =3D CPU_GET_CLASS(cpu); > + int64_t id =3D cc->get_arch_id(cpu); > + > + g_assert((id / 8) < PIIX4_PROC_LEN); > + s->gpe_cpu.sts[id / 8] |=3D (1 << (id % 8)); > + } > memory_region_init_io(&s->io_cpu, OBJECT(s), &cpu_hotplug_ops, s, > "acpi-cpu-hotplug", PIIX4_PROC_LEN); > memory_region_add_subregion(parent, PIIX4_PROC_BASE, &s->io_cpu); > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 6d052ee..3cc28b5 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -401,15 +401,6 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *= data), void *data); > void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *d= ata); > =20 > /** > - * qemu_for_each_cpu: > - * @func: The function to be executed. > - * @data: Data to pass to the function. > - * > - * Executes @func for each CPU. > - */ > -void qemu_for_each_cpu(void (*func)(CPUState *cpu, void *data), void *= data); > - > -/** > * qemu_get_cpu: > * @index: The CPUState@cpu_index value of the CPU to obtain. > * > diff --git a/qom/cpu.c b/qom/cpu.c > index aa95108..ecade59 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -25,30 +25,18 @@ > #include "qemu/log.h" > #include "sysemu/sysemu.h" > =20 > -typedef struct CPUExistsArgs { > - int64_t id; > - bool found; > -} CPUExistsArgs; > - > -static void cpu_exist_cb(CPUState *cpu, void *data) > -{ > - CPUClass *klass =3D CPU_GET_CLASS(cpu); > - CPUExistsArgs *arg =3D data; > - > - if (klass->get_arch_id(cpu) =3D=3D arg->id) { > - arg->found =3D true; > - } > -} > - > bool cpu_exists(int64_t id) > { > - CPUExistsArgs data =3D { > - .id =3D id, > - .found =3D false, > - }; > + CPUState *cpu; > + > + CPU_FOREACH(cpu) { > + CPUClass *cc =3D CPU_GET_CLASS(cpu); > =20 > - qemu_for_each_cpu(cpu_exist_cb, &data); > - return data.found; > + if (cc->get_arch_id(cpu) =3D=3D id) { > + return true; > + } > + } > + return false; > } > =20 > bool cpu_paging_enabled(const CPUState *cpu) > --=20 > 1.8.1.4