[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL v2 05/22] cpu: Convert cpu_index into a bitmap
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PULL v2 05/22] cpu: Convert cpu_index into a bitmap |
Date: |
Wed, 8 Jul 2015 20:25:50 -0700 |
On Wed, Jul 8, 2015 at 8:16 PM, Bharata B Rao
<address@hidden> wrote:
> On Wed, Jul 08, 2015 at 09:07:30PM +0200, Andreas Färber wrote:
>> Am 07.07.2015 um 19:16 schrieb Andreas Färber:
>> > From: Bharata B Rao <address@hidden>
>> >
>> > Currently CPUState::cpu_index is monotonically increasing and a newly
>> > created CPU always gets the next higher index. The next available
>> > index is calculated by counting the existing number of CPUs. This is
>> > fine as long as we only add CPUs, but there are architectures which
>> > are starting to support CPU removal, too. For an architecture like PowerPC
>> > which derives its CPU identifier (device tree ID) from cpu_index, the
>> > existing logic of generating cpu_index values causes problems.
>> >
>> > With the currently proposed method of handling vCPU removal by parking
>> > the vCPU fd in QEMU
>> > (Ref: http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02604.html),
>> > generating cpu_index this way will not work for PowerPC.
>> >
>> > This patch changes the way cpu_index is handed out by maintaining
>> > a bit map of the CPUs that tracks both addition and removal of CPUs.
>> >
>> > The CPU bitmap allocation logic is part of cpu_exec_init(), which is
>> > called by instance_init routines of various CPU targets. Newly added
>> > cpu_exec_exit() API handles the deallocation part and this routine is
>> > called from generic CPU instance_finalize.
>> >
>> > Note: This new CPU enumeration is for !CONFIG_USER_ONLY only.
>> > CONFIG_USER_ONLY continues to have the old enumeration logic.
>> >
>> > Signed-off-by: Bharata B Rao <address@hidden>
>> > Reviewed-by: Eduardo Habkost <address@hidden>
>> > Reviewed-by: Igor Mammedov <address@hidden>
>> > Reviewed-by: David Gibson <address@hidden>
>> > Reviewed-by: Peter Crosthwaite <address@hidden>
>> > Acked-by: Paolo Bonzini <address@hidden>
>> > Signed-off-by: Peter Crosthwaite <address@hidden>
>> > [AF: max_cpus -> MAX_CPUMASK_BITS]
>> > Signed-off-by: Andreas Färber <address@hidden>
>> > ---
>> > exec.c | 55
>> > ++++++++++++++++++++++++++++++++++++++++++++++++++-----
>> > include/qom/cpu.h | 1 +
>> > qom/cpu.c | 7 +++++++
>> > 3 files changed, 58 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/exec.c b/exec.c
>> > index 8abac69..02602b6 100644
>> > --- a/exec.c
>> > +++ b/exec.c
>> [...]
>> > @@ -542,11 +587,11 @@ void cpu_exec_init(CPUArchState *env, Error **errp)
>> > #if defined(CONFIG_USER_ONLY)
>> > cpu_list_lock();
>> > #endif
>> > - cpu_index = 0;
>> > - CPU_FOREACH(some_cpu) {
>> > - cpu_index++;
>> > + cpu_index = cpu->cpu_index = cpu_get_free_index(&local_err);
>> > + if (local_err) {
>> > + error_propagate(errp, local_err);
>>
>> This is lacking a matching cpu_list_unlock() in the CONFIG_USER_ONLY case:
>>
>> diff --git a/exec.c b/exec.c
>> index ee5bf7c..d817e5f 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -590,6 +590,9 @@ void cpu_exec_init(CPUArchState *env, Error **errp)
>> cpu_index = cpu->cpu_index = cpu_get_free_index(&local_err);
>> if (local_err) {
>> error_propagate(errp, local_err);
>> +#if defined(CONFIG_USER_ONLY)
>> + cpu_list_unlock();
>> +#endif
>> return;
>> }
>> QTAILQ_INSERT_TAIL(&cpus, cpu, node);
>>
>
> Sorry about this breakage, I should have been more careful.
>
> If this doesn't involve additional effort, feel free to drop these 3 patches,
> I will rework them in the next cycle.
>
If that is the course of action, we should keep the first patch to
avoid rebase-confilct of my target-foo change pattern.
Regards,
Peter
> Regards,
> Bharata.
>
>
- [Qemu-devel] [PULL v2 00/22] QOM CPUState patch queue 2015-07-07, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 02/22] cpu: Initialize breakpoint/watchpoint lists in cpu_common_initfn(), Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 03/22] cpu: Reorder cpu->as, cpu->thread_id, cpu->memory_dispatch init, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 08/22] gdbstub: Change gdbserver_fork() to accept cpu instead of env, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 09/22] cpu: Change tcg_cpu_exec() arg to cpu, not env, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 14/22] hw/arm/boot: Use cpu_set_pc(), Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 06/22] target-ppc: Move cpu_exec_init() call to realize function, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 05/22] cpu: Convert cpu_index into a bitmap, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 15/22] microblaze: boot: Use cpu_set_pc(), Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 13/22] gdbstub: Use cpu_set_pc() helper, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 21/22] disas: cris: Fix 0 buffer length case, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 20/22] disas: microblaze: QOMify target specific disas setup, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 22/22] disas: cris: QOMify target specific disas setup, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 18/22] disas: arm-a64: Make printfer and stream variable, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 07/22] translate-all: Change tb_flush() env argument to cpu, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 19/22] disas: arm: QOMify target specific disas setup, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 17/22] disas: QOMify target specific setup, Andreas Färber, 2015/07/07
- [Qemu-devel] [PULL v2 01/22] cpu: No need to zero-initialize CPUState::numa_node, Andreas Färber, 2015/07/07