Skip site navigation (1)Skip section navigation (2)
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>