On Thu, Jul 21, 2016 at 05:54:34PM +0200, Igor Mammedov wrote: > it keeps the legacy behavior for all users that doesn't care > about stable cpu_index value, but would allow boards that > would support device_add/device_del to set stable cpu_index > that won't depend on order in which cpus are created/destroyed. > > While at that simplify cpu_get_free_index() as cpu_index > generated by USER_ONLY and softmmu variants is the same > since none of the users support cpu-remove so far except > of not yet released spapr/x86 device_add/del. which > will be altered in follow up patches to provide stable > cpu_index. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
It looks like the 1-line description needs a "not" in it somewhere,
but otherwise:
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> include/qom/cpu.h | 2 ++
> exec.c | 44 ++++++--------------------------------------
> qom/cpu.c | 2 +-
> 3 files changed, 9 insertions(+), 39 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index cbcd64c..ce0c406 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -883,4 +883,6 @@ extern const struct VMStateDescription vmstate_cpu_common;
> .offset = 0, \
> }
>
> +#define UNASSIGNED_CPU_INDEX -1
> +
> #endif
> diff --git a/exec.c b/exec.c
> index 8c5da32..8e8416b 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -598,30 +598,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int
> asidx)
> }
> #endif
>
> -#ifndef CONFIG_USER_ONLY
> -static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS);
> -
> -static int cpu_get_free_index(Error **errp)
> -{
> - int cpu = find_first_zero_bit(cpu_index_map, MAX_CPUMASK_BITS);
> -
> - if (cpu >= MAX_CPUMASK_BITS) {
> - error_setg(errp, "Trying to use more CPUs than max of %d",
> - MAX_CPUMASK_BITS);
> - return -1;
> - }
> -
> - bitmap_set(cpu_index_map, cpu, 1);
> - return cpu;
> -}
> -
> -static void cpu_release_index(CPUState *cpu)
> -{
> - bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
> -}
> -#else
> -
> -static int cpu_get_free_index(Error **errp)
> +static int cpu_get_free_index(void)
> {
> CPUState *some_cpu;
> int cpu_index = 0;
> @@ -632,12 +609,6 @@ static int cpu_get_free_index(Error **errp)
> return cpu_index;
> }
>
> -static void cpu_release_index(CPUState *cpu)
> -{
> - return;
> -}
> -#endif
> -
> void cpu_exec_exit(CPUState *cpu)
> {
> CPUClass *cc = CPU_GET_CLASS(cpu);
> @@ -650,8 +621,7 @@ void cpu_exec_exit(CPUState *cpu)
> }
>
> QTAILQ_REMOVE(&cpus, cpu, node);
> - cpu_release_index(cpu);
> - cpu->cpu_index = -1;
> + cpu->cpu_index = UNASSIGNED_CPU_INDEX;
> cpu_list_unlock();
>
> if (cc->vmsd != NULL) {
> @@ -665,7 +635,7 @@ void cpu_exec_exit(CPUState *cpu)
> void cpu_exec_init(CPUState *cpu, Error **errp)
> {
> CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
> - Error *local_err = NULL;
> + Error *local_err ATTRIBUTE_UNUSED = NULL;
>
> cpu->as = NULL;
> cpu->num_ases = 0;
> @@ -689,11 +659,9 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
> #endif
>
> cpu_list_lock();
> - cpu->cpu_index = cpu_get_free_index(&local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - cpu_list_unlock();
> - return;
> + if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
> + cpu->cpu_index = cpu_get_free_index();
> + assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
> }
> QTAILQ_INSERT_TAIL(&cpus, cpu, node);
> cpu_list_unlock();
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 42b5631..2553247 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -340,7 +340,7 @@ static void cpu_common_initfn(Object *obj)
> CPUState *cpu = CPU(obj);
> CPUClass *cc = CPU_GET_CLASS(obj);
>
> - cpu->cpu_index = -1;
> + cpu->cpu_index = UNASSIGNED_CPU_INDEX;
> cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
> qemu_mutex_init(&cpu->work_mutex);
> QTAILQ_INIT(&cpu->breakpoints);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
