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.