diff mbox

sparc: Remove bogus of_set_property_mutex

Message ID 20091107184538.543684450@linutronix.de
State Rejected
Delegated to: David Miller
Headers show

Commit Message

Thomas Gleixner Nov. 7, 2009, 6:58 p.m. UTC 
of_set_property_mutex is taken inside the devtree_lock write locked
region which triggers the might_sleep check. The mutex protects the
call to prom_setprop() which is not necessary as the code is the only
caller of prom_setprop() and already serialized by devtree_lock. The
mutex is nowhere else used despite being exported. So it can be
removed safely.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
---
 arch/sparc/include/asm/prom.h   |    2 --
 arch/sparc/kernel/prom_common.c |    5 -----
 2 files changed, 7 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller Nov. 8, 2009, 4:39 a.m. UTC | #1 
From: Thomas Gleixner <tglx@linutronix.de>
Date: Sat, 07 Nov 2009 18:58:13 -0000

> of_set_property_mutex is taken inside the devtree_lock write locked
> region which triggers the might_sleep check. The mutex protects the
> call to prom_setprop() which is not necessary as the code is the only
> caller of prom_setprop() and already serialized by devtree_lock. The
> mutex is nowhere else used despite being exported. So it can be
> removed safely.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Please see commit 2481d76615d5e15340ccfb0243fe8779766dfc6e to see why
this mutex really is necessary, regardless of the locking done by the
caller(s) of of_set_property().  And how it will thus need to be used
in the future.

The long and short of it is that the firmware uses I2C accesses to
write the property values on some systems, and therefore the sparc64
I2C bus drivers will need this mutex to coordinate with callers of
prom_set_property().  Those I2C drivers aren't merged yet, but I
definitely plan to get them in soon :-)
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Gleixner Nov. 8, 2009, 8:16 p.m. UTC | #2 
On Sat, 7 Nov 2009, David Miller wrote:

> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Sat, 07 Nov 2009 18:58:13 -0000
> 
> > of_set_property_mutex is taken inside the devtree_lock write locked
> > region which triggers the might_sleep check. The mutex protects the
> > call to prom_setprop() which is not necessary as the code is the only
> > caller of prom_setprop() and already serialized by devtree_lock. The
> > mutex is nowhere else used despite being exported. So it can be
> > removed safely.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> Please see commit 2481d76615d5e15340ccfb0243fe8779766dfc6e to see why
> this mutex really is necessary, regardless of the locking done by the
> caller(s) of of_set_property().  And how it will thus need to be used
> in the future.
> 
> The long and short of it is that the firmware uses I2C accesses to
> write the property values on some systems, and therefore the sparc64
> I2C bus drivers will need this mutex to coordinate with callers of
> prom_set_property().  Those I2C drivers aren't merged yet, but I
> definitely plan to get them in soon :-)

Then you need to fix the problem that you lock the mutex inside the
preempt disabled region under devtree_lock. :)

Thanks,

	tglx

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: linux-2.6/arch/sparc/include/asm/prom.h
===================================================================
--- linux-2.6.orig/arch/sparc/include/asm/prom.h
+++ linux-2.6/arch/sparc/include/asm/prom.h
@@ -18,7 +18,6 @@ 
  */
 #include <linux/types.h>
 #include <linux/proc_fs.h>
-#include <linux/mutex.h>
 #include <asm/atomic.h>
 
 #define OF_ROOT_NODE_ADDR_CELLS_DEFAULT	2
@@ -74,7 +73,6 @@  struct of_irq_controller {
 
 extern struct device_node *of_find_node_by_cpuid(int cpuid);
 extern int of_set_property(struct device_node *node, const char *name, void *val, int len);
-extern struct mutex of_set_property_mutex;
 extern int of_getintprop_default(struct device_node *np,
 				 const char *name,
 				 int def);
Index: linux-2.6/arch/sparc/kernel/prom_common.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/prom_common.c
+++ linux-2.6/arch/sparc/kernel/prom_common.c
@@ -62,9 +62,6 @@  int of_getintprop_default(struct device_
 }
 EXPORT_SYMBOL(of_getintprop_default);
 
-DEFINE_MUTEX(of_set_property_mutex);
-EXPORT_SYMBOL(of_set_property_mutex);
-
 int of_set_property(struct device_node *dp, const char *name, void *val, int len)
 {
 	struct property **prevp;
@@ -88,9 +85,7 @@  int of_set_property(struct device_node *
 			void *old_val = prop->value;
 			int ret;
 
-			mutex_lock(&of_set_property_mutex);
 			ret = prom_setprop(dp->node, name, val, len);
-			mutex_unlock(&of_set_property_mutex);
 
 			err = -EINVAL;
 			if (ret >= 0) {