[PATCH] driver core: remove unneeded klist methods

From: Alan Stern
Date: Fri Jan 20 2006 - 11:38:04 EST


Greg:

This patch (as641) removes unneeded klist methods from the driver core and
changes a klist_del call to klist_remove in device_del.

The _get and _put methods have no effect, because the klist nodes are
deleted by calling klist_remove, which waits until they are unreferenced
by any klist iterators. Furthermore, the _puts cause problems because
they occur while the iterator is holding a spinlock.

Alan Stern



Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

---

James should check this, since he added these routines in the first place.
The change to device_for_each_child is unfortunate, but also unavoidable
due to the change in device_del. There's still a potentially dangerous
call to klist_del in attribute_container.c, but I'm not familiar enough
with that code to change it.

The SCSI core needs work. The basic problem is that it has to keep a
reference to a SCSI device until the last command for that device
completes, but command completion occurs in an interrupt handler. That
makes it hard to release the last reference. I don't know how this should
be fixed; James will have to work on it.

Index: usb-2.6/drivers/base/core.c
===================================================================
--- usb-2.6.orig/drivers/base/core.c
+++ usb-2.6/drivers/base/core.c
@@ -198,20 +198,6 @@ void device_remove_file(struct device *
}
}

-static void klist_children_get(struct klist_node *n)
-{
- struct device *dev = container_of(n, struct device, knode_parent);
-
- get_device(dev);
-}
-
-static void klist_children_put(struct klist_node *n)
-{
- struct device *dev = container_of(n, struct device, knode_parent);
-
- put_device(dev);
-}
-

/**
* device_initialize - init device structure.
@@ -228,8 +214,7 @@ void device_initialize(struct device *de
{
kobj_set_kset_s(dev, devices_subsys);
kobject_init(&dev->kobj);
- klist_init(&dev->klist_children, klist_children_get,
- klist_children_put);
+ klist_init(&dev->klist_children, NULL, NULL);
INIT_LIST_HEAD(&dev->dma_pools);
init_MUTEX(&dev->sem);
device_init_wakeup(dev, 0);
@@ -365,7 +350,7 @@ void device_del(struct device * dev)
struct device * parent = dev->parent;

if (parent)
- klist_del(&dev->knode_parent);
+ klist_remove(&dev->knode_parent);
device_remove_file(dev, &dev->uevent_attr);

/* Notify the platform of the removal, in case they
@@ -417,17 +402,25 @@ static struct device * next_device(struc
*
* We check the return of @fn each time. If it returns anything
* other than 0, we break out and return that value.
+ *
+ * The code is complicated because @fn may want to call device_del().
+ * Since the iterator holds a reference to the klist_node it is
+ * using, the call would hang. We get around the problem by
+ * keeping the iterator one cycle ahead of @fn.
*/
int device_for_each_child(struct device * parent, void * data,
int (*fn)(struct device *, void *))
{
struct klist_iter i;
- struct device * child;
+ struct device * child, * next_child;
int error = 0;

klist_iter_init(&parent->klist_children, &i);
- while ((child = next_device(&i)) && !error)
+ next_child = next_device(&i);
+ while ((child = next_child) && !error) {
+ next_child = next_device(&i);
error = fn(child, data);
+ }
klist_iter_exit(&i);
return error;
}
Index: usb-2.6/drivers/base/bus.c
===================================================================
--- usb-2.6.orig/drivers/base/bus.c
+++ usb-2.6/drivers/base/bus.c
@@ -599,36 +599,6 @@ static void bus_remove_attrs(struct bus_
}
}

-static void klist_devices_get(struct klist_node *n)
-{
- struct device *dev = container_of(n, struct device, knode_bus);
-
- get_device(dev);
-}
-
-static void klist_devices_put(struct klist_node *n)
-{
- struct device *dev = container_of(n, struct device, knode_bus);
-
- put_device(dev);
-}
-
-static void klist_drivers_get(struct klist_node *n)
-{
- struct device_driver *drv = container_of(n, struct device_driver,
- knode_bus);
-
- get_driver(drv);
-}
-
-static void klist_drivers_put(struct klist_node *n)
-{
- struct device_driver *drv = container_of(n, struct device_driver,
- knode_bus);
-
- put_driver(drv);
-}
-
/**
* bus_register - register a bus with the system.
* @bus: bus.
@@ -663,8 +633,8 @@ int bus_register(struct bus_type * bus)
if (retval)
goto bus_drivers_fail;

- klist_init(&bus->klist_devices, klist_devices_get, klist_devices_put);
- klist_init(&bus->klist_drivers, klist_drivers_get, klist_drivers_put);
+ klist_init(&bus->klist_devices, NULL, NULL);
+ klist_init(&bus->klist_drivers, NULL, NULL);
bus_add_attrs(bus);

pr_debug("bus type '%s' registered\n", bus->name);
Index: usb-2.6/drivers/base/driver.c
===================================================================
--- usb-2.6.orig/drivers/base/driver.c
+++ usb-2.6/drivers/base/driver.c
@@ -143,20 +143,6 @@ void put_driver(struct device_driver * d
kobject_put(&drv->kobj);
}

-static void klist_devices_get(struct klist_node *n)
-{
- struct device *dev = container_of(n, struct device, knode_driver);
-
- get_device(dev);
-}
-
-static void klist_devices_put(struct klist_node *n)
-{
- struct device *dev = container_of(n, struct device, knode_driver);
-
- put_device(dev);
-}
-
/**
* driver_register - register driver with bus
* @drv: driver to register
@@ -176,7 +162,7 @@ int driver_register(struct device_driver
(drv->bus->shutdown && drv->shutdown)) {
printk(KERN_WARNING "Driver '%s' needs updating - please use bus_type methods\n", drv->name);
}
- klist_init(&drv->klist_devices, klist_devices_get, klist_devices_put);
+ klist_init(&drv->klist_devices, NULL, NULL);
init_completion(&drv->unloaded);
return bus_add_driver(drv);
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/