From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35703) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3yNu-0000Hv-HD for qemu-devel@nongnu.org; Tue, 01 Dec 2015 22:47:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a3yNr-0008Uy-Ac for qemu-devel@nongnu.org; Tue, 01 Dec 2015 22:47:14 -0500 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:50414) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a3yNq-0008Ub-K1 for qemu-devel@nongnu.org; Tue, 01 Dec 2015 22:47:11 -0500 Received: from localhost by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 2 Dec 2015 09:17:07 +0530 Date: Wed, 2 Dec 2015 09:17:00 +0530 From: Bharata B Rao Message-ID: <20151202034700.GC16342@in.ibm.com> References: <1448024079-20808-1-git-send-email-bharata@linux.vnet.ibm.com> <1448024079-20808-3-git-send-email-bharata@linux.vnet.ibm.com> <20151201004403.GA31343@voom.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151201004403.GA31343@voom.redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 02/10] exec: Remove cpu from cpus list during cpu_exec_exit() Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: mdroth@linux.vnet.ibm.com, aik@ozlabs.ru, agraf@suse.de, qemu-devel@nongnu.org, pbonzini@redhat.com, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, imammedo@redhat.com, afaerber@suse.de 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.