[PATCH 2/3] struct class sem to mutex converting

[PATCH 2/3] struct class sem to mutex converting

[Dave Young]

The class_device is already removed, so do the class->sem to mutex converting.

Signed-off-by: Dave Young <hidave.darkstar@gmail.com>

---
drivers/base/class.c   |   22 +++++++++++-----------
drivers/base/core.c    |    9 ++++-----
include/linux/device.h |    3 ++-
3 files changed, 17 insertions(+), 17 deletions(-)

diff -upr linux/drivers/base/class.c linux.new/drivers/base/class.c
--- linux/drivers/base/class.c	2008-05-09 12:53:35.000000000 +0800
+++ linux.new/drivers/base/class.c	2008-05-09 13:03:48.000000000 +0800
@@ -144,7 +144,7 @@ int class_register(struct class *cls)
 	INIT_LIST_HEAD(&cls->devices);
 	INIT_LIST_HEAD(&cls->interfaces);
 	kset_init(&cls->class_dirs);
-	init_MUTEX(&cls->sem);
+	mutex_init(&cls->mutex);
 	error = kobject_set_name(&cls->subsys.kobj, "%s", cls->name);
 	if (error)
 		return error;
@@ -262,7 +262,7 @@ char *make_class_name(const char *name, 
  * We check the return of @fn each time. If it returns anything
  * other than 0, we break out and return that value.
  *
- * Note, we hold class->sem in this function, so it can not be
+ * Note, we hold class->mutex in this function, so it can not be
  * re-acquired in @fn, otherwise it will self-deadlocking. For
  * example, calls to add or remove class members would be verboten.
  */
@@ -274,7 +274,7 @@ int class_for_each_device(struct class *
 
 	if (!class)
 		return -EINVAL;
-	down(&class->sem);
+	mutex_lock(&class->mutex);
 	list_for_each_entry(dev, &class->devices, node) {
 		dev = get_device(dev);
 		if (dev) {
@@ -285,7 +285,7 @@ int class_for_each_device(struct class *
 		if (error)
 			break;
 	}
-	up(&class->sem);
+	mutex_unlock(&class->mutex);
 
 	return error;
 }
@@ -307,7 +307,7 @@ EXPORT_SYMBOL_GPL(class_for_each_device)
  *
  * Note, you will need to drop the reference with put_device() after use.
  *
- * We hold class->sem in this function, so it can not be
+ * We hold class->mutex in this function, so it can not be
  * re-acquired in @match, otherwise it will self-deadlocking. For
  * example, calls to add or remove class members would be verboten.
  */
@@ -320,7 +320,7 @@ struct device *class_find_device(struct 
 	if (!class)
 		return NULL;
 
-	down(&class->sem);
+	mutex_lock(&class->mutex);
 	list_for_each_entry(dev, &class->devices, node) {
 		dev = get_device(dev);
 		if (dev) {
@@ -332,7 +332,7 @@ struct device *class_find_device(struct 
 		} else
 			break;
 	}
-	up(&class->sem);
+	mutex_unlock(&class->mutex);
 
 	return found ? dev : NULL;
 }
@@ -350,13 +350,13 @@ int class_interface_register(struct clas
 	if (!parent)
 		return -EINVAL;
 
-	down(&parent->sem);
+	mutex_lock(&parent->mutex);
 	list_add_tail(&class_intf->node, &parent->interfaces);
 	if (class_intf->add_dev) {
 		list_for_each_entry(dev, &parent->devices, node)
 			class_intf->add_dev(dev, class_intf);
 	}
-	up(&parent->sem);
+	mutex_unlock(&parent->mutex);
 
 	return 0;
 }
@@ -369,13 +369,13 @@ void class_interface_unregister(struct c
 	if (!parent)
 		return;
 
-	down(&parent->sem);
+	mutex_lock(&parent->mutex);
 	list_del_init(&class_intf->node);
 	if (class_intf->remove_dev) {
 		list_for_each_entry(dev, &parent->devices, node)
 			class_intf->remove_dev(dev, class_intf);
 	}
-	up(&parent->sem);
+	mutex_unlock(&parent->mutex);
 
 	class_put(parent);
 }
diff -upr linux/drivers/base/core.c linux.new/drivers/base/core.c
--- linux/drivers/base/core.c	2008-05-09 12:53:35.000000000 +0800
+++ linux.new/drivers/base/core.c	2008-05-09 12:59:41.000000000 +0800
@@ -20,7 +20,6 @@
 #include <linux/notifier.h>
 #include <linux/genhd.h>
 #include <linux/kallsyms.h>
-#include <linux/semaphore.h>
 
 #include "base.h"
 #include "power/power.h"
@@ -833,7 +832,7 @@ int device_add(struct device *dev)
 		klist_add_tail(&dev->knode_parent, &parent->klist_children);
 
 	if (dev->class) {
-		down(&dev->class->sem);
+		mutex_lock(&dev->class->mutex);
 		/* tie the class to the device */
 		list_add_tail(&dev->node, &dev->class->devices);
 
@@ -841,7 +840,7 @@ int device_add(struct device *dev)
 		list_for_each_entry(class_intf, &dev->class->interfaces, node)
 			if (class_intf->add_dev)
 				class_intf->add_dev(dev, class_intf);
-		up(&dev->class->sem);
+		mutex_unlock(&dev->class->mutex);
 	}
  Done:
 	put_device(dev);
@@ -937,14 +936,14 @@ void device_del(struct device *dev)
 	if (dev->class) {
 		device_remove_class_symlinks(dev);
 
-		down(&dev->class->sem);
+		mutex_lock(&dev->class->mutex);
 		/* notify any interfaces that the device is now gone */
 		list_for_each_entry(class_intf, &dev->class->interfaces, node)
 			if (class_intf->remove_dev)
 				class_intf->remove_dev(dev, class_intf);
 		/* remove the device from the class list */
 		list_del_init(&dev->node);
-		up(&dev->class->sem);
+		mutex_unlock(&dev->class->mutex);
 	}
 	device_remove_file(dev, &uevent_attr);
 	device_remove_attrs(dev);
diff -upr linux/include/linux/device.h linux.new/include/linux/device.h
--- linux/include/linux/device.h	2008-05-09 12:53:17.000000000 +0800
+++ linux.new/include/linux/device.h	2008-05-09 12:56:08.000000000 +0800
@@ -20,6 +20,7 @@
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/pm.h>
+#include <linux/mutex.h>
 #include <linux/semaphore.h>
 #include <asm/atomic.h>
 #include <asm/device.h>
@@ -187,7 +188,7 @@ struct class {
 	struct list_head	devices;
 	struct list_head	interfaces;
 	struct kset		class_dirs;
-	struct semaphore	sem; /* locks children, devices, interfaces */
+	struct mutex		mutex; /* locks devices, interfaces */
 	struct class_attribute		*class_attrs;
 	struct device_attribute		*dev_attrs;
 

Re: [PATCH 2/3] struct class sem to mutex converting

[Greg KH]

On Fri, May 09, 2008 at 03:22:31PM +0800, Dave Young wrote:
> The class_device is already removed, so do the class->sem to mutex converting.

Are you sure this will work?  Have you tried running lockdep to verify
that there are no problems here?  People have tried this in the past
with devices and I don't think it will work here either :(

thanks,

greg k-h

Re: [PATCH 2/3] struct class sem to mutex converting

[Dave Young]

On Wed, May 14, 2008 at 6:01 AM, Greg KH <greg@kroah.com> wrote:
> On Fri, May 09, 2008 at 03:22:31PM +0800, Dave Young wrote:
>  > The class_device is already removed, so do the class->sem to mutex converting.
>
>  Are you sure this will work?  Have you tried running lockdep to verify
>  that there are no problems here?  People have tried this in the past
>  with devices and I don't think it will work here either :(

Yes, lockdep doesn't generate warnings on my side.

I ever wanted to do the conversion, please see:
http://lkml.org/lkml/2008/1/12/49

The result is that device sem2mutex is not possible now, but class
sem2mutex is possible.  After the class_device removing, all the
class->sem usage exist in class.c except for i2c part.

So I make i2c to use class_for_each_device api, so we can do the
class->sem conversion.

>
>  thanks,
>
>  greg k-h
>

Re: [PATCH 2/3] struct class sem to mutex converting

[Greg KH]

On Wed, May 14, 2008 at 09:34:13AM +0800, Dave Young wrote:
> On Wed, May 14, 2008 at 6:01 AM, Greg KH <greg@kroah.com> wrote:
> > On Fri, May 09, 2008 at 03:22:31PM +0800, Dave Young wrote:
> >  > The class_device is already removed, so do the class->sem to mutex converting.
> >
> >  Are you sure this will work?  Have you tried running lockdep to verify
> >  that there are no problems here?  People have tried this in the past
> >  with devices and I don't think it will work here either :(
> 
> Yes, lockdep doesn't generate warnings on my side.

I think Andrew's recent post to linux-scsi proves this is incorrect :)

> I ever wanted to do the conversion, please see:
> http://lkml.org/lkml/2008/1/12/49
> 
> The result is that device sem2mutex is not possible now, but class
> sem2mutex is possible.  After the class_device removing, all the
> class->sem usage exist in class.c except for i2c part.
> 
> So I make i2c to use class_for_each_device api, so we can do the
> class->sem conversion.

Ah, I didn't realize that the i2c patch was required.  Let me know when
Jean takes that patch in his tree.

thanks,

greg k-h