From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41051) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aJEQ9-0001lb-7G for qemu-devel@nongnu.org; Wed, 13 Jan 2016 00:56:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aJEQ6-0001dV-0D for qemu-devel@nongnu.org; Wed, 13 Jan 2016 00:56:37 -0500 Date: Wed, 13 Jan 2016 15:57:00 +1100 From: David Gibson Message-ID: <20160113045700.GA22925@voom.redhat.com> References: <1452236119-24452-1-git-send-email-bharata@linux.vnet.ibm.com> <1452236119-24452-12-git-send-email-bharata@linux.vnet.ibm.com> <20160112060634.GW22925@voom.redhat.com> <20160113041054.GE11785@in.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="T0u6oYl84yPn00Od" Content-Disposition: inline In-Reply-To: <20160113041054.GE11785@in.ibm.com> Subject: Re: [Qemu-devel] [PATCH v6 11/11] spapr: CPU hot unplug support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao 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, ehabkost@redhat.com --T0u6oYl84yPn00Od Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jan 13, 2016 at 09:40:54AM +0530, Bharata B Rao wrote: > On Tue, Jan 12, 2016 at 05:06:34PM +1100, David Gibson wrote: > > On Fri, Jan 08, 2016 at 12:25:19PM +0530, Bharata B Rao wrote: > > > Remove the CPU core device by removing the underlying CPU thread devi= ces. > > > Support hot removal of CPU for sPAPR guests by sending the hot unplug > > > notification to the guest via EPOW interrupt. Release the vCPU object > > > after CPU hot unplug so that vCPU fd can be parked and reused. > > >=20 > > > Signed-off-by: Bharata B Rao > > > --- > > > hw/ppc/spapr.c | 113 +++++++++++++++++++++++++++++++++++++++= ++++++++++ > > > include/hw/ppc/spapr.h | 6 +++ > > > 2 files changed, 119 insertions(+) > > >=20 > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index c2af9ca..43cb726 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -93,6 +93,9 @@ > > > =20 > > > #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) > > > =20 > > > +static QLIST_HEAD(cpu_unplug_list, sPAPRCPUUnplug) cpu_unplug_list > > > + =3D QLIST_HEAD_INITIALIZER(cpu_unplug_list); > > > + > > > static XICSState *try_create_xics(const char *type, int nr_servers, > > > int nr_irqs, Error **errp) > > > { > > > @@ -2432,11 +2435,121 @@ static void spapr_machine_device_plug(Hotplu= gHandler *hotplug_dev, > > > } > > > } > > > =20 > > > +static void spapr_cpu_destroy(PowerPCCPU *cpu) > > > +{ > > > + sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine()); > > > + > > > + xics_cpu_destroy(spapr->icp, cpu); > > > + qemu_unregister_reset(spapr_cpu_reset, cpu); > > > +} > > > + > > > +static void spapr_cpu_core_cleanup(void) > > > +{ > > > + sPAPRCPUUnplug *unplug, *next; > > > + Object *cpu; > > > + > > > + QLIST_FOREACH_SAFE(unplug, &cpu_unplug_list, node, next) { > > > + cpu =3D unplug->cpu; > > > + object_unparent(cpu); > > > + QLIST_REMOVE(unplug, node); > > > + g_free(unplug); > > > + } > > > +} > > > + > > > +static void spapr_add_cpu_to_unplug_list(Object *cpu) > > > +{ > > > + sPAPRCPUUnplug *unplug =3D g_malloc(sizeof(*unplug)); > > > + > > > + unplug->cpu =3D cpu; > > > + QLIST_INSERT_HEAD(&cpu_unplug_list, unplug, node); > > > +} > > > + > > > +static int spapr_cpu_release(Object *obj, void *opaque) > > > +{ > > > + DeviceState *dev =3D DEVICE(obj); > > > + CPUState *cs =3D CPU(dev); > > > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > > > + > > > + spapr_cpu_destroy(cpu); > > > + cpu_remove_sync(cs); > > > + > > > + /* > > > + * We are still walking the core object's children list, and > > > + * hence can't cleanup this CPU thread object just yet. Put > > > + * it on a list for later removal. > > > + */ > > > + spapr_add_cpu_to_unplug_list(obj); > > > + return 0; > > > +} > > > + > > > +static void spapr_core_release(DeviceState *dev, void *opaque) > > > +{ > > > + object_child_foreach(OBJECT(dev), spapr_cpu_release, opaque); > > > + spapr_cpu_core_cleanup(); > > > + object_unparent(OBJECT(dev)); > >=20 > > I'd prefer to see the unplug_list as a local to this function (passed > > to spapr_cpu_release via opaque) rather than using a new global. >=20 > Will try that in the next iteration. >=20 > >=20 > > > +} > > > + > > > +static int spapr_core_detach(Object *obj, void *opaque) > > > +{ > > > + sPAPRCoreState *core =3D opaque; > > > + DeviceState *dev =3D DEVICE(obj); > > > + CPUState *cs =3D CPU(dev); > > > + PowerPCCPU *cpu =3D POWERPC_CPU(cs); > > > + int id =3D ppc_get_vcpu_dt_id(cpu); > > > + int smt =3D kvmppc_smt_threads(); > > > + sPAPRDRConnector *drc =3D > > > + spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id); > > > + sPAPRDRConnectorClass *drck; > > > + Error *local_err =3D NULL; > > > + > > > + /* > > > + * SMT threads return from here, only main thread (thread 0) will > > > + * continue and signal hot unplug event to the guest. > > > + */ > >=20 > > This seems silly. spapr_core_unplug() iterates through all the > > children only to have all of them except thread 0 ignore the call.. > >=20 > > Can't you just grab the first thread and do this logic directly from > > spapr_core_unplug()? >=20 > If I purely rely on the children list of the core object like I am doing > here, I don't see how to grab the first thread object from the list w/o > doing what I am doing here. However I can store the first thread object > in the core object during the core object's instance_init and use it here. > Will give it a try in the next iteration. It should be accessible by name as "thread[0]" no? --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --T0u6oYl84yPn00Od Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWldkcAAoJEGw4ysog2bOSvCAP/2GGEGJKgxXmmiob0g80USol xSnJe0JlSgdxGwAO/WelktOvJzgC1lsIUPQAcVfrw7BF+jV7N3rTgECayvOCaKOd fryPjQVaXmLENc8wEGflA/5QzpP96I/gaBiZaVBvtq03MW8RYjusE1/Bd2LkAvfV 3xIx6d4+vkLHoEDGFp5LnMn93DIr2QNjhHbNhY7G2ngkbIZ9ZiUK3w5K+mMal9zb dOQBIbQHtfZDNCsdzuSdjz6MINkBXoQtKfBElJ2apJNZheBUCurcH9feVARDjr/g LUTLFHc6JIw2dBSPlgYIrXcswxKH9wy9HlcJJPAIvR4kJ3tKECz496EK2yWtivni fNnPg64fUMC9NER+0WB8Gq6CMnuPvC0+grf3RatJ3L+Q8otMZ7y+4opYUbdwTt+H vIIYxqY1kq1+WKhuJsvMllR10Vww0CjrSw1ZP0NBxxDtqtIS4CFjXx46un267ujE Lh1TA52s12hJkcjJbNY2/l6boC+JELcw6bFuTSN5OL8dsxFPeRL7gqlfXbkwTIDJ rQcxMTJSrURJHX5eTuazmcAeyO7URjmjkArIdoaFda6TYIcx7oGcIzunRp2WEwSD ajT0ZyixlM846Qpf6DVtQqn3C6snySHF1Vc+R9JDhzHwIYcDQC9vyFAZE0ahJYib CxQLUwD1jqLfDYnCMKAj =rmOO -----END PGP SIGNATURE----- --T0u6oYl84yPn00Od--