Re: [ANNOUNCE] 2.6.31-rc4-rt1

Thomas Gleixner
2009-08-13T07:31:08Z

Will,

On Wed, 12 Aug 2009, Will Schmidt wrote:
> The .find_legacy_serial_ports function in the backtrace is very very
> early during boot. I'll do a bit more looking around to see if it's
> anything obvious,..

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 <tglx@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 <tglx@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);