From owner-svn-src-head@FreeBSD.ORG Sat Sep 6 20:41:59 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 F24898D7; Sat, 6 Sep 2014 20:41:58 +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 D6D6F1379; Sat, 6 Sep 2014 20:41:58 +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 s86KftEI014037 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Sat, 6 Sep 2014 13:41:56 -0700 Message-ID: <540B7193.8060601@freebsd.org> Date: Sat, 06 Sep 2014 13:41:55 -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 Subject: Re: svn commit: r271202 - head/sys/dev/ofw References: <201409061843.s86IhHMJ054856@svn.freebsd.org> <540B5DA8.3080601@freebsd.org> <1410032406.1150.370.camel@revolution.hippie.lan> In-Reply-To: <1410032406.1150.370.camel@revolution.hippie.lan> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Sonic-CAuth: UmFuZG9tSVZ5sIj8gt8EWHdlNQIJQfF7g8qmtuQFDYZGzMVimm99B9FZdCDiFIizs5/Su+gZ2vSgqxjtFqppuPv7Xy5kRkH79wjNxogP/H8= X-Sonic-ID: C;5hktPQY25BGELUpcoK8kYw== M;BI+ePQY25BGELUpcoK8kYw== X-Spam-Flag: No X-Sonic-Spam-Details: 0.0/5.0 by cerberusd 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 20:41:59 -0000 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 >>> #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"); >>> >