Re: [Qemu-devel] [RFC 2/2] linux-user: Fix cpu_index generation

[Peter Maydell]

On 14 July 2016 at 08:57, David Gibson <address@hidden> wrote:
> With CONFIG_USER_ONLY, generation of cpu_index values is done differently
> than for full system targets.  This method turns out to be broken, since
> it can fairly easily result in duplicate cpu_index values for
> simultaneously active cpus (i.e. threads in the emulated process).
>
> Consider this sequence:
>     Create thread 1
>     Create thread 2
>     Exit thread 1
>     Create thread 3
>
> With the current logic thread 1 will get cpu_index 1, thread 2 will get
> cpu_index 2 and thread 3 will also get cpu_index 2 (because there are 2
> threads in the cpus list at the point of its creation).
>
> We mostly get away with this because cpu_index values aren't that important
> for userspace emulation.  Still, it can't be good, so this patch fixes it
> by making CONFIG_USER_ONLY use the same bitmap based allocation that full
> system targets already use.
>
> Signed-off-by: David Gibson <address@hidden>
> ---
>  exec.c | 19 -------------------
>  1 file changed, 19 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 011babd..e410dab 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -596,7 +596,6 @@ 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)
> @@ -617,24 +616,6 @@ 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)
> -{
> -    CPUState *some_cpu;
> -    int cpu_index = 0;
> -
> -    CPU_FOREACH(some_cpu) {
> -        cpu_index++;
> -    }
> -    return cpu_index;
> -}
> -
> -static void cpu_release_index(CPUState *cpu)
> -{
> -    return;
> -}
> -#endif

Won't this change impose a maximum limit of 256 simultaneous
threads? That seems a little low for comfort.

thanks
-- PMM