Will,

On Wed, 12 Aug 2009, Will Schmidt wrote:

I almost expected some wreckage from converting a rwlock to a spinlock. Grr. That code really takes the lock recursive. None of my powerpc systems triggered that code path. :(

So the reason for having a rwlock was not the concurrent access, it was just the lazy^W reuse of the existing functions which lead to recursive locking.

As it's the only code in tree which would need the real rwlock version I fixed up the of code instead of creating all the extra rwlock functions for rt.

Does the patch below fix it ?

Thanks,

tglx

----- commit 925df9b36dc9abe0bab32cbd2ac544d773da71ff Author: Thomas Gleixner <tg...@linutronix.de> Date: Thu Aug 13 09:04:10 2009 +0200

OF: Fixup resursive locking code paths

The conversion of devtree_lock from rwlock to spinlock unearthed recursive locking pathes. Instead of going down the hassle of having an atomic_rwlock implementation fixup the code pathes to avoid the recursive locking.

Signed-off-by: Thomas Gleixner <tg...@linutronix.de>

diff --git a/drivers/of/base.c b/drivers/of/base.c index 26f4a62..3626e37 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -59,16 +59,14 @@ int of_n_size_cells(struct device_node *np) } EXPORT_SYMBOL(of_n_size_cells);

-struct property *of_find_property(const struct device_node *np, - const char *name, - int *lenp) +static struct property *__of_find_property(const struct device_node *np, + const char *name, int *lenp) { struct property *pp;

if (!np) return NULL;

- atomic_spin_lock(&devtree_lock); for (pp = np->properties; pp != 0; pp = pp->next) { if (of_prop_cmp(pp->name, name) == 0) { if (lenp != 0) @@ -76,6 +74,18 @@ struct property *of_find_property(const struct device_node
*np, break; } } + + return pp; +} + +struct property *of_find_property(const struct device_node *np, + const char *name, + int *lenp) +{ + struct property *pp; + + atomic_spin_lock(&devtree_lock); + pp = __of_find_property(np, name, lenp); atomic_spin_unlock(&devtree_lock);

return pp; @@ -86,8 +96,20 @@ EXPORT_SYMBOL(of_find_property); * Find a property with a given name for a given node * and return the value. */ +static const void *__of_get_property(const struct device_node *np, + const char *name, int *lenp) +{ + struct property *pp = __of_find_property(np, name, lenp); + + return pp ? pp->value : NULL; +} + +/* + * Find a property with a given name for a given node + * and return the value. + */ const void *of_get_property(const struct device_node *np, const char *name, - int *lenp) + int *lenp) { struct property *pp = of_find_property(np, name, lenp);

@@ -98,13 +120,13 @@ EXPORT_SYMBOL(of_get_property); /** Checks if the given "compat" string matches one of the strings in * the device's "compatible" property */ -int of_device_is_compatible(const struct device_node *device, - const char *compat) +static int __of_device_is_compatible(const struct device_node *device, + const char *compat) { const char* cp; - int cplen, l; + int uninitialized_var(cplen), l;

- cp = of_get_property(device, "compatible", &cplen); + cp = __of_get_property(device, "compatible", &cplen); if (cp == NULL) return 0; while (cplen > 0) { @@ -117,6 +139,20 @@ int of_device_is_compatible(const struct device_node
*device,

return 0; } + +/** Checks if the given "compat" string matches one of the strings in + * the device's "compatible" property + */ +int of_device_is_compatible(const struct device_node *device, + const char *compat) +{ + int res; + + atomic_spin_lock(&devtree_lock); + res = __of_device_is_compatible(device, compat); + atomic_spin_unlock(&devtree_lock); + return res; +} EXPORT_SYMBOL(of_device_is_compatible);

/** @@ -319,7 +355,8 @@ struct device_node *of_find_compatible_node(struct
device_node *from, if (type && !(np->type && (of_node_cmp(np->type, type) == 0))) continue; - if (of_device_is_compatible(np, compatible) && of_node_get(np)) + if (__of_device_is_compatible(np, compatible) && + of_node_get(np)) break; } of_node_put(from); @@ -363,15 +400,9 @@ out: } EXPORT_SYMBOL(of_find_node_with_property);

-/** - * of_match_node - Tell if an device_node has a matching of_match structure - * @matches: array of of device match structures to search in - * @node: the of device structure to match against - * - * Low level utility function used by device matching. - */ -const struct of_device_id *of_match_node(const struct of_device_id *matches, - const struct device_node *node) +static const struct of_device_id * +__of_match_node(const struct of_device_id *matches, + const struct device_node *node) { while (matches->name[0] || matches->type[0] || matches->compatible[0]) { int match = 1; @@ -382,14 +413,32 @@ const struct of_device_id *of_match_node(const struct
of_device_id *matches, match &= node->type && !strcmp(matches->type, node->type); if (matches->compatible[0]) - match &= of_device_is_compatible(node, - matches->compatible); + match &= __of_device_is_compatible(node, + matches->compatible); if (match) return matches; matches++; } return NULL; } + +/** + * of_match_node - Tell if an device_node has a matching of_match structure + * @matches: array of of device match structures to search in + * @node: the of device structure to match against + * + * Low level utility function used by device matching. + */ +const struct of_device_id *of_match_node(const struct of_device_id *matches, + const struct device_node *node) +{ + const struct of_device_id *match; + + atomic_spin_lock(&devtree_lock); + match = __of_match_node(matches, node); + atomic_spin_unlock(&devtree_lock); + return match; +} EXPORT_SYMBOL(of_match_node);

/** @@ -412,7 +461,7 @@ struct device_node *of_find_matching_node(struct device_node
*from, atomic_spin_lock(&devtree_lock); np = from ? from->allnext : allnodes; for (; np; np = np->allnext) { - if (of_match_node(matches, np) && of_node_get(np)) + if (__of_match_node(matches, np) && of_node_get(np)) break; } of_node_put(from);