From owner-svn-src-head@FreeBSD.ORG Sat Sep 6 19:17:00 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 67EA0612; Sat, 6 Sep 2014 19:17:00 +0000 (UTC) Received: from c.mail.sonic.net (c.mail.sonic.net [64.142.111.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4E5331AD1; Sat, 6 Sep 2014 19:16:59 +0000 (UTC) Received: from zeppelin.tachypleus.net (polaris.tachypleus.net [75.101.50.44]) (authenticated bits=0) by c.mail.sonic.net (8.14.9/8.14.9) with ESMTP id s86JGu0C022796 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Sat, 6 Sep 2014 12:16:57 -0700 Message-ID: <540B5DA8.3080601@freebsd.org> Date: Sat, 06 Sep 2014 12:16:56 -0700 From: Nathan Whitehorn User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:31.0) Gecko/20100101 Thunderbird/31.0 MIME-Version: 1.0 To: Ian Lepore , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r271202 - head/sys/dev/ofw References: <201409061843.s86IhHMJ054856@svn.freebsd.org> In-Reply-To: <201409061843.s86IhHMJ054856@svn.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVbSeCFrWlii72fyLpzyJ4Za+ZgKmzM/AodZHrEzSCQ8eRq3es6tTMuuUeLfO4khe+KdpuItFem6kSyJFRKZ2WFoskKDM/xKkuI= X-Sonic-ID: C;+P84Xvo15BGjnEpcoK8kYw== M;+KiUXvo15BGjnEpcoK8kYw== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd 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:17:00 -0000 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 > #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"); >