Date: Sat, 06 Sep 2014 13:41:55 -0700 From: Nathan Whitehorn <nwhitehorn@freebsd.org> To: Ian Lepore <ian@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r271202 - head/sys/dev/ofw Message-ID: <540B7193.8060601@freebsd.org> In-Reply-To: <1410032406.1150.370.camel@revolution.hippie.lan> References: <201409061843.s86IhHMJ054856@svn.freebsd.org> <540B5DA8.3080601@freebsd.org> <1410032406.1150.370.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
On 09/06/14 12:40, Ian Lepore wrote: > 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 Thanks for the explanation! That seems like the most reasonable approach. -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?540B7193.8060601>