Delayed hotplug events

[Hannes Reinecke <hare@suse.de>]

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

Hi all,

currently we have the problem that the driver core might send out 
hotplug events even though the initialisation of a device has not been 
finished; worse, we might get an "add" followed by an "remove" event if 
the device fails to initialise properly, with the corresponding device 
never appearing in sysfs properly.
This is due to the design of the driver core:
a call to device_register will trigger a hotplug event, but any sysfs 
attributes which are added _after_ device_register() will in fact 
created after the hotplug event has been triggered. For the unbelievers, 
look at drivers/scsi/scsi_sysfs.c:scsi_sysfs_add_sdev().
This leads to all sorts of nasty race conditions and waiting loops in 
the hotplug agents.

To solve this I've wrapped up a patch for delaying / suspending hotplug 
events until they are explicitely enabled again.
As an example of how it should be used I've patched 
drivers/scsi/scsi_sysfs.c:scsi_sysfs_add_sdev(), so that the device 
"add" event will now be triggered only _after_ the device has been 
initialised properly, including all sysfs attributes.

Is this approach feasible or am I completely off kilter here?
And if the latter, how should it be solved properly?

(Note: this is a proof of concept. No devices have been harmed in 
creating this patch. I just wanted to get a general opinion before 
venturing any further.)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux AG				S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

[-- Attachment #2: delay-hotplug-events.patch --]
[-- Type: text/x-patch, Size: 3864 bytes --]

--- ./lib/kobject.c.orig	2004-06-17 09:18:27.000000000 +0200
+++ ./lib/kobject.c	2004-06-17 12:59:55.000000000 +0200
@@ -472,6 +472,53 @@
 		kobject_cleanup(kobj);
 }
 
+static int kobject_hotplug_filter_all(struct kset *kset, struct kobject *kobj)
+{
+	return 0;
+}
+
+/**
+ *      kobject_disable_events - filter out all hotplug events
+ *      @kobj:  object
+ *
+ */
+
+void kobject_disable_events(struct kobject * kobj )
+{
+	struct kset_hotplug_ops *delayed_ops;
+
+	if (kobj && kobj->kset) {
+		down_write(&kobj->kset->subsys->rwsem);
+
+		delayed_ops = kobj->kset->hotplug_ops;
+		kobj->kset->hotplug_ops_stored = delayed_ops;
+
+		delayed_ops->filter = kobject_hotplug_filter_all;
+		kobj->kset->hotplug_ops = delayed_ops;
+
+		up_write(&kobj->kset->subsys->rwsem);
+	}
+}	
+
+/**
+ *      kobject_enable_events - reset hotplug filter to original value
+ *      @kobj:  object
+ *
+ */
+
+void kobject_enable_events(struct kobject * kobj )
+{
+	struct kset_hotplug_ops *delayed_ops;
+
+	if (kobj && kobj->kset && kobj->kset->hotplug_ops_stored) {
+		down_write(&kobj->kset->subsys->rwsem);
+
+		kobj->kset->hotplug_ops = kobj->kset->hotplug_ops_stored;
+		kobj->kset->hotplug_ops_stored = NULL;
+
+		up_write(&kobj->kset->subsys->rwsem);
+	}
+}	
 
 /**
  *	kset_init - initialize a kset for use
@@ -635,6 +682,8 @@
 EXPORT_SYMBOL(kobject_del);
 EXPORT_SYMBOL(kobject_rename);
 EXPORT_SYMBOL(kobject_hotplug);
+EXPORT_SYMBOL(kobject_enable_events);
+EXPORT_SYMBOL(kobject_disable_events);
 
 EXPORT_SYMBOL(kset_register);
 EXPORT_SYMBOL(kset_unregister);
--- ./drivers/base/core.c.orig	2004-06-17 09:03:58.000000000 +0200
+++ ./drivers/base/core.c	2004-06-17 13:06:29.581368704 +0200
@@ -271,6 +271,25 @@
 	return device_add(dev);
 }
 
+void device_suspend_hotplug(struct device *dev)
+{
+	get_device(dev);
+
+	kobject_disable_events(&dev->kobj);
+
+	put_device(dev);
+}
+
+void device_resume_hotplug(struct device *dev)
+{
+	get_device(dev);
+
+	kobject_enable_events(&dev->kobj);
+	
+	kobject_hotplug("add", &dev->kobj);
+
+	put_device(dev);
+}
 
 /**
  *	get_device - increment reference count for device.
@@ -405,3 +424,5 @@
 
 EXPORT_SYMBOL(device_create_file);
 EXPORT_SYMBOL(device_remove_file);
+EXPORT_SYMBOL(device_suspend_hotplug);
+EXPORT_SYMBOL(device_resume_hotplug);
--- ./drivers/scsi/scsi_sysfs.c.orig	2004-06-17 13:00:45.000000000 +0200
+++ ./drivers/scsi/scsi_sysfs.c	2004-06-17 13:03:13.071318353 +0200
@@ -379,6 +379,7 @@
 	if ((error = scsi_device_set_state(sdev, SDEV_RUNNING)) != 0)
 		return error;
 
+	device_suspend_hotplug(&sdev->sdev_gendev);
 	error = device_add(&sdev->sdev_gendev);
 	if (error) {
 		printk(KERN_INFO "error 1\n");
@@ -439,6 +440,8 @@
  		}
  	}
 
+	device_resume_hotplug(&sdev->sdev_gendev);
+
  out:
 	return error;
 
--- ./include/linux/kobject.h.orig	2004-06-17 11:22:29.000000000 +0200
+++ ./include/linux/kobject.h	2004-06-17 12:55:06.000000000 +0200
@@ -47,6 +47,8 @@
 
 extern int kobject_add(struct kobject *);
 extern void kobject_del(struct kobject *);
+extern void kobject_enable_events(struct kobject *kobj);
+extern void kobject_disable_events(struct kobject *kobj);
 
 extern int kobject_rename(struct kobject *, char *new_name);
 
@@ -96,6 +98,7 @@
 	struct list_head	list;
 	struct kobject		kobj;
 	struct kset_hotplug_ops	* hotplug_ops;
+	struct kset_hotplug_ops	* hotplug_ops_stored;
 };
 
 
--- ./include/linux/device.h.orig	2004-06-17 12:14:21.000000000 +0200
+++ ./include/linux/device.h	2004-06-17 12:14:56.000000000 +0200
@@ -332,6 +332,8 @@
 extern void device_bind_driver(struct device * dev);
 extern void device_release_driver(struct device * dev);
 extern void driver_attach(struct device_driver * drv);
+extern void device_suspend_hotplug(struct device *dev);
+extern void device_resume_hotplug(struct device *dev);
 
 
 /* driverfs interface for exporting device attributes */