From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58396) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNGLP-0002T4-49 for qemu-devel@nongnu.org; Wed, 13 Jul 2016 05:20:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bNGLJ-0005Ri-IU for qemu-devel@nongnu.org; Wed, 13 Jul 2016 05:20:37 -0400 Received: from 4.mo178.mail-out.ovh.net ([46.105.49.171]:53138) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bNGLJ-0005RT-8E for qemu-devel@nongnu.org; Wed, 13 Jul 2016 05:20:33 -0400 Received: from player169.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo178.mail-out.ovh.net (Postfix) with ESMTP id 234B410087CD for ; Wed, 13 Jul 2016 11:20:31 +0200 (CEST) Date: Wed, 13 Jul 2016 11:20:23 +0200 From: Greg Kurz Message-ID: <20160713112023.2192fe5a@bahia.lan> In-Reply-To: <20160713094254.04658871@nial.brq.redhat.com> References: <1468392620-25599-1-git-send-email-bharata@linux.vnet.ibm.com> <20160713094254.04658871@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/1] spapr: Ensure CPU cores are added contiguously and removed in LIFO order List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Bharata B Rao , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, david@gibson.dropbear.id.au On Wed, 13 Jul 2016 09:42:54 +0200 Igor Mammedov wrote: > On Wed, 13 Jul 2016 12:20:20 +0530 > Bharata B Rao wrote: > > > If CPU core addition or removal is allowed in random order leading to > > holes in the core id range (and hence in the cpu_index range), migration > > can fail as migration with holes in cpu_index range isn't yet handled > > correctly. > > > > Prevent this situation by enforcing the addition in contiguous order > > and removal in LIFO order so that we never end up with holes in > > cpu_index range. > Adding this limitation looks better than adding migration_id as > it will allow libvirt to use the current -numa cpus=... while > doing the same amount of guess work as it does now. > > Similar patch for x86 won't be so simple as cpu-add can add cpus > with gaps (breaking migration at that), so I'd need to keep it > that way with some compat code, but that shouldn't be issue. > And does libvirt take care not to add cpus with gaps ? If so, maybe it can do the same with PPC, and we'd simply warn the user that migration is broken if we introduce gaps. Makes sense ? > > > Signed-off-by: Bharata B Rao > > --- > > While there is work in progress to support migration when there are holes > > in cpu_index range resulting from out-of-order plug or unplug, this patch > > is intended as a last resort if no easy, risk-free and elegant solution > > emerges before 2.7 dev cycle ends. > > > > hw/ppc/spapr_cpu_core.c | 21 ++++++++++++++++++++- > > 1 file changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index bc52b3c..4bfc96b 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -126,12 +126,23 @@ static void spapr_core_release(DeviceState *dev, void *opaque) > > void spapr_core_unplug(HotplugHandler *hotplug_dev, DeviceState *dev, > > Error **errp) > > { > > + sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev)); > > CPUCore *cc = CPU_CORE(dev); > > sPAPRDRConnector *drc = > > spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, cc->core_id); > > sPAPRDRConnectorClass *drck; > > Error *local_err = NULL; > > + int smt = kvmppc_smt_threads(); > > + int index = cc->core_id / smt; > > + int spapr_max_cores = max_cpus / smp_threads; > > + int i; > > > > + for (i = spapr_max_cores - 1; i > index; i--) { > > + if (spapr->cores[i]) { > > + error_setg(errp, "core-id %d should be removed first", i * smt); > > + return; > > + } > > + } > > g_assert(drc); > > > > drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); > > @@ -214,7 +225,7 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(OBJECT(hotplug_dev)); > > sPAPRMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev)); > > int spapr_max_cores = max_cpus / smp_threads; > > - int index; > > + int index, i; > > int smt = kvmppc_smt_threads(); > > Error *local_err = NULL; > > CPUCore *cc = CPU_CORE(dev); > > @@ -252,6 +263,14 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev, > > goto out; > > } > > > > + for (i = 0; i < index; i++) { > > + if (!spapr->cores[i]) { > > + error_setg(&local_err, "core-id %d should be added first", > > + i * smt); > > + goto out; > > + } > > + } > > + > > out: > > g_free(base_core_type); > > error_propagate(errp, local_err); > >