From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bRrLJ-000895-1w for qemu-devel@nongnu.org; Mon, 25 Jul 2016 21:39:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bRrLH-0005Sn-FN for qemu-devel@nongnu.org; Mon, 25 Jul 2016 21:39:32 -0400 Date: Tue, 26 Jul 2016 11:13:52 +1000 From: David Gibson Message-ID: <20160726011352.GG17429@voom.fritz.box> References: <1468833560-17397-1-git-send-email-david@gibson.dropbear.id.au> <1468833560-17397-2-git-send-email-david@gibson.dropbear.id.au> <20160719115240.o5aivhfq2yktlvf2@andariel.pipo.sk> <20160720010005.GB27358@voom.fritz.box> <20160720070111.hf6xdszfzswpeocc@andariel.pipo.sk> <20160725060904.GK24621@voom.fritz.box> <20160725101418.268e37ea@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zGQnqpIoxlsbsOfg" Content-Disposition: inline In-Reply-To: <20160725101418.268e37ea@nial.brq.redhat.com> Subject: Re: [Qemu-devel] [RFC 1/2] spapr: Reverse order of hotpluggable cpus list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Peter Krempa , bharata@linux.vnet.ibm.com, groug@kaod.org, qemu-ppc@nongnu.org, abologna@redhat.com, qemu-devel@nongnu.org --zGQnqpIoxlsbsOfg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 25, 2016 at 10:14:18AM +0200, Igor Mammedov wrote: > On Mon, 25 Jul 2016 16:09:04 +1000 > David Gibson wrote: >=20 > > On Wed, Jul 20, 2016 at 09:01:11AM +0200, Peter Krempa wrote: > > > On Wed, Jul 20, 2016 at 11:00:05 +1000, David Gibson wrote: =20 > > > > On Tue, Jul 19, 2016 at 01:52:40PM +0200, Peter Krempa wrote: =20 > > > > > On Mon, Jul 18, 2016 at 19:19:19 +1000, David Gibson wrote: =20 > > > > > > The spapr implementation of query-hotpluggable-cpus builds the = list of > > > > > > hotpluggable cores from the end (most removed from the list hea= d) > > > > > > because that's the easiest way with a singly linked list. Beca= use it > > > > > > also traverses the possible CPU cores starting from low indexes= the > > > > > > resulting list has the cpu cores listed in reverse order by cor= e-id. > > > > > >=20 > > > > > > That's not generally harmful, but it means the output from "info > > > > > > hotpluggable-cpus" is a little harder to read than it could be. > > > > > >=20 > > > > > > Therefore, traverse the cpus in reverse order so that the final= list > > > > > > ends up in increasing-core-id order. =20 > > > > >=20 > > > > > To make this interface usable with in-order hotplug the ordering = of the > > > > > entries should be codified in the schema documentation. (see my r= esponse > > > > > on the cover letter for justification). =20 > > > >=20 > > > > I'm not really sure what you mean by this. =20 > > >=20 > > > Currently the order of entries in reply of query-hotpluggable-cpus is > > > arbitrary as this patch hints. > > >=20 > > > If qemu won't support arbitrary order hotplug but libvirt should be a= ble > > > to use the new interface, then the order of replies of > > > query-hotpluggable-cpus need to corelate (in a documented fashion) wi= th > > > the order we need to plug the cpus in. By not documenting any order > > > libvirt can just guess it (by reimplementing the algorithm in qemu). > > >=20 > > > I've pointed this out in a sub-thread of the cover-letter. =20 > >=20 > > So, based on Peter's comments (and others) I've concluded that the > > cpu-add implementation in terms of the new interface isn't really > > useful. > >=20 > > However, I think this re-ordering is still a good idea, because: > > * It makes info hotpluggable-cpus easier to read in the HMP > >=20 > > * Although we certainly need a better approach to handling hotplug + > > NUMA, correcting the order should allow the existing NUMA > > interface to work with only as much guesswork in libvirt as we > > already have. > >=20 > > * We haven't released a qemu with query-hotpluggable-cpus yet, so we > > shouldn't need version conditions in order to use the order. > I've talked with Peter and meanwhile (till we have sane NUMA interface) > libvirt will be guessing ids based on query-hotpluggable-cpus output > sorted by socket/core/thread-id, so forward/reverse order won't really > matter to it. >=20 > Forward sorting is fine wrt 'info hotpluggable-cpus', however it might > be better to do sorting in HMP callback so that each target won't have > to do it nor regress command output in future or introduce another > ordering. >=20 > Considering that ordering affects only HMP won't require any compat > glue even if it's not fixed in 2.7 release. Hm, ok, I'll drop this patch. >=20 > >=20 > > For that reason I've tentatively merged this patch into ppc-for-2.7. > > It would be good to get an R-b or A-b from someone before I send a > > pull request (which I'm hoping to do tomorrow). > >=20 >=20 --=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 --zGQnqpIoxlsbsOfg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXlrlQAAoJEGw4ysog2bOSDTUP/3gqbJH2wuDq3isFQn83abbu Nh70qSPbUTjhslX4sgkAIiAMREUIBpRS3BnrAe5DiCfE2rmq6MFVPRxNV32vCg1i H5oR3RucHG/mO6zEBb1HkhCABUQFG9uQxEFvXPrKppo07QmeVCEHcxHOR9uiui7J BP2A6I/MHJv6LKpCNACxeWNcHbQIRuiaMVx06MpyfzLIhVrsfg3JBpn4uCj/co2f lCK5IASMh0+H9hwaYJ1yQCYeL9sy19hpzI4PIhXPFaNglFVEzjdNyPbwgRUYxazJ LjiaaIlzX0KGk22X866q7yLmfhUt6GPfF+nUPUhAlChJDZ5pTJgkp4mGqD6QBaIr 9ekH4buplKPmVMYDTiNPTS1RkjURzCC6YcUpOuOGE1NF2hdskVQBHbiFDiGJ5pna QJTLsjjjye2nziRd+Gm1if1fUtk6p52Ydbp7a68rKYZ+s+SmSX8f5+Ro0sEEc3SM 9JGyys5sNgCYOEFC8SnqhqbMayXjz+W+5xqep+tsIaxXh2zHst2iMLq4LFbVMTax IayeBi1VhpjXoF8KL1TyFEAcSOMVJ0TNth7btpDG6Mo8ER3/o2aT0/XJuu36VvXN 8+t4884KyoXwfhZRms+kxgJAhHwif/wvKUnzKBsYplk74WLUSXpC11ncIM1o6r/D G2irKI2GpjQE3mAldH4/ =8GRv -----END PGP SIGNATURE----- --zGQnqpIoxlsbsOfg--