Date: Sat, 06 Sep 2014 12:16:56 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Ian Lepore <ian@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r271202 - head/sys/dev/ofw Message-ID: <540B5DA8.3080601@freebsd.org> In-Reply-To: <201409061843.s86IhHMJ054856@svn.freebsd.org> References: <201409061843.s86IhHMJ054856@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 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 <sys/param.h> > #include <sys/kernel.h> > +#include <sys/lock.h> > #include <sys/malloc.h> > +#include <sys/mutex.h> > +#include <sys/queue.h> > #include <sys/systm.h> > #include <sys/endian.h> > > @@ -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"); >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?540B5DA8.3080601>