[PATCH] of: Fix locking vs. interrupts

Subsystems: open firmware and flattened device tree, sparc + ultrasparc (sparc/sparc64), the rest

STALE4860d

5 messages, 4 authors, 2013-06-12 · open the first message on its own page

[PATCH] of: Fix locking vs. interrupts

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2013-06-12 05:39:29

The OF code uses irqsafe locks everywhere except in a handful of functions
for no obvious reasons. Since the conversion from the old rwlocks, this
now triggers lockdep warnings when used at interrupt time. At least one
driver (ibmvscsi) seems to be doing that from softirq context.

This converts the few non-irqsafe locks into irqsafe ones, making them
consistent with the rest of the code.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <redacted> [v3.9+]
---

Note: It's silly to access the device-tree at interrupt time in most cases,
and we should probably fix ibmvscsi, but for the time being, let's fix the
obvious bug. Thomas, this can probably still go into 3.10... If not, I've
CCed stable.
diff --git a/arch/sparc/kernel/prom_common.c b/arch/sparc/kernel/prom_common.c
index 9f20566..79cc0d1 100644
--- a/arch/sparc/kernel/prom_common.c
+++ b/arch/sparc/kernel/prom_common.c
@@ -54,6 +54,7 @@ EXPORT_SYMBOL(of_set_property_mutex);
 int of_set_property(struct device_node *dp, const char *name, void *val, int len)
 {
 	struct property **prevp;
+	unsigned long flags;
 	void *new_val;
 	int err;
 
@@ -64,7 +65,7 @@ int of_set_property(struct device_node *dp, const char *name, void *val, int len
 	err = -ENODEV;
 
 	mutex_lock(&of_set_property_mutex);
-	raw_spin_lock(&devtree_lock);
+	raw_spin_lock_irqsave(&devtree_lock, flags);
 	prevp = &dp->properties;
 	while (*prevp) {
 		struct property *prop = *prevp;
@@ -91,7 +92,7 @@ int of_set_property(struct device_node *dp, const char *name, void *val, int len
 		}
 		prevp = &(*prevp)->next;
 	}
-	raw_spin_unlock(&devtree_lock);
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 	mutex_unlock(&of_set_property_mutex);
 
 	/* XXX Upate procfs if necessary... */
diff --git a/drivers/of/base.c b/drivers/of/base.c
index f53b992..951452c 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -192,14 +192,15 @@ EXPORT_SYMBOL(of_find_property);
 struct device_node *of_find_all_nodes(struct device_node *prev)
 {
 	struct device_node *np;
+	unsigned long flags;
 
-	raw_spin_lock(&devtree_lock);
+	raw_spin_lock_irqsave(&devtree_lock, flags);
 	np = prev ? prev->allnext : of_allnodes;
 	for (; np != NULL; np = np->allnext)
 		if (of_node_get(np))
 			break;
 	of_node_put(prev);
-	raw_spin_unlock(&devtree_lock);
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 	return np;
 }
 EXPORT_SYMBOL(of_find_all_nodes);
@@ -421,8 +422,9 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
 	struct device_node *prev)
 {
 	struct device_node *next;
+	unsigned long flags;
 
-	raw_spin_lock(&devtree_lock);
+	raw_spin_lock_irqsave(&devtree_lock, flags);
 	next = prev ? prev->sibling : node->child;
 	for (; next; next = next->sibling) {
 		if (!__of_device_is_available(next))
@@ -431,7 +433,7 @@ struct device_node *of_get_next_available_child(const struct device_node *node,
 			break;
 	}
 	of_node_put(prev);
-	raw_spin_unlock(&devtree_lock);
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 	return next;
 }
 EXPORT_SYMBOL(of_get_next_available_child);
@@ -735,13 +737,14 @@ EXPORT_SYMBOL_GPL(of_modalias_node);
 struct device_node *of_find_node_by_phandle(phandle handle)
 {
 	struct device_node *np;
+	unsigned long flags;
 
-	raw_spin_lock(&devtree_lock);
+	raw_spin_lock_irqsave(&devtree_lock, flags);
 	for (np = of_allnodes; np; np = np->allnext)
 		if (np->phandle == handle)
 			break;
 	of_node_get(np);
-	raw_spin_unlock(&devtree_lock);
+	raw_spin_unlock_irqrestore(&devtree_lock, flags);
 	return np;
 }
 EXPORT_SYMBOL(of_find_node_by_phandle);

Re: [PATCH] of: Fix locking vs. interrupts

From: Thomas Gleixner <hidden>
Date: 2013-06-12 08:26:07

On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
The OF code uses irqsafe locks everywhere except in a handful of functions
for no obvious reasons. Since the conversion from the old rwlocks, this
now triggers lockdep warnings when used at interrupt time. At least one
driver (ibmvscsi) seems to be doing that from softirq context.

This converts the few non-irqsafe locks into irqsafe ones, making them
consistent with the rest of the code.
Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost 
 
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <redacted> [v3.9+]
Acked-by: Thomas Gleixner <redacted>
---

Note: It's silly to access the device-tree at interrupt time in most cases,
and we should probably fix ibmvscsi, but for the time being, let's fix the
Right.
obvious bug. Thomas, this can probably still go into 3.10... If not, I've
CCed stable.
Should go through Grant I think.

Thanks,

	tglx

Re: [PATCH] of: Fix locking vs. interrupts

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2013-06-12 08:56:56

On Wed, 2013-06-12 at 10:25 +0200, Thomas Gleixner wrote:
On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
quoted
The OF code uses irqsafe locks everywhere except in a handful of functions
for no obvious reasons. Since the conversion from the old rwlocks, this
now triggers lockdep warnings when used at interrupt time. At least one
driver (ibmvscsi) seems to be doing that from softirq context.

This converts the few non-irqsafe locks into irqsafe ones, making them
consistent with the rest of the code.
Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost 
 
quoted
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <redacted> [v3.9+]
Acked-by: Thomas Gleixner <redacted>
quoted
---

Note: It's silly to access the device-tree at interrupt time in most cases,
and we should probably fix ibmvscsi, but for the time being, let's fix the
Right.
quoted
obvious bug. Thomas, this can probably still go into 3.10... If not, I've
CCed stable.
Should go through Grant I think.
Right, thinko. Sent to you due to the bug being exposed by your
conversion to spinlocks. Anyway, Grant got it now.

Cheers,
Ben.

Re: [PATCH] of: Fix locking vs. interrupts

From: Grant Likely <hidden>
Date: 2013-06-12 09:06:23

On Wed, 12 Jun 2013 10:25:56 +0200 (CEST), Thomas Gleixner [off-list ref] wrote:
On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
quoted
The OF code uses irqsafe locks everywhere except in a handful of functions
for no obvious reasons. Since the conversion from the old rwlocks, this
now triggers lockdep warnings when used at interrupt time. At least one
driver (ibmvscsi) seems to be doing that from softirq context.

This converts the few non-irqsafe locks into irqsafe ones, making them
consistent with the rest of the code.
Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost 
 
quoted
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <redacted> [v3.9+]
Acked-by: Thomas Gleixner <redacted>
quoted
---

Note: It's silly to access the device-tree at interrupt time in most cases,
and we should probably fix ibmvscsi, but for the time being, let's fix the
Right.
quoted
obvious bug. Thomas, this can probably still go into 3.10... If not, I've
CCed stable.
Should go through Grant I think.
Applied, thanks.

g.

Re: [PATCH] of: Fix locking vs. interrupts

From: David Miller <davem@davemloft.net>
Date: 2013-06-12 09:15:44

From: Grant Likely <redacted>
Date: Wed, 12 Jun 2013 10:06:12 +0100
On Wed, 12 Jun 2013 10:25:56 +0200 (CEST), Thomas Gleixner [off-list ref] wrote:
quoted
On Wed, 12 Jun 2013, Benjamin Herrenschmidt wrote:
quoted
The OF code uses irqsafe locks everywhere except in a handful of functions
for no obvious reasons. Since the conversion from the old rwlocks, this
now triggers lockdep warnings when used at interrupt time. At least one
driver (ibmvscsi) seems to be doing that from softirq context.

This converts the few non-irqsafe locks into irqsafe ones, making them
consistent with the rest of the code.
Fun. https://lkml.org/lkml/2013/2/4/416 seems to have got lost 
 
quoted
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <redacted> [v3.9+]
Acked-by: Thomas Gleixner <redacted>
quoted
---

Note: It's silly to access the device-tree at interrupt time in most cases,
and we should probably fix ibmvscsi, but for the time being, let's fix the
Right.
quoted
obvious bug. Thomas, this can probably still go into 3.10... If not, I've
CCed stable.
Should go through Grant I think.
Applied, thanks.
FWIW:

Acked-by: David S. Miller <davem@davemloft.net>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help