From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46269) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcbSm-0005Ze-HZ for qemu-devel@nongnu.org; Thu, 17 Sep 2015 11:51:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZcbSj-0004VA-AS for qemu-devel@nongnu.org; Thu, 17 Sep 2015 11:51:08 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:42786) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcbSj-0004UN-3m for qemu-devel@nongnu.org; Thu, 17 Sep 2015 11:51:05 -0400 Received: from /spool/local by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 17 Sep 2015 09:51:03 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <55F6D897.1090100@redhat.com> References: <1442194913-26545-1-git-send-email-david@gibson.dropbear.id.au> <1442194913-26545-2-git-send-email-david@gibson.dropbear.id.au> <55F683CB.6000508@ozlabs.ru> <20150914114503.GP2547@voom.fritz.box> <55F6B989.8010900@redhat.com> <55F6D479.4010301@ozlabs.ru> <55F6D897.1090100@redhat.com> Message-ID: <20150917150117.27212.45630@loki> Date: Thu, 17 Sep 2015 10:01:17 -0500 Subject: Re: [Qemu-devel] [RFCv2 1/2] spapr: Remove unnecessary owner field from sPAPRDRConnector List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Alexey Kardashevskiy , David Gibson Cc: bharata@linux.vnet.ibm.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de Quoting Paolo Bonzini (2015-09-14 09:24:23) > = > = > On 14/09/2015 16:06, Alexey Kardashevskiy wrote: > >>>>> > >>>>> =3D=3D=3D * There is no way for a child to determine what its parent > >>>>> is. It is not * a bidirectional relationship. This is by > >>>>> design. =3D=3D=3D > >>>>> > >>>>> This part always confused me as there is "Object *parent" in > >>>>> the "struct Object". So there is way to determine but it must > >>>>> not be used? Is it debug only? > >>>>> > >>>>> Anyway, all members of the Object class are under /*< private > >>>>>> */ so they should not be accesses in sPAPR code, I believe. > >>> Ah, good point, I missed that. I guess we have to keep the owner > >>> field, redundant though it seems. Blech. > >> > >> I think the comment is wrong or at least inaccurate; it only applies > >> to the external QOM interface. > > = > > Is this case external? > = > I meant external as in qom-get, qom-set, qom-list. There isn't a ".." > property. > = > > Originally I was looking for a object_get_parent() but it is not there > > so I decided that the comment is correct or I just fail to understand i= t :) > = > Yes, we can add such an API. > = > Let's look also at what ->owner is used for. > = > > object_property_add_alias(root_container, link_name, > > drc->owner, child_name, &err); > = > This can be rewritten as > = > object_property_add_const_link(root_container, link_name, > drc, &err); > = > > QTAILQ_FOREACH(prop, &root_container->properties, node) { > > Object *obj; > > sPAPRDRConnector *drc; > > sPAPRDRConnectorClass *drck; > > uint32_t drc_index, drc_power_domain; > > = > > if (!strstart(prop->type, "link<", NULL)) { > > continue; > > } > > = > > obj =3D object_property_get_link(root_container, prop->name, NU= LL); > > drc =3D SPAPR_DR_CONNECTOR(obj); > > drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > > = > > if (owner && (drc->owner !=3D owner)) { > = > Could the PCI host bridge instead store the DR connectors when it > creates them with spapr_dr_connector_new? Then you can just call > spapr_drc_populate_dt directly with the right objects, and avoid another > O(n^2) loop. It could be done I think, but in some cases the DRC lists can include a mix of DRC types/indexes from multiple parents, so we'd need to add some logic to modify/append to existing paths in FDT. We end up needing a global registry of some sort anyway, since RTAS calls from the guest address the DRCs purely by the DRC index, so I think if we can make use of that same registry to simply the above case there's no reason not to. > = > Paolo >=20