Re: [Qemu-devel] [PATCH v7 02/13] exec: Remove cpu from cpus list during cpu_exec_exit()

[Bharata B Rao <bharata@linux.vnet.ibm.com>]

On Thu, Jan 28, 2016 at 05:19:33PM -0200, Eduardo Habkost wrote:
> On Thu, Jan 28, 2016 at 11:19:44AM +0530, Bharata B Rao wrote:
> > CPUState *cpu gets added to the cpus list during cpu_exec_init(). It
> > should be removed from cpu_exec_exit().
> > 
> > cpu_exec_init() is called from generic CPU::instance_finalize and some
> > archs like PowerPC call it from CPU unrealizefn. So ensure that we
> > dequeue the cpu only once.
> > 
> > Now -1 value for cpu->cpu_index indicates that we have already dequeued
> > the cpu for CONFIG_USER_ONLY case also.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  exec.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/exec.c b/exec.c
> > index 7115403..c8da9d4 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -596,6 +596,7 @@ void cpu_exec_exit(CPUState *cpu)
> >          return;
> >      }
> >  
> > +    QTAILQ_REMOVE(&cpus, cpu, node);
> >      bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
> >      cpu->cpu_index = -1;
> >  }
> > @@ -614,6 +615,15 @@ static int cpu_get_free_index(Error **errp)
> >  
> >  void cpu_exec_exit(CPUState *cpu)
> >  {
> > +    cpu_list_lock();
> > +    if (cpu->cpu_index == -1) {
> > +        cpu_list_unlock();
> > +        return;
> > +    }
> > +
> > +    QTAILQ_REMOVE(&cpus, cpu, node);
> > +    cpu->cpu_index = -1;
> > +    cpu_list_unlock();
> 
> With this, the only differences between the two cpu_exec_exit()
> implementations are:
> 
> * cpu_list_lock()/cpu_list_unlock() functions.
>   * We can add !CONFIG_USER_ONLY stubs for them.
> * The bitmap_clear() call.
>   * It can be abstracted away in a cpu_release_index() function,
>     just like we already have a CONFIG_USER_ONLY version of
>     cpu_get_free_index().

Ok, made those changes so that cpu_exec_exit() will be a common routine
with some CONFIG_USER_ONLY ifdefs in between.

Regards,
Bharata.