From owner-svn-src-head@FreeBSD.ORG Sat Sep 6 19:40:09 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 94612CA8; Sat, 6 Sep 2014 19:40:09 +0000 (UTC) Received: from mho-01-ewr.mailhop.org (mho-03-ewr.mailhop.org [204.13.248.66]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 54B991CA6; Sat, 6 Sep 2014 19:40:09 +0000 (UTC) Received: from [73.34.117.227] (helo=ilsoft.org) by mho-01-ewr.mailhop.org with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.72) (envelope-from ) id 1XQLqB-0005BA-T0; Sat, 06 Sep 2014 19:40:08 +0000 Received: from [172.22.42.240] (revolution.hippie.lan [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id s86Je6nK015936; Sat, 6 Sep 2014 13:40:06 -0600 (MDT) (envelope-from ian@FreeBSD.org) X-Mail-Handler: Dyn Standard SMTP by Dyn X-Originating-IP: 73.34.117.227 X-Report-Abuse-To: abuse@dyndns.com (see http://www.dyndns.com/services/sendlabs/outbound_abuse.html for abuse reporting information) X-MHO-User: U2FsdGVkX1/zprqVhuVIyJNbWbgKHqj7 X-Authentication-Warning: paranoia.hippie.lan: Host revolution.hippie.lan [172.22.42.240] claimed to be [172.22.42.240] Subject: Re: svn commit: r271202 - head/sys/dev/ofw From: Ian Lepore To: Nathan Whitehorn In-Reply-To: <540B5DA8.3080601@freebsd.org> References: <201409061843.s86IhHMJ054856@svn.freebsd.org> <540B5DA8.3080601@freebsd.org> Content-Type: text/plain; charset="us-ascii" Date: Sat, 06 Sep 2014 13:40:06 -0600 Message-ID: <1410032406.1150.370.camel@revolution.hippie.lan> Mime-Version: 1.0 X-Mailer: Evolution 2.32.1 FreeBSD GNOME Team Port Content-Transfer-Encoding: 7bit Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Sep 2014 19:40:09 -0000 On Sat, 2014-09-06 at 12:16 -0700, Nathan Whitehorn wrote: > Not looking at the code: what happens if you ask for the node > corresponding to a phandle but the device corresponding to that phandle > has not registered yet and it has an implicit crossreference mapping? > -Nathan > The node<->xref behavior is the same as it has always been: if there isn't a phandle property in a node to supply an xref handle then the node and xref handles are the synonyms. The only thing that has changed is that with my first implementation an xref handle had exist (meaning it had to have been discovered as a phandle property during the init-time scan of the tree) before you could associate a device_t with it. Now you can make associations on the fly whether the data author intended for such associations to exist or not. I can't decide whether that's a good or a bad thing. -- Ian > On 09/06/14 11:43, Ian Lepore wrote: > > Author: ian > > Date: Sat Sep 6 18:43:17 2014 > > New Revision: 271202 > > URL: http://svnweb.freebsd.org/changeset/base/271202 > > > > Log: > > When registering an association between a device and an xref phandle, create > > an entry in the xref list if one doesn't already exist for the given handle. > > > > On a system that uses phandle properties, the init-time scan of the tree > > which builds the xref list will pre-create entries for every xref handle > > that exists in the data. On systems where the xref and node handles are > > synonymous there is no phandle property in referenced nodes, and the xref > > list will initialize to an empty state. In the latter case, we still need > > to be able to associate a device_t with an xref handle, so we create list > > entries on the fly as needed. Since the node and xref handles are > > synonymous, we have all the info needed to create a list entry at device > > registration time. > > > > The downside to this change is that it basically allows on the fly creation > > of xref handles as synonyms of node handles, and the association of a > > device_t with them. Whether this is a bug or a feature is in the eye of > > the beholder, I guess. > > > > Modified: > > head/sys/dev/ofw/openfirm.c > > > > Modified: head/sys/dev/ofw/openfirm.c > > ============================================================================== > > --- head/sys/dev/ofw/openfirm.c Sat Sep 6 18:20:50 2014 (r271201) > > +++ head/sys/dev/ofw/openfirm.c Sat Sep 6 18:43:17 2014 (r271202) > > @@ -62,7 +62,10 @@ __FBSDID("$FreeBSD$"); > > > > #include > > #include > > +#include > > #include > > +#include > > +#include > > #include > > #include > > > > @@ -92,6 +95,7 @@ struct xrefinfo { > > }; > > > > static SLIST_HEAD(, xrefinfo) xreflist = SLIST_HEAD_INITIALIZER(xreflist); > > +static struct mtx xreflist_lock; > > static boolean_t xref_init_done; > > > > #define FIND_BY_XREF 0 > > @@ -138,6 +142,12 @@ static void > > xrefinfo_init(void *unsed) > > { > > > > + /* > > + * There is no locking during this init because it runs much earlier > > + * than any of the clients/consumers of the xref list data, but we do > > + * initialize the mutex that will be used for access later. > > + */ > > + mtx_init(&xreflist_lock, "OF xreflist lock", NULL, MTX_DEF); > > xrefinfo_create(OF_peer(0)); > > xref_init_done = true; > > } > > @@ -146,17 +156,35 @@ SYSINIT(xrefinfo, SI_SUB_KMEM, SI_ORDER_ > > static struct xrefinfo * > > xrefinfo_find(phandle_t phandle, int find_by) > > { > > - struct xrefinfo * xi; > > + struct xrefinfo *rv, *xi; > > > > + rv = NULL; > > + mtx_lock(&xreflist_lock); > > SLIST_FOREACH(xi, &xreflist, next_entry) { > > - if (find_by == FIND_BY_XREF && phandle == xi->xref) > > - return (xi); > > - else if (find_by == FIND_BY_NODE && phandle == xi->node) > > - return (xi); > > - else if (find_by == FIND_BY_DEV && phandle == (uintptr_t)xi->dev) > > - return (xi); > > + if ((find_by == FIND_BY_XREF && phandle == xi->xref) || > > + (find_by == FIND_BY_NODE && phandle == xi->node) || > > + (find_by == FIND_BY_DEV && phandle == (uintptr_t)xi->dev)) { > > + rv = xi; > > + break; > > + } > > } > > - return (NULL); > > + mtx_unlock(&xreflist_lock); > > + return (rv); > > +} > > + > > +static struct xrefinfo * > > +xrefinfo_add(phandle_t node, phandle_t xref, device_t dev) > > +{ > > + struct xrefinfo *xi; > > + > > + xi = malloc(sizeof(*xi), M_OFWPROP, M_WAITOK); > > + xi->node = node; > > + xi->xref = xref; > > + xi->dev = dev; > > + mtx_lock(&xreflist_lock); > > + SLIST_INSERT_HEAD(&xreflist, xi, next_entry); > > + mtx_unlock(&xreflist_lock); > > + return (xi); > > } > > > > /* > > @@ -605,10 +633,17 @@ OF_device_register_xref(phandle_t xref, > > { > > struct xrefinfo *xi; > > > > + /* > > + * If the given xref handle doesn't already exist in the list then we > > + * add a list entry. In theory this can only happen on a system where > > + * nodes don't contain phandle properties and xref and node handles are > > + * synonymous, so the xref handle is added as the node handle as well. > > + */ > > if (xref_init_done) { > > if ((xi = xrefinfo_find(xref, FIND_BY_XREF)) == NULL) > > - return (ENXIO); > > - xi->dev = dev; > > + xrefinfo_add(xref, xref, dev); > > + else > > + xi->dev = dev; > > return (0); > > } > > panic("Attempt to register device before xreflist_init"); > > >