From 07e4a3e27fe414980ddc85a358e5a56abc48b363 Mon Sep 17 00:00:00 2001 From: "mochel@digitalimplant.org" Date: Mon, 21 Mar 2005 10:52:54 -0800 Subject: [PATCH] Move device/driver code to drivers/base/dd.c This relocates the driver binding/unbinding code to drivers/base/dd.c. This is done for two reasons: One, it's not code related to the bus_type itself; it uses some from that, some from devices, and some from drivers. And Two, it will make it easier to do some of the upcoming lock removal on that code.. Signed-off-by: Patrick Mochel Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 drivers/base/dd.c (limited to 'drivers/base/dd.c') diff --git a/drivers/base/dd.c b/drivers/base/dd.c new file mode 100644 index 000000000000..b709b1e0cb2a --- /dev/null +++ b/drivers/base/dd.c @@ -0,0 +1,200 @@ +/* + * drivers/base/dd.c - The core device/driver interactions. + * + * This file contains the (sometimes tricky) code that controls the + * interactions between devices and drivers, which primarily includes + * driver binding and unbinding. + * + * All of this code used to exist in drivers/base/bus.c, but was + * relocated to here in the name of compartmentalization (since it wasn't + * strictly code just for the 'struct bus_type'. + * + * Copyright (c) 2002-5 Patrick Mochel + * Copyright (c) 2002-3 Open Source Development Labs + * + * This file is released under the GPLv2 + */ + +#include +#include + +#include "base.h" +#include "power/power.h" + +#define to_drv(node) container_of(node, struct device_driver, kobj.entry) + + +/** + * device_bind_driver - bind a driver to one device. + * @dev: device. + * + * Allow manual attachment of a driver to a device. + * Caller must have already set @dev->driver. + * + * Note that this does not modify the bus reference count + * nor take the bus's rwsem. Please verify those are accounted + * for before calling this. (It is ok to call with no other effort + * from a driver's probe() method.) + */ +void device_bind_driver(struct device * dev) +{ + pr_debug("bound device '%s' to driver '%s'\n", + dev->bus_id, dev->driver->name); + list_add_tail(&dev->driver_list, &dev->driver->devices); + sysfs_create_link(&dev->driver->kobj, &dev->kobj, + kobject_name(&dev->kobj)); + sysfs_create_link(&dev->kobj, &dev->driver->kobj, "driver"); +} + +/** + * driver_probe_device - attempt to bind device & driver. + * @drv: driver. + * @dev: device. + * + * First, we call the bus's match function, if one present, which + * should compare the device IDs the driver supports with the + * device IDs of the device. Note we don't do this ourselves + * because we don't know the format of the ID structures, nor what + * is to be considered a match and what is not. + * + * If we find a match, we call @drv->probe(@dev) if it exists, and + * call device_bind_driver() above. + */ +int driver_probe_device(struct device_driver * drv, struct device * dev) +{ + int error = 0; + + if (drv->bus->match && !drv->bus->match(dev, drv)) + return -ENODEV; + + down(&dev->sem); + dev->driver = drv; + if (drv->probe) { + error = drv->probe(dev); + if (error) { + dev->driver = NULL; + up(&dev->sem); + return error; + } + } + up(&dev->sem); + device_bind_driver(dev); + return 0; +} + +/** + * device_attach - try to attach device to a driver. + * @dev: device. + * + * Walk the list of drivers that the bus has and call + * driver_probe_device() for each pair. If a compatible + * pair is found, break out and return. + */ +int device_attach(struct device * dev) +{ + struct bus_type * bus = dev->bus; + struct list_head * entry; + int error; + + if (dev->driver) { + device_bind_driver(dev); + return 1; + } + + if (bus->match) { + list_for_each(entry, &bus->drivers.list) { + struct device_driver * drv = to_drv(entry); + error = driver_probe_device(drv, dev); + if (!error) + /* success, driver matched */ + return 1; + if (error != -ENODEV && error != -ENXIO) + /* driver matched but the probe failed */ + printk(KERN_WARNING + "%s: probe of %s failed with error %d\n", + drv->name, dev->bus_id, error); + } + } + + return 0; +} + +/** + * driver_attach - try to bind driver to devices. + * @drv: driver. + * + * Walk the list of devices that the bus has on it and try to + * match the driver with each one. If driver_probe_device() + * returns 0 and the @dev->driver is set, we've found a + * compatible pair. + * + * Note that we ignore the -ENODEV error from driver_probe_device(), + * since it's perfectly valid for a driver not to bind to any devices. + */ +void driver_attach(struct device_driver * drv) +{ + struct bus_type * bus = drv->bus; + struct list_head * entry; + int error; + + if (!bus->match) + return; + + list_for_each(entry, &bus->devices.list) { + struct device * dev = container_of(entry, struct device, bus_list); + if (!dev->driver) { + error = driver_probe_device(drv, dev); + if (error && (error != -ENODEV)) + /* driver matched but the probe failed */ + printk(KERN_WARNING + "%s: probe of %s failed with error %d\n", + drv->name, dev->bus_id, error); + } + } +} + +/** + * device_release_driver - manually detach device from driver. + * @dev: device. + * + * Manually detach device from driver. + * Note that this is called without incrementing the bus + * reference count nor taking the bus's rwsem. Be sure that + * those are accounted for before calling this function. + */ +void device_release_driver(struct device * dev) +{ + struct device_driver * drv; + + down(&dev->sem); + drv = dev->driver; + if (drv) { + sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); + sysfs_remove_link(&dev->kobj, "driver"); + list_del_init(&dev->driver_list); + if (drv->remove) + drv->remove(dev); + dev->driver = NULL; + } + up(&dev->sem); +} + +/** + * driver_detach - detach driver from all devices it controls. + * @drv: driver. + */ +void driver_detach(struct device_driver * drv) +{ + while (!list_empty(&drv->devices)) { + struct device * dev = container_of(drv->devices.next, struct device, driver_list); + device_release_driver(dev); + } +} + + +EXPORT_SYMBOL_GPL(driver_probe_device); +EXPORT_SYMBOL_GPL(device_bind_driver); +EXPORT_SYMBOL_GPL(device_release_driver); +EXPORT_SYMBOL_GPL(device_attach); +EXPORT_SYMBOL_GPL(driver_attach); + -- cgit v1.2.3 From 94e7b1c5ff2055571703e38b059afffe17658432 Mon Sep 17 00:00:00 2001 From: "mochel@digitalimplant.org" Date: Mon, 21 Mar 2005 12:25:36 -0800 Subject: [PATCH] Add a klist to struct device_driver for the devices bound to it. - Use it in driver_for_each_device() instead of the regular list_head and stop using the bus's rwsem for protection. - Use driver_for_each_device() in driver_detach() so we don't deadlock on the bus's rwsem. - Remove ->devices. - Move klist access and sysfs link access out from under device's semaphore, since they're synchronized through other means. Signed-off-by: Patrick Mochel Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 2 +- drivers/base/dd.c | 34 +++++++++++++++++++--------------- drivers/base/driver.c | 27 +++++++++++++++------------ include/linux/device.h | 3 ++- 4 files changed, 37 insertions(+), 29 deletions(-) (limited to 'drivers/base/dd.c') diff --git a/drivers/base/core.c b/drivers/base/core.c index 93440824b800..bc5bec61a01a 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -209,8 +209,8 @@ void device_initialize(struct device *dev) kobject_init(&dev->kobj); INIT_LIST_HEAD(&dev->node); INIT_LIST_HEAD(&dev->children); - INIT_LIST_HEAD(&dev->driver_list); INIT_LIST_HEAD(&dev->bus_list); + INIT_LIST_HEAD(&dev->driver_list); INIT_LIST_HEAD(&dev->dma_pools); init_MUTEX(&dev->sem); } diff --git a/drivers/base/dd.c b/drivers/base/dd.c index b709b1e0cb2a..47cbb5641235 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -40,7 +40,7 @@ void device_bind_driver(struct device * dev) { pr_debug("bound device '%s' to driver '%s'\n", dev->bus_id, dev->driver->name); - list_add_tail(&dev->driver_list, &dev->driver->devices); + klist_add_tail(&dev->driver->klist_devices, &dev->knode_driver); sysfs_create_link(&dev->driver->kobj, &dev->kobj, kobject_name(&dev->kobj)); sysfs_create_link(&dev->kobj, &dev->driver->kobj, "driver"); @@ -164,31 +164,35 @@ void driver_attach(struct device_driver * drv) */ void device_release_driver(struct device * dev) { - struct device_driver * drv; + struct device_driver * drv = dev->driver; + + if (!drv) + return; + + sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); + sysfs_remove_link(&dev->kobj, "driver"); + klist_remove(&dev->knode_driver); down(&dev->sem); - drv = dev->driver; - if (drv) { - sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); - sysfs_remove_link(&dev->kobj, "driver"); - list_del_init(&dev->driver_list); - if (drv->remove) - drv->remove(dev); - dev->driver = NULL; - } + if (drv->remove) + drv->remove(dev); + dev->driver = NULL; up(&dev->sem); } +static int __remove_driver(struct device * dev, void * unused) +{ + device_release_driver(dev); + return 0; +} + /** * driver_detach - detach driver from all devices it controls. * @drv: driver. */ void driver_detach(struct device_driver * drv) { - while (!list_empty(&drv->devices)) { - struct device * dev = container_of(drv->devices.next, struct device, driver_list); - device_release_driver(dev); - } + driver_for_each_device(drv, NULL, NULL, __remove_driver); } diff --git a/drivers/base/driver.c b/drivers/base/driver.c index 484fed1985aa..34bd38aa7eb8 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -19,6 +19,12 @@ #define to_drv(obj) container_of(obj, struct device_driver, kobj) +static struct device * next_device(struct klist_iter * i) +{ + struct klist_node * n = klist_next(i); + return n ? container_of(n, struct device, knode_driver) : NULL; +} + /** * driver_for_each_device - Iterator for devices bound to a driver. * @drv: Driver we're iterating. @@ -32,21 +38,18 @@ int driver_for_each_device(struct device_driver * drv, struct device * start, void * data, int (*fn)(struct device *, void *)) { - struct list_head * head; + struct klist_iter i; struct device * dev; int error = 0; - down_read(&drv->bus->subsys.rwsem); - head = &drv->devices; - dev = list_prepare_entry(start, head, driver_list); - list_for_each_entry_continue(dev, head, driver_list) { - get_device(dev); + if (!drv) + return -EINVAL; + + klist_iter_init_node(&drv->klist_devices, &i, + start ? &start->knode_driver : NULL); + while ((dev = next_device(&i)) && !error) error = fn(dev, data); - put_device(dev); - if (error) - break; - } - up_read(&drv->bus->subsys.rwsem); + klist_iter_exit(&i); return error; } @@ -120,7 +123,7 @@ void put_driver(struct device_driver * drv) */ int driver_register(struct device_driver * drv) { - INIT_LIST_HEAD(&drv->devices); + klist_init(&drv->klist_devices); init_completion(&drv->unloaded); return bus_add_driver(drv); } diff --git a/include/linux/device.h b/include/linux/device.h index ea9ab33dfe71..96c71b59fdef 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -105,7 +105,7 @@ struct device_driver { struct completion unloaded; struct kobject kobj; - struct list_head devices; + struct klist klist_devices; struct klist_node knode_bus; struct module * owner; @@ -266,6 +266,7 @@ struct device { struct list_head bus_list; /* node in bus's list */ struct list_head driver_list; struct list_head children; + struct klist_node knode_driver; struct klist_node knode_bus; struct device * parent; -- cgit v1.2.3 From 2287c322b61fced7e0c326a1a9606aa73147e3df Mon Sep 17 00:00:00 2001 From: "mochel@digitalimplant.org" Date: Thu, 24 Mar 2005 10:50:24 -0800 Subject: [PATCH] Use bus_for_each_{dev,drv} for driver binding. - Now possible, since the lists are locked using the klist lock and not the global rwsem. Signed-off-by: Patrick Mochel Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 72 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 33 deletions(-) (limited to 'drivers/base/dd.c') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 47cbb5641235..85042ada8a5b 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -82,6 +82,28 @@ int driver_probe_device(struct device_driver * drv, struct device * dev) return 0; } +static int __device_attach(struct device_driver * drv, void * data) +{ + struct device * dev = data; + int error; + + error = driver_probe_device(drv, dev); + + if (error == -ENODEV && error == -ENXIO) { + /* Driver matched, but didn't support device + * or device not found. + * Not an error; keep going. + */ + error = 0; + } else { + /* driver matched but the probe failed */ + printk(KERN_WARNING + "%s: probe of %s failed with error %d\n", + drv->name, dev->bus_id, error); + } + return 0; +} + /** * device_attach - try to attach device to a driver. * @dev: device. @@ -92,30 +114,31 @@ int driver_probe_device(struct device_driver * drv, struct device * dev) */ int device_attach(struct device * dev) { - struct bus_type * bus = dev->bus; - struct list_head * entry; - int error; - if (dev->driver) { device_bind_driver(dev); return 1; } - if (bus->match) { - list_for_each(entry, &bus->drivers.list) { - struct device_driver * drv = to_drv(entry); - error = driver_probe_device(drv, dev); - if (!error) - /* success, driver matched */ - return 1; - if (error != -ENODEV && error != -ENXIO) + return bus_for_each_drv(dev->bus, NULL, dev, __device_attach); +} + +static int __driver_attach(struct device * dev, void * data) +{ + struct device_driver * drv = data; + int error = 0; + + if (!dev->driver) { + error = driver_probe_device(drv, dev); + if (error) { + if (error != -ENODEV) { /* driver matched but the probe failed */ printk(KERN_WARNING - "%s: probe of %s failed with error %d\n", - drv->name, dev->bus_id, error); + "%s: probe of %s failed with error %d\n", + drv->name, dev->bus_id, error); + } else + error = 0; } } - return 0; } @@ -133,24 +156,7 @@ int device_attach(struct device * dev) */ void driver_attach(struct device_driver * drv) { - struct bus_type * bus = drv->bus; - struct list_head * entry; - int error; - - if (!bus->match) - return; - - list_for_each(entry, &bus->devices.list) { - struct device * dev = container_of(entry, struct device, bus_list); - if (!dev->driver) { - error = driver_probe_device(drv, dev); - if (error && (error != -ENODEV)) - /* driver matched but the probe failed */ - printk(KERN_WARNING - "%s: probe of %s failed with error %d\n", - drv->name, dev->bus_id, error); - } - } + bus_for_each_dev(drv->bus, NULL, drv, __driver_attach); } /** -- cgit v1.2.3 From 0956af53afea290c5676c75249fc2c180d831375 Mon Sep 17 00:00:00 2001 From: "mochel@digitalimplant.org" Date: Thu, 24 Mar 2005 18:58:45 -0800 Subject: [PATCH] Call klist_del() instead of klist_remove(). - Can't wait on removing the current item in the list (the positive refcount *because* we are using it causes it to deadlock). Signed-off-by: Patrick Mochel Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base/dd.c') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 85042ada8a5b..159e0623a681 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -177,7 +177,7 @@ void device_release_driver(struct device * dev) sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); sysfs_remove_link(&dev->kobj, "driver"); - klist_remove(&dev->knode_driver); + klist_del(&dev->knode_driver); down(&dev->sem); if (drv->remove) -- cgit v1.2.3 From b86c1df1f98d16c999423a3907eb40a9423f481e Mon Sep 17 00:00:00 2001 From: "gregkh@suse.de" Date: Thu, 31 Mar 2005 12:53:00 -0800 Subject: [PATCH] Driver core: Fix up the driver and device iterators to be quieter Also stops looping over the lists when a match is found. Signed-off-by: Greg Kroah-Hartman name, dev->bus_id, error); + if (error) { + if ((error == -ENODEV) || (error == -ENXIO)) { + /* Driver matched, but didn't support device + * or device not found. + * Not an error; keep going. + */ + error = 0; + } else { + /* driver matched but the probe failed */ + printk(KERN_WARNING + "%s: probe of %s failed with error %d\n", + drv->name, dev->bus_id, error); + } + return error; } - return 0; + /* stop looking, this device is attached */ + return 1; } /** @@ -137,7 +140,10 @@ static int __driver_attach(struct device * dev, void * data) drv->name, dev->bus_id, error); } else error = 0; + return error; } + /* stop looking, this driver is attached */ + return 1; } return 0; } -- cgit v1.2.3 From 0d3e5a2e39b6ba2974e9e7c2a429018c45de8e76 Mon Sep 17 00:00:00 2001 From: Patrick Mochel Date: Tue, 5 Apr 2005 23:46:33 -0700 Subject: [PATCH] Driver Core: fix bk-driver-core kills ppc64 There's no check to see if the device is already bound to a driver, which could do bad things. The first thing to go wrong is that it will try to match a driver with a device already bound to one. In some cases (it appears with USB with drivers/usb/core/usb.c::usb_match_id()), some drivers will match a device based on the class type, so it would be common (especially for HID devices) to match a device that is already bound. The fun comes when ->probe() is called, it fails, then driver_probe_device() does this: dev->driver = NULL; Later on, that pointer could be be dereferenced without checking and cause hell to break loose. This problem could be nasty. It's very hardware dependent, since some devices could have a different set of matching qualifiers than others. Now, I don't quite see exactly where/how you were getting that crash. You're dereferencing bad memory, but I'm not sure which pointer was bad and where it came from, but it could have come from a couple of different places. The patch below will hopefully fix it all up for you. It's against 2.6.12-rc2-mm1, and does the following: - Move logic to driver_probe_device() and comments uncommon returns: 1 - If device is bound 0 - If device not bound, and no error error - If there was an error. - Move locking to caller of that function, since we want to lock a device for the entire time we're trying to bind it to a driver (to prevent against a driver being loaded at the same time). - Update __device_attach() and __driver_attach() to do that locking. - Check if device is already bound in __driver_attach() - Update the converse device_release_driver() so it locks the device around all of the operations. - Mark driver_probe_device() as static and remove export. It's an internal function, it should stay that way, and there are no other callers. If there is ever a need to export it, we can audit it as necessary. Signed-off-by: Andrew Morton --- drivers/base/dd.c | 142 ++++++++++++++++++++++++++----------------------- include/linux/device.h | 1 - 2 files changed, 74 insertions(+), 69 deletions(-) (limited to 'drivers/base/dd.c') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index dd2a8a79c121..8510918109e0 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -35,6 +35,8 @@ * nor take the bus's rwsem. Please verify those are accounted * for before calling this. (It is ok to call with no other effort * from a driver's probe() method.) + * + * This function must be called with @dev->sem held. */ void device_bind_driver(struct device * dev) { @@ -57,54 +59,56 @@ void device_bind_driver(struct device * dev) * because we don't know the format of the ID structures, nor what * is to be considered a match and what is not. * - * If we find a match, we call @drv->probe(@dev) if it exists, and - * call device_bind_driver() above. + * + * This function returns 1 if a match is found, an error if one + * occurs (that is not -ENODEV or -ENXIO), and 0 otherwise. + * + * This function must be called with @dev->sem held. */ -int driver_probe_device(struct device_driver * drv, struct device * dev) +static int driver_probe_device(struct device_driver * drv, struct device * dev) { - int error = 0; + int ret = 0; if (drv->bus->match && !drv->bus->match(dev, drv)) - return -ENODEV; + goto Done; - down(&dev->sem); + pr_debug("%s: Matched Device %s with Driver %s\n", + drv->bus->name, dev->bus_id, drv->name); dev->driver = drv; if (drv->probe) { - error = drv->probe(dev); - if (error) { + ret = drv->probe(dev); + if (ret) { dev->driver = NULL; - up(&dev->sem); - return error; + goto ProbeFailed; } } - up(&dev->sem); device_bind_driver(dev); - return 0; + ret = 1; + pr_debug("%s: Bound Device %s to Driver %s\n", + drv->bus->name, dev->bus_id, drv->name); + goto Done; + + ProbeFailed: + if (ret == -ENODEV || ret == -ENXIO) { + /* Driver matched, but didn't support device + * or device not found. + * Not an error; keep going. + */ + ret = 0; + } else { + /* driver matched but the probe failed */ + printk(KERN_WARNING + "%s: probe of %s failed with error %d\n", + drv->name, dev->bus_id, ret); + } + Done: + return ret; } static int __device_attach(struct device_driver * drv, void * data) { struct device * dev = data; - int error; - - error = driver_probe_device(drv, dev); - if (error) { - if ((error == -ENODEV) || (error == -ENXIO)) { - /* Driver matched, but didn't support device - * or device not found. - * Not an error; keep going. - */ - error = 0; - } else { - /* driver matched but the probe failed */ - printk(KERN_WARNING - "%s: probe of %s failed with error %d\n", - drv->name, dev->bus_id, error); - } - return error; - } - /* stop looking, this device is attached */ - return 1; + return driver_probe_device(drv, dev); } /** @@ -114,37 +118,43 @@ static int __device_attach(struct device_driver * drv, void * data) * Walk the list of drivers that the bus has and call * driver_probe_device() for each pair. If a compatible * pair is found, break out and return. + * + * Returns 1 if the device was bound to a driver; 0 otherwise. */ int device_attach(struct device * dev) { + int ret = 0; + + down(&dev->sem); if (dev->driver) { device_bind_driver(dev); - return 1; - } - - return bus_for_each_drv(dev->bus, NULL, dev, __device_attach); + ret = 1; + } else + ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach); + up(&dev->sem); + return ret; } static int __driver_attach(struct device * dev, void * data) { struct device_driver * drv = data; - int error = 0; - - if (!dev->driver) { - error = driver_probe_device(drv, dev); - if (error) { - if (error != -ENODEV) { - /* driver matched but the probe failed */ - printk(KERN_WARNING - "%s: probe of %s failed with error %d\n", - drv->name, dev->bus_id, error); - } else - error = 0; - return error; - } - /* stop looking, this driver is attached */ - return 1; - } + + /* + * Lock device and try to bind to it. We drop the error + * here and always return 0, because we need to keep trying + * to bind to devices and some drivers will return an error + * simply if it didn't support the device. + * + * driver_probe_device() will spit a warning if there + * is an error. + */ + + down(&dev->sem); + if (!dev->driver) + driver_probe_device(drv, dev); + up(&dev->sem); + + return 0; } @@ -156,9 +166,6 @@ static int __driver_attach(struct device * dev, void * data) * match the driver with each one. If driver_probe_device() * returns 0 and the @dev->driver is set, we've found a * compatible pair. - * - * Note that we ignore the -ENODEV error from driver_probe_device(), - * since it's perfectly valid for a driver not to bind to any devices. */ void driver_attach(struct device_driver * drv) { @@ -176,19 +183,19 @@ void driver_attach(struct device_driver * drv) */ void device_release_driver(struct device * dev) { - struct device_driver * drv = dev->driver; - - if (!drv) - return; - - sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); - sysfs_remove_link(&dev->kobj, "driver"); - klist_del(&dev->knode_driver); + struct device_driver * drv; down(&dev->sem); - if (drv->remove) - drv->remove(dev); - dev->driver = NULL; + if (dev->driver) { + drv = dev->driver; + sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); + sysfs_remove_link(&dev->kobj, "driver"); + klist_del(&dev->knode_driver); + + if (drv->remove) + drv->remove(dev); + dev->driver = NULL; + } up(&dev->sem); } @@ -208,7 +215,6 @@ void driver_detach(struct device_driver * drv) } -EXPORT_SYMBOL_GPL(driver_probe_device); EXPORT_SYMBOL_GPL(device_bind_driver); EXPORT_SYMBOL_GPL(device_release_driver); EXPORT_SYMBOL_GPL(device_attach); diff --git a/include/linux/device.h b/include/linux/device.h index 43249260cd1c..91aac349b9a7 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -325,7 +325,6 @@ extern int device_for_each_child(struct device *, void *, * Manual binding of a device to driver. See drivers/base/bus.c * for information on use. */ -extern int driver_probe_device(struct device_driver * drv, struct device * dev); extern void device_bind_driver(struct device * dev); extern void device_release_driver(struct device * dev); extern int device_attach(struct device * dev); -- cgit v1.2.3 From c95a6b057b108c2b7add35cba1354f9af921349e Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 6 May 2005 15:38:33 -0400 Subject: [PATCH] driver core: Fix races in driver_detach() This patch is intended for your "driver" tree. It fixes several subtle races in driver_detach() and device_release_driver() in the driver-model core. The major change is to use klist_remove() rather than klist_del() when taking a device off its driver's list. There's no other way to guarantee that the list pointers will be updated before some other driver binds to the device. For this to work driver_detach() can't use a klist iterator, so the loop over the devices must be written out in full. In addition the patch protects against the possibility that, when a driver and a device are unregistered at the same time, one may be unloaded from memory before the other is finished using it. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/base/dd.c | 51 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 13 deletions(-) (limited to 'drivers/base/dd.c') diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 8510918109e0..eab2030c506d 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -177,41 +177,66 @@ void driver_attach(struct device_driver * drv) * @dev: device. * * Manually detach device from driver. - * Note that this is called without incrementing the bus - * reference count nor taking the bus's rwsem. Be sure that - * those are accounted for before calling this function. + * + * __device_release_driver() must be called with @dev->sem held. */ -void device_release_driver(struct device * dev) + +static void __device_release_driver(struct device * dev) { struct device_driver * drv; - down(&dev->sem); - if (dev->driver) { - drv = dev->driver; + drv = dev->driver; + if (drv) { + get_driver(drv); sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); sysfs_remove_link(&dev->kobj, "driver"); - klist_del(&dev->knode_driver); + klist_remove(&dev->knode_driver); if (drv->remove) drv->remove(dev); dev->driver = NULL; + put_driver(drv); } - up(&dev->sem); } -static int __remove_driver(struct device * dev, void * unused) +void device_release_driver(struct device * dev) { - device_release_driver(dev); - return 0; + /* + * If anyone calls device_release_driver() recursively from + * within their ->remove callback for the same device, they + * will deadlock right here. + */ + down(&dev->sem); + __device_release_driver(dev); + up(&dev->sem); } + /** * driver_detach - detach driver from all devices it controls. * @drv: driver. */ void driver_detach(struct device_driver * drv) { - driver_for_each_device(drv, NULL, NULL, __remove_driver); + struct device * dev; + + for (;;) { + spin_lock_irq(&drv->klist_devices.k_lock); + if (list_empty(&drv->klist_devices.k_list)) { + spin_unlock_irq(&drv->klist_devices.k_lock); + break; + } + dev = list_entry(drv->klist_devices.k_list.prev, + struct device, knode_driver.n_node); + get_device(dev); + spin_unlock_irq(&drv->klist_devices.k_lock); + + down(&dev->sem); + if (dev->driver == drv) + __device_release_driver(dev); + up(&dev->sem); + put_device(dev); + } } -- cgit v1.2.3 From ca2b94ba12f3c36fd3d6ed9d38b3798d4dad0d8b Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Wed, 18 May 2005 10:42:23 +0200 Subject: [PATCH] driver core: fix error handling in bus_add_device The error handling in bus_add_device() and device_attach() is simply non-existing. This patch propagates any error from device_attach to the upper layers to allow for a proper recovery. From: Hannes Reinecke Signed-off-by: Greg Kroah-Hartman --- drivers/base/bus.c | 13 ++++++++----- drivers/base/dd.c | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-) (limited to 'drivers/base/dd.c') diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 4eb19525e064..43722af90bdd 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -270,11 +270,14 @@ int bus_add_device(struct device * dev) if (bus) { pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id); - device_attach(dev); + error = device_attach(dev); klist_add_tail(&bus->klist_devices, &dev->knode_bus); - device_add_attrs(bus, dev); - sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id); - sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, "bus"); + if (error >= 0) + error = device_add_attrs(bus, dev); + if (!error) { + sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id); + sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, "bus"); + } } return error; } @@ -394,7 +397,7 @@ static int bus_rescan_devices_helper(struct device *dev, void *data) { int *count = data; - if (!dev->driver && device_attach(dev)) + if (!dev->driver && (device_attach(dev) > 0)) (*count)++; return 0; diff --git a/drivers/base/dd.c b/drivers/base/dd.c index eab2030c506d..6db3a789c54f 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -119,7 +119,8 @@ static int __device_attach(struct device_driver * drv, void * data) * driver_probe_device() for each pair. If a compatible * pair is found, break out and return. * - * Returns 1 if the device was bound to a driver; 0 otherwise. + * Returns 1 if the device was bound to a driver; + * 0 if no matching device was found; error code otherwise. */ int device_attach(struct device * dev) { -- cgit v1.2.3