Re: [PATCH v1 5/8] cpus-common: ensure auto-assigned cpu_indexes don't clash

["Alex Bennée" <alex.bennee@linaro.org>]

a
Alex Bennée <alex.bennee@linaro.org> writes:

> Basing the cpu_index on the number of currently allocated vCPUs fails
> when vCPUs aren't removed in a LIFO manner. This is especially true
> when we are allocating a cpu_index for each guest thread in
> linux-user where there is no ordering constraint on their allocation
> and de-allocation.
>
> [I've dropped the assert which is there to guard against out-of-order
> removal as this should probably be caught higher up the stack. Maybe
> we could just ifdef CONFIG_SOFTTMU it?]
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Nikolay Igotti <igotti@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  cpus-common.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/cpus-common.c b/cpus-common.c
> index 55d5df89237..5a7d2f6132b 100644
> --- a/cpus-common.c
> +++ b/cpus-common.c
> @@ -61,13 +61,14 @@ static bool cpu_index_auto_assigned;
>  static int cpu_get_free_index(void)
>  {
>      CPUState *some_cpu;
> -    int cpu_index = 0;
> +    int max_cpu_index = 0;
>  
>      cpu_index_auto_assigned = true;
>      CPU_FOREACH(some_cpu) {
> -        cpu_index++;
> +        max_cpu_index = MAX(some_cpu->cpu_index, max_cpu_index);
>      }
> -    return cpu_index;
> +    max_cpu_index++;
> +    return max_cpu_index;
>  }

OK some ending up with cpu_index = 1 threw off devices that would do
qemu_get_cpu(0) so I've tweaked the algorithm to:

  static int cpu_get_free_index(void)
  {
      CPUState *some_cpu;
      int max_cpu_index = 0;

      cpu_index_auto_assigned = true;
      CPU_FOREACH(some_cpu) {
          if (some_cpu->cpu_index >= max_cpu_index) {
              max_cpu_index = some_cpu->cpu_index + 1;
          }
      }
      return max_cpu_index;
  }

>  
>  void cpu_list_add(CPUState *cpu)
> @@ -90,8 +91,6 @@ void cpu_list_remove(CPUState *cpu)
>          return;
>      }
>  
> -    assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus)));
> -
>      QTAILQ_REMOVE_RCU(&cpus, cpu, node);
>      cpu->cpu_index = UNASSIGNED_CPU_INDEX;
>  }


-- 
Alex Bennée