On Thu, Jul 14, 2016 at 3:24 PM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 14 July 2016 at 08:57, David Gibson <da...@gibson.dropbear.id.au> 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 <da...@gibson.dropbear.id.au>
>> ---
>> 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.
This was the reason why the bitmap logic wasn't applied to
CONFIG_USER_ONLY when it was introduced.
https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01980.html
But then we didn't have actual removal, but we do now.
Regards,
Bharata.
