On Wed, Jul 8, 2015 at 8:16 PM, Bharata B Rao <bhar...@linux.vnet.ibm.com> wrote: > 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 <bhar...@linux.vnet.ibm.com> >> > >> > 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 <bhar...@linux.vnet.ibm.com> >> > Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> >> > Reviewed-by: Igor Mammedov <imamm...@redhat.com> >> > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> >> > Reviewed-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com> >> > Acked-by: Paolo Bonzini <pbonz...@redhat.com> >> > Signed-off-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com> >> > [AF: max_cpus -> MAX_CPUMASK_BITS] >> > Signed-off-by: Andreas Färber <afaer...@suse.de> >> > --- >> > 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. >
If that is the course of action, we should keep the first patch to avoid rebase-confilct of my target-foo change pattern. Regards, Peter > Regards, > Bharata. > >
