Commit 789e39b0 authored by Patrick Mochel's avatar Patrick Mochel

Update device model locking

Change all iterators of devices to:
- use list_for_each
- check return of get_device_locked
- don't break until we hold the lock if we get an error

When a device's reference count hits 0, remove it from all lists, including
bus and driver lists.

Between the iterator algorithm and the guaranteed removal from the lists,
there should never be a device in a list with a reference count of 0. 
So, whenever we're iterating over the lists, we'll always have a valid 
device. We don't decrement the refcount until the next iteration of the
loop, so we're also guaranteed to get the correct next item in the list.
parent 266c24ad
...@@ -16,6 +16,9 @@ ...@@ -16,6 +16,9 @@
static LIST_HEAD(bus_driver_list); static LIST_HEAD(bus_driver_list);
#define to_dev(node) container_of(node,struct device,bus_list)
#define to_drv(node) container_of(node,struct device_driver,bus_list)
/** /**
* bus_for_each_dev - walk list of devices and do something to each * bus_for_each_dev - walk list of devices and do something to each
* @bus: bus in question * @bus: bus in question
...@@ -26,42 +29,41 @@ static LIST_HEAD(bus_driver_list); ...@@ -26,42 +29,41 @@ static LIST_HEAD(bus_driver_list);
* counting on devices as we touch each one. * counting on devices as we touch each one.
* *
* Algorithm: * Algorithm:
* Take the bus lock and get the first node in the list. We increment * Take device_lock and get the first node in the list.
* the reference count and unlock the bus. If we have a device from a * Try and increment the reference count on it. If we can't, it's in the
* previous iteration, we decrement the reference count. * process of being removed, but that process hasn't acquired device_lock.
* After we call the callback, we get the next node in the list and loop. * It's still in the list, so we grab the next node and try that one.
* At the end, if @dev is not null, we still have it pinned, so we need * We drop the lock to call the callback.
* to let it go. * We can't decrement the reference count yet, because we need the next
* node in the list. So, we set @prev to point to the current device.
* On the next go-round, we decrement the reference count on @prev, so if
* it's being removed, it won't affect us.
*/ */
int bus_for_each_dev(struct bus_type * bus, void * data, int bus_for_each_dev(struct bus_type * bus, void * data,
int (*callback)(struct device * dev, void * data)) int (*callback)(struct device * dev, void * data))
{ {
struct device * next;
struct device * dev = NULL;
struct list_head * node; struct list_head * node;
struct device * prev = NULL;
int error = 0; int error = 0;
get_bus(bus); get_bus(bus);
spin_lock(&device_lock); spin_lock(&device_lock);
node = bus->devices.next; list_for_each(node,&bus->devices) {
while (node != &bus->devices) { struct device * dev = get_device_locked(to_dev(node));
next = list_entry(node,struct device,bus_list); if (dev) {
get_device_locked(next); spin_unlock(&device_lock);
spin_unlock(&device_lock); error = callback(dev,data);
if (prev)
if (dev) put_device(prev);
put_device(dev); prev = dev;
dev = next; spin_lock(&device_lock);
if ((error = callback(dev,data))) { if (error)
put_device(dev); break;
break;
} }
spin_lock(&device_lock);
node = dev->bus_list.next;
} }
spin_unlock(&device_lock); spin_unlock(&device_lock);
if (dev) if (prev)
put_device(dev); put_device(prev);
put_bus(bus); put_bus(bus);
return error; return error;
} }
...@@ -69,34 +71,30 @@ int bus_for_each_dev(struct bus_type * bus, void * data, ...@@ -69,34 +71,30 @@ int bus_for_each_dev(struct bus_type * bus, void * data,
int bus_for_each_drv(struct bus_type * bus, void * data, int bus_for_each_drv(struct bus_type * bus, void * data,
int (*callback)(struct device_driver * drv, void * data)) int (*callback)(struct device_driver * drv, void * data))
{ {
struct device_driver * next;
struct device_driver * drv = NULL;
struct list_head * node; struct list_head * node;
struct device_driver * prev = NULL;
int error = 0; int error = 0;
/* pin bus in memory */ /* pin bus in memory */
get_bus(bus); get_bus(bus);
spin_lock(&device_lock); spin_lock(&device_lock);
node = bus->drivers.next; list_for_each(node,&bus->drivers) {
while (node != &bus->drivers) { struct device_driver * drv = get_driver(to_drv(node));
next = list_entry(node,struct device_driver,bus_list); if (drv) {
get_driver(next); spin_unlock(&device_lock);
spin_unlock(&device_lock); error = callback(drv,data);
if (prev)
if (drv) put_driver(prev);
put_driver(drv); prev = drv;
drv = next; spin_lock(&device_lock);
if ((error = callback(drv,data))) { if (error)
put_driver(drv); break;
break;
} }
spin_lock(&device_lock);
node = drv->bus_list.next;
} }
spin_unlock(&device_lock); spin_unlock(&device_lock);
if (drv) if (prev)
put_driver(drv); put_driver(prev);
put_bus(bus); put_bus(bus);
return error; return error;
} }
...@@ -134,9 +132,6 @@ void bus_remove_device(struct device * dev) ...@@ -134,9 +132,6 @@ void bus_remove_device(struct device * dev)
{ {
if (dev->bus) { if (dev->bus) {
device_remove_symlink(&dev->bus->device_dir,dev->bus_id); device_remove_symlink(&dev->bus->device_dir,dev->bus_id);
spin_lock(&device_lock);
list_del_init(&dev->bus_list);
spin_unlock(&device_lock);
put_bus(dev->bus); put_bus(dev->bus);
} }
} }
......
...@@ -25,6 +25,8 @@ int (*platform_notify_remove)(struct device * dev) = NULL; ...@@ -25,6 +25,8 @@ int (*platform_notify_remove)(struct device * dev) = NULL;
spinlock_t device_lock = SPIN_LOCK_UNLOCKED; spinlock_t device_lock = SPIN_LOCK_UNLOCKED;
#define to_dev(node) container_of(node,struct device,driver_list)
/** /**
* found_match - do actual binding of device to driver * found_match - do actual binding of device to driver
...@@ -101,14 +103,10 @@ static void device_detach(struct device * dev) ...@@ -101,14 +103,10 @@ static void device_detach(struct device * dev)
struct device_driver * drv; struct device_driver * drv;
if (dev->driver) { if (dev->driver) {
lock_device(dev); spin_lock(&device_lock);
drv = dev->driver; drv = dev->driver;
dev->driver = NULL; dev->driver = NULL;
unlock_device(dev); spin_unlock(&device_lock);
write_lock(&drv->lock);
list_del_init(&dev->driver_list);
write_unlock(&drv->lock);
/* detach from driver */ /* detach from driver */
if (drv->remove) if (drv->remove)
...@@ -136,45 +134,37 @@ int driver_attach(struct device_driver * drv) ...@@ -136,45 +134,37 @@ int driver_attach(struct device_driver * drv)
static int do_driver_detach(struct device * dev, struct device_driver * drv) static int do_driver_detach(struct device * dev, struct device_driver * drv)
{ {
lock_device(dev); spin_lock(&device_lock);
if (dev->driver == drv) { if (dev->driver == drv) {
dev->driver = NULL; dev->driver = NULL;
unlock_device(dev); spin_unlock(&device_lock);
if (drv->remove) if (drv->remove)
drv->remove(dev); drv->remove(dev);
} else } else
unlock_device(dev); spin_unlock(&device_lock);
return 0; return 0;
} }
void driver_detach(struct device_driver * drv) void driver_detach(struct device_driver * drv)
{ {
struct device * next;
struct device * dev = NULL;
struct list_head * node; struct list_head * node;
int error = 0; struct device * prev = NULL;
spin_lock(&device_lock); spin_lock(&device_lock);
node = drv->devices.next; list_for_each(node,&drv->devices) {
while (node != &drv->devices) { struct device * dev = get_device_locked(to_dev(node));
next = list_entry(node,struct device,driver_list); if (dev) {
get_device_locked(next); list_del_init(node);
list_del_init(&next->driver_list); spin_unlock(&device_lock);
spin_unlock(&device_lock); do_driver_detach(dev,drv);
if (prev)
if (dev) put_device(prev);
put_device(dev); prev = dev;
dev = next; spin_lock(&device_lock);
if ((error = do_driver_detach(dev,drv))) {
put_device(dev);
break;
} }
spin_lock(&device_lock);
node = drv->devices.next;
} }
spin_unlock(&device_lock); spin_unlock(&device_lock);
if (dev)
put_device(dev);
} }
/** /**
...@@ -245,7 +235,7 @@ int device_register(struct device *dev) ...@@ -245,7 +235,7 @@ int device_register(struct device *dev)
struct device * get_device_locked(struct device * dev) struct device * get_device_locked(struct device * dev)
{ {
struct device * ret = dev; struct device * ret = dev;
if (dev && atomic_read(&dev->refcount)) if (dev && atomic_read(&dev->refcount) > 0)
atomic_inc(&dev->refcount); atomic_inc(&dev->refcount);
else else
ret = NULL; ret = NULL;
...@@ -271,6 +261,8 @@ void put_device(struct device * dev) ...@@ -271,6 +261,8 @@ void put_device(struct device * dev)
return; return;
list_del_init(&dev->node); list_del_init(&dev->node);
list_del_init(&dev->g_list); list_del_init(&dev->g_list);
list_del_init(&dev->bus_list);
list_del_init(&dev->driver_list);
spin_unlock(&device_lock); spin_unlock(&device_lock);
pr_debug("DEV: Unregistering device. ID = '%s', name = '%s'\n", pr_debug("DEV: Unregistering device. ID = '%s', name = '%s'\n",
......
...@@ -10,35 +10,31 @@ ...@@ -10,35 +10,31 @@
#include <linux/errno.h> #include <linux/errno.h>
#include "base.h" #include "base.h"
#define to_dev(node) container_of(node,struct device,driver_list)
int driver_for_each_dev(struct device_driver * drv, void * data, int (*callback)(struct device *, void * )) int driver_for_each_dev(struct device_driver * drv, void * data,
int (*callback)(struct device *, void * ))
{ {
struct device * next;
struct device * dev = NULL;
struct list_head * node; struct list_head * node;
struct device * prev = NULL;
int error = 0; int error = 0;
get_driver(drv); get_driver(drv);
spin_lock(&device_lock); spin_lock(&device_lock);
node = drv->devices.next; list_for_each(node,&drv->devices) {
while (node != &drv->devices) { struct device * dev = get_device_locked(to_dev(node));
next = list_entry(node,struct device,driver_list); if (dev) {
get_device_locked(next); spin_unlock(&device_lock);
spin_unlock(&device_lock); error = callback(dev,data);
if (prev)
if (dev) put_device(prev);
put_device(dev); prev = dev;
dev = next; spin_lock(&device_lock);
if ((error = callback(dev,data))) { if (error)
put_device(dev); break;
break;
} }
spin_lock(&device_lock);
node = dev->driver_list.next;
} }
spin_unlock(&device_lock); spin_unlock(&device_lock);
if (dev)
put_device(dev);
put_driver(drv); put_driver(drv);
return error; return error;
} }
......
...@@ -14,6 +14,8 @@ ...@@ -14,6 +14,8 @@
#include <linux/module.h> #include <linux/module.h>
#include "base.h" #include "base.h"
#define to_dev(node) container_of(node,struct device,g_list)
/** /**
* device_suspend - suspend all devices on the device tree * device_suspend - suspend all devices on the device tree
* @state: state we're entering * @state: state we're entering
...@@ -25,30 +27,26 @@ ...@@ -25,30 +27,26 @@
*/ */
int device_suspend(u32 state, u32 level) int device_suspend(u32 state, u32 level)
{ {
struct device * dev; struct list_head * node;
struct device * prev = &device_root; struct device * prev = NULL;
int error = 0; int error = 0;
printk(KERN_EMERG "Suspending Devices\n"); printk(KERN_EMERG "Suspending Devices\n");
get_device(prev);
spin_lock(&device_lock); spin_lock(&device_lock);
dev = g_list_to_dev(prev->g_list.next); list_for_each(node,&device_root.g_list) {
while(dev != &device_root && !error) { struct device * dev = get_device_locked(dev);
get_device_locked(dev); if (dev) {
spin_unlock(&device_lock); spin_unlock(&device_lock);
put_device(prev); if (dev->driver && dev->driver->suspend)
error = dev->driver->suspend(dev,state,level);
if (dev->driver && dev->driver->suspend) if (prev)
error = dev->driver->suspend(dev,state,level); put_device(prev);
prev = dev;
spin_lock(&device_lock); spin_lock(&device_lock);
prev = dev; }
dev = g_list_to_dev(prev->g_list.next);
} }
spin_unlock(&device_lock); spin_unlock(&device_lock);
put_device(prev);
return error; return error;
} }
...@@ -63,27 +61,23 @@ int device_suspend(u32 state, u32 level) ...@@ -63,27 +61,23 @@ int device_suspend(u32 state, u32 level)
*/ */
void device_resume(u32 level) void device_resume(u32 level)
{ {
struct device * dev; struct list_head * node;
struct device * prev = &device_root; struct device * prev = NULL;
get_device(prev);
spin_lock(&device_lock); spin_lock(&device_lock);
dev = g_list_to_dev(prev->g_list.prev); list_for_each_prev(node,&device_root.g_list) {
while(dev != &device_root) { struct device * dev = get_device_locked(to_dev(node));
get_device_locked(dev); if (dev) {
spin_unlock(&device_lock); spin_unlock(&device_lock);
put_device(prev); if (dev->driver && dev->driver->resume)
dev->driver->resume(dev,level);
if (dev->driver && dev->driver->resume) if (prev)
dev->driver->resume(dev,level); put_device(prev);
prev = dev;
spin_lock(&device_lock); spin_lock(&device_lock);
prev = dev; }
dev = g_list_to_dev(prev->g_list.prev);
} }
spin_unlock(&device_lock); spin_unlock(&device_lock);
put_device(prev);
printk(KERN_EMERG "Devices Resumed\n"); printk(KERN_EMERG "Devices Resumed\n");
} }
...@@ -98,29 +92,25 @@ void device_resume(u32 level) ...@@ -98,29 +92,25 @@ void device_resume(u32 level)
*/ */
void device_shutdown(void) void device_shutdown(void)
{ {
struct device * dev; struct list_head * node;
struct device * prev = &device_root; struct device * prev = NULL;
printk(KERN_EMERG "Shutting down devices\n"); printk(KERN_EMERG "Shutting down devices\n");
get_device(prev);
spin_lock(&device_lock); spin_lock(&device_lock);
dev = g_list_to_dev(prev->g_list.next); list_for_each(node,&device_root.g_list) {
while(dev != &device_root) { struct device * dev = get_device_locked(to_dev(node));
dev = get_device_locked(dev); if (dev) {
spin_unlock(&device_lock); spin_unlock(&device_lock);
put_device(prev); if (dev->driver && dev->driver->remove)
dev->driver->remove(dev);
if (dev->driver && dev->driver->remove) if (prev)
dev->driver->remove(dev); put_device(prev);
prev = dev;
spin_lock(&device_lock); spin_lock(&device_lock);
prev = dev; }
dev = g_list_to_dev(prev->g_list.next);
} }
spin_unlock(&device_lock); spin_unlock(&device_lock);
put_device(prev);
} }
EXPORT_SYMBOL(device_suspend); EXPORT_SYMBOL(device_suspend);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment