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

[Bharata B Rao]

On Tue, Dec 01, 2015 at 11:44:03AM +1100, David Gibson wrote:
> On Fri, Nov 20, 2015 at 06:24:31PM +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.
> 
> It's not clear to me if you're intending this just as an interim step
> or not.  Surely we should fix the existing code to be consistent about
> where the QTAILQ_REMOVE is done, rather than using a special -1 flag?

cpu_index for a CPU starts with -1 and comes back to -1 when the CPU is
destroyed. So -1 value is already being used to prevent double freeing of
this particular CPU bit from the cpu_index_map. In this patch, I am depending
on the same (-1 value) to guard against double removal of the CPU from the
list. Setting cpu_index to -1 wasn't being done for CONFIG_USER_ONLY and
this patch adds that bit too.

This check against double freeing from bitmap and double removal from
list is needed since we call cpu_exec_init() from CPU::instance_finalize
for all archs and archs like PowerPC call it from unrealizefn. If and
when all archs switch to calling cpu_exec_init()/_exit() from
realizefn/unrealizefn, we wouldn't have to worry about this double freeing.

Regards,
Bharata.