Re: [linux-usb-devel] Bug creating USB endpoints in 2.6.20.x (kernel bug 8198)

[Tejun Heo <htejun@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Chris Rankin wrote:
> --- Tejun Heo <htejun@gmail.com> wrote:
>> So, we can fix the problem Chris is seeing by breaking module unload (by
>> allowing it to unload too early).  It doesn't sound too hot but module
>> unloading race is much less likely than sysfs node deletion/open race.
> 
> Yikes! Just temporary breakage, I hope :-)!! The only modules I unload on a regular basis these
> days are things like "microcode", which the init scripts take care of as part of the boot-up
> process.

Okay, here's a half-assed fix.  With this patch applied, if you try to
unload a module while you're opening it's dev attribute, kernel will
oops later when the file is accessed or closed later but it should fix
the bug winecfg triggers.  I really dunno how to fix this the right way
in the stable kernel.  Better ideas?  Anyone?

-- 
tejun

[-- Attachment #2: sysfs-race-fix.patch --]
[-- Type: text/x-patch, Size: 2413 bytes --]

---
 drivers/base/core.c    |   33 ++++++++-------------------------
 include/linux/device.h |    1 -
 2 files changed, 8 insertions(+), 26 deletions(-)

Index: tree0/drivers/base/core.c
===================================================================
--- tree0.orig/drivers/base/core.c
+++ tree0/drivers/base/core.c
@@ -287,6 +287,8 @@ static ssize_t show_dev(struct device *d
 	return print_dev_t(buf, dev->devt);
 }
 
+static struct device_attribute devt_attr = __ATTR(dev, S_IRUGO, show_dev, NULL );
+
 /*
  *	devices_subsys - structure to be registered with kobject core.
  */
@@ -490,24 +492,9 @@ int device_add(struct device *dev)
 		goto attrError;
 
 	if (MAJOR(dev->devt)) {
-		struct device_attribute *attr;
-		attr = kzalloc(sizeof(*attr), GFP_KERNEL);
-		if (!attr) {
-			error = -ENOMEM;
-			goto ueventattrError;
-		}
-		attr->attr.name = "dev";
-		attr->attr.mode = S_IRUGO;
-		if (dev->driver)
-			attr->attr.owner = dev->driver->owner;
-		attr->show = show_dev;
-		error = device_create_file(dev, attr);
-		if (error) {
-			kfree(attr);
+		error = device_create_file(dev, &devt_attr);
+		if (error)
 			goto ueventattrError;
-		}
-
-		dev->devt_attr = attr;
 	}
 
 	if (dev->class) {
@@ -568,10 +555,8 @@ int device_add(struct device *dev)
  GroupError:
  	device_remove_attrs(dev);
  AttrsError:
-	if (dev->devt_attr) {
-		device_remove_file(dev, dev->devt_attr);
-		kfree(dev->devt_attr);
-	}
+	if (MAJOR(dev->devt))
+		device_remove_file(dev, &devt_attr);
  ueventattrError:
 	device_remove_file(dev, &dev->uevent_attr);
  attrError:
@@ -650,10 +635,8 @@ void device_del(struct device * dev)
 
 	if (parent)
 		klist_del(&dev->knode_parent);
-	if (dev->devt_attr) {
-		device_remove_file(dev, dev->devt_attr);
-		kfree(dev->devt_attr);
-	}
+	if (MAJOR(dev->devt))
+		device_remove_file(dev, &devt_attr);
 	if (dev->class) {
 		sysfs_remove_link(&dev->kobj, "subsystem");
 		/* If this is not a "fake" compatible device, remove the
Index: tree0/include/linux/device.h
===================================================================
--- tree0.orig/include/linux/device.h
+++ tree0/include/linux/device.h
@@ -357,7 +357,6 @@ struct device {
 	char	bus_id[BUS_ID_SIZE];	/* position on parent bus */
 	unsigned		is_registered:1;
 	struct device_attribute uevent_attr;
-	struct device_attribute *devt_attr;
 
 	struct semaphore	sem;	/* semaphore to synchronize calls to
 					 * its driver.