From owner-svn-src-head@FreeBSD.ORG Sat Sep 6 19:17:00 2014
Return-Path: <owner-svn-src-head@FreeBSD.ORG>
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 <nwhitehorn@freebsd.org>
User-Agent: Mozilla/5.0 (X11; FreeBSD amd64;
rv:31.0) Gecko/20100101 Thunderbird/31.0
MIME-Version: 1.0
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
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
<svn-src-head.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/svn-src-head>,
<mailto:svn-src-head-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-head/>
List-Post: <mailto:svn-src-head@freebsd.org>
List-Help: <mailto:svn-src-head-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-head>,
<mailto:svn-src-head-request@freebsd.org?subject=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 <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");
>