From 2e646ed096850667d67db5d86070dc2b6c136677 Mon Sep 17 00:00:00 2001 From: Patrick Mochel Date: Mon, 3 Mar 2003 00:48:38 -0600 Subject: [PATCH] driver model: fix device interfaces. - Remove struct intf_data. It was silly, and gave us an unbalanced interface. Instead, when removing a device_interface, all of a class's devices are iterated over, and intf->remove_device() is called for each. (Instead of the device's list of interfaces it belongs to). Interfaces are expected to tell if they support the device being removed. - Remove struct device::intf_list. - Protect classes' device list access with devclass_sem, which is taken when a device or interface is added or removed. This allows access to the list w/o taking the class's lock, which allows subordinate objects to be registered (e.g. the interface itself, or objects the interfaces create for a device). - This should fix a deadlock when devices are added to interfaces, which calls intf->add_device(), then interface_add_data(), which call kobject_register() and hangs, since it tries to take the class's lock again. --- drivers/base/base.h | 1 + drivers/base/class.c | 80 +++++++++++++++------------ drivers/base/core.c | 1 - drivers/base/intf.c | 119 ++++++++++++----------------------------- include/linux/device.h | 26 +-------- 5 files changed, 83 insertions(+), 144 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index ab31ff74207c9..a13680120c479 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -1,6 +1,7 @@ #undef DEBUG extern struct semaphore device_sem; +extern struct semaphore devclass_sem; extern int bus_add_device(struct device * dev); extern void bus_remove_device(struct device * dev); diff --git a/drivers/base/class.c b/drivers/base/class.c index 52c42ad287bb3..cf8260017f235 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -13,6 +13,8 @@ #define to_class_attr(_attr) container_of(_attr,struct devclass_attribute,attr) #define to_class(obj) container_of(obj,struct device_class,subsys.kset.kobj) +DECLARE_MUTEX(devclass_sem); + static ssize_t devclass_attr_show(struct kobject * kobj, struct attribute * attr, char * buf) { @@ -163,29 +165,34 @@ int devclass_add_device(struct device * dev) struct device_class * cls; int error = 0; + down(&devclass_sem); if (dev->driver) { cls = get_devclass(dev->driver->devclass); - if (cls) { - down_write(&cls->subsys.rwsem); - pr_debug("device class %s: adding device %s\n", - cls->name,dev->name); - if (cls->add_device) - error = cls->add_device(dev); - if (!error) { - enum_device(cls,dev); - interface_add_dev(dev); - } - - list_add_tail(&dev->class_list,&cls->devices.list); - - /* notify userspace (call /sbin/hotplug) */ - class_hotplug (dev, "add"); - - up_write(&cls->subsys.rwsem); - if (error) - put_devclass(cls); + + if (!cls) + goto Done; + + pr_debug("device class %s: adding device %s\n", + cls->name,dev->name); + if (cls->add_device) + error = cls->add_device(dev); + if (error) { + put_devclass(cls); + goto Done; } + + down_write(&cls->subsys.rwsem); + enum_device(cls,dev); + list_add_tail(&dev->class_list,&cls->devices.list); + /* notify userspace (call /sbin/hotplug) */ + class_hotplug (dev, "add"); + + up_write(&cls->subsys.rwsem); + + interface_add_dev(dev); } + Done: + up(&devclass_sem); return error; } @@ -193,26 +200,33 @@ void devclass_remove_device(struct device * dev) { struct device_class * cls; + down(&devclass_sem); if (dev->driver) { cls = dev->driver->devclass; - if (cls) { - down_write(&cls->subsys.rwsem); - pr_debug("device class %s: removing device %s\n", - cls->name,dev->name); - interface_remove_dev(dev); - unenum_device(cls,dev); + if (!cls) + goto Done; - list_del(&dev->class_list); + interface_remove_dev(dev); - /* notify userspace (call /sbin/hotplug) */ - class_hotplug (dev, "remove"); + down_write(&cls->subsys.rwsem); + pr_debug("device class %s: removing device %s\n", + cls->name,dev->name); - if (cls->remove_device) - cls->remove_device(dev); - up_write(&cls->subsys.rwsem); - put_devclass(cls); - } + unenum_device(cls,dev); + + list_del(&dev->class_list); + + /* notify userspace (call /sbin/hotplug) */ + class_hotplug (dev, "remove"); + + up_write(&cls->subsys.rwsem); + + if (cls->remove_device) + cls->remove_device(dev); + put_devclass(cls); } + Done: + up(&devclass_sem); } struct device_class * get_devclass(struct device_class * cls) diff --git a/drivers/base/core.c b/drivers/base/core.c index 40fe758731baf..e37311995422c 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -143,7 +143,6 @@ void device_initialize(struct device *dev) INIT_LIST_HEAD(&dev->driver_list); INIT_LIST_HEAD(&dev->bus_list); INIT_LIST_HEAD(&dev->class_list); - INIT_LIST_HEAD(&dev->intf_list); } /** diff --git a/drivers/base/intf.c b/drivers/base/intf.c index 809547aed60d5..f1422dae312d3 100644 --- a/drivers/base/intf.c +++ b/drivers/base/intf.c @@ -12,80 +12,31 @@ #define to_intf(node) container_of(node,struct device_interface,kset.kobj.entry) -#define to_data(e) container_of(e,struct intf_data,kobj.entry) +#define to_dev(d) container_of(d,struct device,class_list) /** * intf_dev_link - create sysfs symlink for interface. - * @data: interface data descriptor. + * @intf: interface. + * @dev: device. * * Create a symlink 'phys' in the interface's directory to */ -static int intf_dev_link(struct intf_data * data) +static int intf_dev_link(struct device_interface * intf, struct device * dev) { - char name[16]; - snprintf(name,16,"%d",data->intf_num); - return sysfs_create_link(&data->intf->kset.kobj,&data->dev->kobj,name); + return sysfs_create_link(&intf->kset.kobj,&dev->kobj,dev->bus_id); } /** * intf_dev_unlink - remove symlink for interface. - * @intf: interface data descriptor. - * - */ - -static void intf_dev_unlink(struct intf_data * data) -{ - char name[16]; - snprintf(name,16,"%d",data->intf_num); - sysfs_remove_link(&data->intf->kset.kobj,name); -} - - -/** - * interface_add_data - attach data descriptor - * @data: interface data descriptor. - * - * This attaches the per-instance interface object to the - * interface (by registering its kobject) and the device - * itself (by inserting it into the device's list). - * - * Note that there is no explicit protection done in this - * function. This should be called from the interface's - * add_device() method, which is called under the protection - * of the class's rwsem. - */ - -int interface_add_data(struct intf_data * data) -{ - struct device_interface * intf = data->intf; - - if (intf) { - data->intf_num = intf->devnum++; - data->kobj.kset = &intf->kset; - kobject_register(&data->kobj); - - list_add_tail(&data->dev_entry,&data->dev->intf_list); - return intf_dev_link(data); - } - return -EINVAL; -} - - -/** - * interface_remove_data - detach data descriptor. - * @data: interface data descriptor. + * @intf: interface. + * @dev: device. * - * This detaches the per-instance data descriptor by removing - * it from the device's list and unregistering the kobject from - * the subsystem. */ -void interface_remove_data(struct intf_data * data) +static void intf_dev_unlink(struct device_interface * intf, struct device * dev) { - intf_dev_unlink(data); - list_del_init(&data->dev_entry); - kobject_unregister(&data->kobj); + sysfs_remove_link(&intf->kset.kobj,dev->bus_id); } @@ -103,33 +54,28 @@ static int add(struct device_interface * intf, struct device * dev) { int error = 0; - if (intf->add_device) - error = intf->add_device(dev); + if (intf->add_device) { + if (!(error = intf->add_device(dev))) + intf_dev_link(intf,dev); + } pr_debug(" -> %s (%d)\n",dev->bus_id,error); return error; } /** * del - detach device from interface. - * @data: interface data descriptor. - * - * Another simple helper. Remove the data descriptor from - * the device and the interface, then call the interface's - * remove_device() method. + * @intf: interface. + * @dev: device. */ -static void del(struct intf_data * data) +static void del(struct device_interface * intf, struct device * dev) { - struct device_interface * intf = data->intf; - pr_debug(" -> %s ",intf->name); - interface_remove_data(data); if (intf->remove_device) - intf->remove_device(data); + intf->remove_device(dev); + intf_dev_unlink(intf,dev); } -#define to_dev(entry) container_of(entry,struct device,class_list) - /** * add_intf - add class's devices to interface. @@ -145,10 +91,8 @@ static void add_intf(struct device_interface * intf) struct device_class * cls = intf->devclass; struct list_head * entry; - down_write(&cls->subsys.rwsem); list_for_each(entry,&cls->devices.list) add(intf,to_dev(entry)); - up_write(&cls->subsys.rwsem); } /** @@ -164,6 +108,7 @@ int interface_register(struct device_interface * intf) { struct device_class * cls = get_devclass(intf->devclass); + down(&devclass_sem); if (cls) { pr_debug("register interface '%s' with class '%s'\n", intf->name,cls->name); @@ -173,6 +118,7 @@ int interface_register(struct device_interface * intf) kset_register(&intf->kset); add_intf(intf); } + up(&devclass_sem); return 0; } @@ -188,14 +134,13 @@ int interface_register(struct device_interface * intf) static void del_intf(struct device_interface * intf) { + struct device_class * cls = intf->devclass; struct list_head * entry; - down_write(&intf->devclass->subsys.rwsem); - list_for_each(entry,&intf->kset.list) { - struct intf_data * data = to_data(entry); - del(data); + list_for_each(entry,&cls->devices.list) { + struct device * dev = to_dev(entry); + del(intf,dev); } - up_write(&intf->devclass->subsys.rwsem); } /** @@ -210,6 +155,8 @@ static void del_intf(struct device_interface * intf) void interface_unregister(struct device_interface * intf) { struct device_class * cls = intf->devclass; + + down(&devclass_sem); if (cls) { pr_debug("unregistering interface '%s' from class '%s'\n", intf->name,cls->name); @@ -217,6 +164,7 @@ void interface_unregister(struct device_interface * intf) kset_unregister(&intf->kset); put_devclass(cls); } + up(&devclass_sem); } @@ -255,20 +203,21 @@ int interface_add_dev(struct device * dev) * This is another helper for the class driver core, and called * when the device is being removed from the class. * - * We iterate over the list of interface data descriptors attached - * to the device, and call del() [above] for each. Again, the - * class's rwsem is assumed to be held during this. + * We iterate over the list of the class's devices and call del() + * [above] for each. Again, the class's rwsem is _not_ held, but + * the devclass_sem is (see class.c). */ void interface_remove_dev(struct device * dev) { struct list_head * entry, * next; + struct device_class * cls = dev->driver->devclass; pr_debug("interfaces: removing device %s\n",dev->name); - list_for_each_safe(entry,next,&dev->intf_list) { - struct intf_data * intf_data = to_data(entry); - del(intf_data); + list_for_each_safe(entry,next,&cls->subsys.kset.list) { + struct device_interface * intf = to_intf(entry); + del(intf,dev); } } diff --git a/include/linux/device.h b/include/linux/device.h index b6b3385f31dce..f059058eb3ac9 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -207,8 +207,6 @@ extern void devclass_remove_file(struct device_class *, struct devclass_attribut * it supports the device. */ -struct intf_data; - struct device_interface { char * name; struct device_class * devclass; @@ -217,41 +215,19 @@ struct device_interface { u32 devnum; int (*add_device) (struct device *); - int (*remove_device) (struct intf_data *); + int (*remove_device) (struct device *); }; extern int interface_register(struct device_interface *); extern void interface_unregister(struct device_interface *); -/* - * intf_data - per-device data for an interface - * Each interface typically has a per-device data structure - * that it allocates. It should embed one of these structures - * in that structure and call interface_add_data() to add it - * to the device's list. - * That will also enumerate the device within the interface - * and create a driverfs symlink for it. - */ -struct intf_data { - struct device_interface * intf; - struct device * dev; - u32 intf_num; - struct list_head dev_entry; - struct kobject kobj; -}; - -extern int interface_add_data(struct intf_data *); - - - struct device { struct list_head node; /* node in sibling list */ struct list_head bus_list; /* node in bus's list */ struct list_head class_list; struct list_head driver_list; struct list_head children; - struct list_head intf_list; struct device * parent; struct kobject kobj; -- 2.53.0