Re: [PATCH] Fix removable USB drive oops

Re: [PATCH] Fix removable USB drive oops

[Greg KH]

On Sat, Mar 13, 2004 at 06:48:36PM +0000, Linux Kernel Mailing List wrote:
> ChangeSet 1.1623, 2004/03/13 13:48:36-05:00, James.Bottomley@steeleye.com
> 
> 	[PATCH] Fix removable USB drive oops
> 	
> 	The actual problem reported was because there wasn't a corresponding
> 	check on transport_classdev.class in the unregister.
> 	
> 	However, on closer inspection I also turned up a nasty thinko in the
> 	reference counting.  For reasons best known to the class code authors,
> 	class devices have to obtain their own references to the devices they're
> 	attached to which they release again in their .release routines, so you
> 	have to remember to do a get_device() in the correct place after the
> 	class_device_add().  I put comments in the code so that, hopefully, we
> 	can avoid the problem in future.

Bah, this was my fault, sorry.

Here's a patch that should fix this, and prevent you from needing this
patch.  Can you verify this?

thanks,

greg k-h


# Driver class: remove possible oops
#
# This happens when the device associated with a class device goes away before
# the class does.

diff -Nru a/drivers/base/class.c b/drivers/base/class.c
--- a/drivers/base/class.c	Wed Mar 17 16:57:10 2004
+++ b/drivers/base/class.c	Wed Mar 17 16:57:10 2004
@@ -155,8 +155,7 @@
 
 static void class_device_dev_unlink(struct class_device * class_dev)
 {
-	if (class_dev->dev)
-		sysfs_remove_link(&class_dev->kobj, "device");
+	sysfs_remove_link(&class_dev->kobj, "device");
 }
 
 static int class_device_driver_link(struct class_device * class_dev)
@@ -169,8 +168,7 @@
 
 static void class_device_driver_unlink(struct class_device * class_dev)
 {
-	if ((class_dev->dev) && (class_dev->dev->driver))
-		sysfs_remove_link(&class_dev->kobj, "driver");
+	sysfs_remove_link(&class_dev->kobj, "driver");
 }
 
 

Re: [PATCH] Fix removable USB drive oops

[James Bottomley]

On Wed, 2004-03-17 at 19:58, Greg KH wrote:
> Bah, this was my fault, sorry.
> 
> Here's a patch that should fix this, and prevent you from needing this
> patch.  Can you verify this?

No, no.  It was Martin's fault: He used a NULL class to indicate that we
had no transport class.

The registration code in SCSI looks like:

	if (sdev->transport_classdev.class) {
		error = class_device_add(&sdev->transport_classdev);

i.e. only add the device if we actually have a transport class.  The
oops was caused because the unregister code was unconditional (i.e.
unregister a device we never registered in the first place).  The patch
(among other things) makes it conditional on the
sdev->transport_classdev.class like the registration.

James