From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42691) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZD2L9-0004gm-Tu for qemu-devel@nongnu.org; Wed, 08 Jul 2015 23:17:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZD2Kz-0007Jl-F7 for qemu-devel@nongnu.org; Wed, 08 Jul 2015 23:17:35 -0400 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:54321) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZD2Kx-0007JK-Rw for qemu-devel@nongnu.org; Wed, 08 Jul 2015 23:17:25 -0400 Received: from /spool/local by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 9 Jul 2015 13:17:16 +1000 Received: from d23relay09.au.ibm.com (d23relay09.au.ibm.com [9.185.63.181]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 93AC7357805D for ; Thu, 9 Jul 2015 13:17:14 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay09.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t693H49Q52363466 for ; Thu, 9 Jul 2015 13:17:14 +1000 Received: from d23av03.au.ibm.com (localhost [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t693GeUO007345 for ; Thu, 9 Jul 2015 13:16:40 +1000 Date: Thu, 9 Jul 2015 08:46:20 +0530 From: Bharata B Rao Message-ID: <20150709031619.GA3352@in.ibm.com> References: <1436289389-5403-1-git-send-email-afaerber@suse.de> <1436289389-5403-6-git-send-email-afaerber@suse.de> <559D74F2.5000206@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <559D74F2.5000206@suse.de> Subject: Re: [Qemu-devel] [PULL v2 05/22] cpu: Convert cpu_index into a bitmap Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Paolo Bonzini , Alexander Graf , qemu-devel@nongnu.org, Peter Crosthwaite On Wed, Jul 08, 2015 at 09:07:30PM +0200, Andreas Färber wrote: > Am 07.07.2015 um 19:16 schrieb Andreas Färber: > > From: Bharata B Rao > > > > Currently CPUState::cpu_index is monotonically increasing and a newly > > created CPU always gets the next higher index. The next available > > index is calculated by counting the existing number of CPUs. This is > > fine as long as we only add CPUs, but there are architectures which > > are starting to support CPU removal, too. For an architecture like PowerPC > > which derives its CPU identifier (device tree ID) from cpu_index, the > > existing logic of generating cpu_index values causes problems. > > > > With the currently proposed method of handling vCPU removal by parking > > the vCPU fd in QEMU > > (Ref: http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02604.html), > > generating cpu_index this way will not work for PowerPC. > > > > This patch changes the way cpu_index is handed out by maintaining > > a bit map of the CPUs that tracks both addition and removal of CPUs. > > > > The CPU bitmap allocation logic is part of cpu_exec_init(), which is > > called by instance_init routines of various CPU targets. Newly added > > cpu_exec_exit() API handles the deallocation part and this routine is > > called from generic CPU instance_finalize. > > > > Note: This new CPU enumeration is for !CONFIG_USER_ONLY only. > > CONFIG_USER_ONLY continues to have the old enumeration logic. > > > > Signed-off-by: Bharata B Rao > > Reviewed-by: Eduardo Habkost > > Reviewed-by: Igor Mammedov > > Reviewed-by: David Gibson > > Reviewed-by: Peter Crosthwaite > > Acked-by: Paolo Bonzini > > Signed-off-by: Peter Crosthwaite > > [AF: max_cpus -> MAX_CPUMASK_BITS] > > Signed-off-by: Andreas Färber > > --- > > exec.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++----- > > include/qom/cpu.h | 1 + > > qom/cpu.c | 7 +++++++ > > 3 files changed, 58 insertions(+), 5 deletions(-) > > > > diff --git a/exec.c b/exec.c > > index 8abac69..02602b6 100644 > > --- a/exec.c > > +++ b/exec.c > [...] > > @@ -542,11 +587,11 @@ void cpu_exec_init(CPUArchState *env, Error **errp) > > #if defined(CONFIG_USER_ONLY) > > cpu_list_lock(); > > #endif > > - cpu_index = 0; > > - CPU_FOREACH(some_cpu) { > > - cpu_index++; > > + cpu_index = cpu->cpu_index = cpu_get_free_index(&local_err); > > + if (local_err) { > > + error_propagate(errp, local_err); > > This is lacking a matching cpu_list_unlock() in the CONFIG_USER_ONLY case: > > diff --git a/exec.c b/exec.c > index ee5bf7c..d817e5f 100644 > --- a/exec.c > +++ b/exec.c > @@ -590,6 +590,9 @@ void cpu_exec_init(CPUArchState *env, Error **errp) > cpu_index = cpu->cpu_index = cpu_get_free_index(&local_err); > if (local_err) { > error_propagate(errp, local_err); > +#if defined(CONFIG_USER_ONLY) > + cpu_list_unlock(); > +#endif > return; > } > QTAILQ_INSERT_TAIL(&cpus, cpu, node); > Sorry about this breakage, I should have been more careful. If this doesn't involve additional effort, feel free to drop these 3 patches, I will rework them in the next cycle. Regards, Bharata.