Re: pre/post_reset methods for skel

On Tue, 8 May 2007, Oliver Neukum wrote:

> Hi,
> 
> this shows how the anchors can be used for doing reset support.
> Hibernate, when we get it should be similar.

I'll have more comments on this later.

But first, I think we should be able to get rid of the skel_open_lock
mutex plus similar mutexes in other USB drivers.  The problem they address
is the race between open() and disconnect() -- or more accurately, open()
and unregister_dev().  It should be fixed once and for all in usbcore.  
And without relying on the BKL.

How do you like this approach?

Alan Stern


Index: usb-2.6/drivers/usb/core/file.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/file.c
+++ usb-2.6/drivers/usb/core/file.c
@@ -16,15 +16,15 @@
  */
 
 #include <linux/module.h>
-#include <linux/spinlock.h>
 #include <linux/errno.h>
+#include <linux/rwsem.h>
 #include <linux/usb.h>
 
 #include "usb.h"
 
 #define MAX_USB_MINORS	256
 static const struct file_operations *usb_minors[MAX_USB_MINORS];
-static DEFINE_SPINLOCK(minor_lock);
+static DECLARE_RWSEM(minor_rwsem);
 
 static int usb_open(struct inode * inode, struct file * file)
 {
@@ -33,14 +33,11 @@ static int usb_open(struct inode * inode
 	int err = -ENODEV;
 	const struct file_operations *old_fops, *new_fops = NULL;
 
-	spin_lock (&minor_lock);
+	down_read(&minor_rwsem);
 	c = usb_minors[minor];
 
-	if (!c || !(new_fops = fops_get(c))) {
-		spin_unlock(&minor_lock);
-		return err;
-	}
-	spin_unlock(&minor_lock);
+	if (!c || !(new_fops = fops_get(c)))
+		goto done;
 
 	old_fops = file->f_op;
 	file->f_op = new_fops;
@@ -52,6 +49,8 @@ static int usb_open(struct inode * inode
 		file->f_op = fops_get(old_fops);
 	}
 	fops_put(old_fops);
+ done:
+	up_read(&minor_rwsem);
 	return err;
 }
 
@@ -124,7 +123,7 @@ int usb_register_dev(struct usb_interfac
 	if (class_driver->fops == NULL)
 		goto exit;
 
-	spin_lock (&minor_lock);
+	down_write(&minor_rwsem);
 	for (minor = minor_base; minor < MAX_USB_MINORS; ++minor) {
 		if (usb_minors[minor])
 			continue;
@@ -134,7 +133,7 @@ int usb_register_dev(struct usb_interfac
 		retval = 0;
 		break;
 	}
-	spin_unlock (&minor_lock);
+	up_write(&minor_rwsem);
 
 	if (retval)
 		goto exit;
@@ -150,9 +149,9 @@ int usb_register_dev(struct usb_interfac
 	intf->usb_dev = device_create(&usb_class, &intf->dev,
 				      MKDEV(USB_MAJOR, minor), "%s", temp);
 	if (IS_ERR(intf->usb_dev)) {
-		spin_lock (&minor_lock);
+		down_write(&minor_rwsem);
 		usb_minors[intf->minor] = NULL;
-		spin_unlock (&minor_lock);
+		up_write(&minor_rwsem);
 		retval = PTR_ERR(intf->usb_dev);
 	}
 exit:
@@ -189,9 +188,9 @@ void usb_deregister_dev(struct usb_inter
 
 	dbg ("removing %d minor", intf->minor);
 
-	spin_lock (&minor_lock);
+	down_write(&minor_rwsem);
 	usb_minors[intf->minor] = NULL;
-	spin_unlock (&minor_lock);
+	up_write(&minor_rwsem);
 
 	snprintf(name, BUS_ID_SIZE, class_driver->name, intf->minor - minor_base);
 	device_destroy(&usb_class, MKDEV(USB_MAJOR, intf->minor));


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel