diff options
author | Rafael J. Wysocki <rjw@sisk.pl> | 2008-02-24 18:35:04 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2008-03-04 17:47:05 -0500 |
commit | 7a8d37a37380e2b1500592d40b7ec384dbebe7a0 (patch) | |
tree | 9a23b48e49ffc3303e5998f20498c994da2788c1 /drivers/base/power | |
parent | a4573c488dd531c6e2d308ce8a7413c4a2646207 (diff) |
PM: Do not acquire device semaphores upfront during suspend
Remove the code that acquires all device semaphores from the suspend
code path as it causes multiple problems to appear (most notably,
http://bugzilla.kernel.org/show_bug.cgi?id=10030) and revert the
change introduced by commit 4145ed6dc597a9bea5f6ae8c574653b2de10620f
depending on the code being removed.
Remove pm_sleep_lock()/pm_sleep_unlock() from device_add() to avoid
the issue reported at http://bugzilla.kernel.org/show_bug.cgi?id=9874.
It should fix the regreesions reported at:
http://bugzilla.kernel.org/show_bug.cgi?id=9874
http://bugzilla.kernel.org/show_bug.cgi?id=10030
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Diffstat (limited to 'drivers/base/power')
-rw-r--r-- | drivers/base/power/main.c | 106 |
1 files changed, 16 insertions, 90 deletions
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index ee9d1c8db0d6..b0c16f6fc186 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c | |||
@@ -48,7 +48,6 @@ | |||
48 | */ | 48 | */ |
49 | 49 | ||
50 | LIST_HEAD(dpm_active); | 50 | LIST_HEAD(dpm_active); |
51 | static LIST_HEAD(dpm_locked); | ||
52 | static LIST_HEAD(dpm_off); | 51 | static LIST_HEAD(dpm_off); |
53 | static LIST_HEAD(dpm_off_irq); | 52 | static LIST_HEAD(dpm_off_irq); |
54 | static LIST_HEAD(dpm_destroy); | 53 | static LIST_HEAD(dpm_destroy); |
@@ -81,28 +80,6 @@ void device_pm_add(struct device *dev) | |||
81 | */ | 80 | */ |
82 | void device_pm_remove(struct device *dev) | 81 | void device_pm_remove(struct device *dev) |
83 | { | 82 | { |
84 | /* | ||
85 | * If this function is called during a suspend, it will be blocked, | ||
86 | * because we're holding the device's semaphore at that time, which may | ||
87 | * lead to a deadlock. In that case we want to print a warning. | ||
88 | * However, it may also be called by unregister_dropped_devices() with | ||
89 | * the device's semaphore released, in which case the warning should | ||
90 | * not be printed. | ||
91 | */ | ||
92 | if (down_trylock(&dev->sem)) { | ||
93 | if (down_read_trylock(&pm_sleep_rwsem)) { | ||
94 | /* No suspend in progress, wait on dev->sem */ | ||
95 | down(&dev->sem); | ||
96 | up_read(&pm_sleep_rwsem); | ||
97 | } else { | ||
98 | /* Suspend in progress, we may deadlock */ | ||
99 | dev_warn(dev, "Suspicious %s during suspend\n", | ||
100 | __FUNCTION__); | ||
101 | dump_stack(); | ||
102 | /* The user has been warned ... */ | ||
103 | down(&dev->sem); | ||
104 | } | ||
105 | } | ||
106 | pr_debug("PM: Removing info for %s:%s\n", | 83 | pr_debug("PM: Removing info for %s:%s\n", |
107 | dev->bus ? dev->bus->name : "No Bus", | 84 | dev->bus ? dev->bus->name : "No Bus", |
108 | kobject_name(&dev->kobj)); | 85 | kobject_name(&dev->kobj)); |
@@ -110,7 +87,6 @@ void device_pm_remove(struct device *dev) | |||
110 | dpm_sysfs_remove(dev); | 87 | dpm_sysfs_remove(dev); |
111 | list_del_init(&dev->power.entry); | 88 | list_del_init(&dev->power.entry); |
112 | mutex_unlock(&dpm_list_mtx); | 89 | mutex_unlock(&dpm_list_mtx); |
113 | up(&dev->sem); | ||
114 | } | 90 | } |
115 | 91 | ||
116 | /** | 92 | /** |
@@ -230,6 +206,8 @@ static int resume_device(struct device *dev) | |||
230 | TRACE_DEVICE(dev); | 206 | TRACE_DEVICE(dev); |
231 | TRACE_RESUME(0); | 207 | TRACE_RESUME(0); |
232 | 208 | ||
209 | down(&dev->sem); | ||
210 | |||
233 | if (dev->bus && dev->bus->resume) { | 211 | if (dev->bus && dev->bus->resume) { |
234 | dev_dbg(dev,"resuming\n"); | 212 | dev_dbg(dev,"resuming\n"); |
235 | error = dev->bus->resume(dev); | 213 | error = dev->bus->resume(dev); |
@@ -245,6 +223,8 @@ static int resume_device(struct device *dev) | |||
245 | error = dev->class->resume(dev); | 223 | error = dev->class->resume(dev); |
246 | } | 224 | } |
247 | 225 | ||
226 | up(&dev->sem); | ||
227 | |||
248 | TRACE_RESUME(error); | 228 | TRACE_RESUME(error); |
249 | return error; | 229 | return error; |
250 | } | 230 | } |
@@ -266,7 +246,7 @@ static void dpm_resume(void) | |||
266 | struct list_head *entry = dpm_off.next; | 246 | struct list_head *entry = dpm_off.next; |
267 | struct device *dev = to_device(entry); | 247 | struct device *dev = to_device(entry); |
268 | 248 | ||
269 | list_move_tail(entry, &dpm_locked); | 249 | list_move_tail(entry, &dpm_active); |
270 | mutex_unlock(&dpm_list_mtx); | 250 | mutex_unlock(&dpm_list_mtx); |
271 | resume_device(dev); | 251 | resume_device(dev); |
272 | mutex_lock(&dpm_list_mtx); | 252 | mutex_lock(&dpm_list_mtx); |
@@ -275,25 +255,6 @@ static void dpm_resume(void) | |||
275 | } | 255 | } |
276 | 256 | ||
277 | /** | 257 | /** |
278 | * unlock_all_devices - Release each device's semaphore | ||
279 | * | ||
280 | * Go through the dpm_off list. Put each device on the dpm_active | ||
281 | * list and unlock it. | ||
282 | */ | ||
283 | static void unlock_all_devices(void) | ||
284 | { | ||
285 | mutex_lock(&dpm_list_mtx); | ||
286 | while (!list_empty(&dpm_locked)) { | ||
287 | struct list_head *entry = dpm_locked.prev; | ||
288 | struct device *dev = to_device(entry); | ||
289 | |||
290 | list_move(entry, &dpm_active); | ||
291 | up(&dev->sem); | ||
292 | } | ||
293 | mutex_unlock(&dpm_list_mtx); | ||
294 | } | ||
295 | |||
296 | /** | ||
297 | * unregister_dropped_devices - Unregister devices scheduled for removal | 258 | * unregister_dropped_devices - Unregister devices scheduled for removal |
298 | * | 259 | * |
299 | * Unregister all devices on the dpm_destroy list. | 260 | * Unregister all devices on the dpm_destroy list. |
@@ -305,7 +266,6 @@ static void unregister_dropped_devices(void) | |||
305 | struct list_head *entry = dpm_destroy.next; | 266 | struct list_head *entry = dpm_destroy.next; |
306 | struct device *dev = to_device(entry); | 267 | struct device *dev = to_device(entry); |
307 | 268 | ||
308 | up(&dev->sem); | ||
309 | mutex_unlock(&dpm_list_mtx); | 269 | mutex_unlock(&dpm_list_mtx); |
310 | /* This also removes the device from the list */ | 270 | /* This also removes the device from the list */ |
311 | device_unregister(dev); | 271 | device_unregister(dev); |
@@ -324,7 +284,6 @@ void device_resume(void) | |||
324 | { | 284 | { |
325 | might_sleep(); | 285 | might_sleep(); |
326 | dpm_resume(); | 286 | dpm_resume(); |
327 | unlock_all_devices(); | ||
328 | unregister_dropped_devices(); | 287 | unregister_dropped_devices(); |
329 | up_write(&pm_sleep_rwsem); | 288 | up_write(&pm_sleep_rwsem); |
330 | } | 289 | } |
@@ -388,18 +347,15 @@ int device_power_down(pm_message_t state) | |||
388 | struct list_head *entry = dpm_off.prev; | 347 | struct list_head *entry = dpm_off.prev; |
389 | struct device *dev = to_device(entry); | 348 | struct device *dev = to_device(entry); |
390 | 349 | ||
391 | list_del_init(&dev->power.entry); | ||
392 | error = suspend_device_late(dev, state); | 350 | error = suspend_device_late(dev, state); |
393 | if (error) { | 351 | if (error) { |
394 | printk(KERN_ERR "Could not power down device %s: " | 352 | printk(KERN_ERR "Could not power down device %s: " |
395 | "error %d\n", | 353 | "error %d\n", |
396 | kobject_name(&dev->kobj), error); | 354 | kobject_name(&dev->kobj), error); |
397 | if (list_empty(&dev->power.entry)) | ||
398 | list_add(&dev->power.entry, &dpm_off); | ||
399 | break; | 355 | break; |
400 | } | 356 | } |
401 | if (list_empty(&dev->power.entry)) | 357 | if (!list_empty(&dev->power.entry)) |
402 | list_add(&dev->power.entry, &dpm_off_irq); | 358 | list_move(&dev->power.entry, &dpm_off_irq); |
403 | } | 359 | } |
404 | 360 | ||
405 | if (!error) | 361 | if (!error) |
@@ -419,6 +375,8 @@ static int suspend_device(struct device *dev, pm_message_t state) | |||
419 | { | 375 | { |
420 | int error = 0; | 376 | int error = 0; |
421 | 377 | ||
378 | down(&dev->sem); | ||
379 | |||
422 | if (dev->power.power_state.event) { | 380 | if (dev->power.power_state.event) { |
423 | dev_dbg(dev, "PM: suspend %d-->%d\n", | 381 | dev_dbg(dev, "PM: suspend %d-->%d\n", |
424 | dev->power.power_state.event, state.event); | 382 | dev->power.power_state.event, state.event); |
@@ -441,6 +399,9 @@ static int suspend_device(struct device *dev, pm_message_t state) | |||
441 | error = dev->bus->suspend(dev, state); | 399 | error = dev->bus->suspend(dev, state); |
442 | suspend_report_result(dev->bus->suspend, error); | 400 | suspend_report_result(dev->bus->suspend, error); |
443 | } | 401 | } |
402 | |||
403 | up(&dev->sem); | ||
404 | |||
444 | return error; | 405 | return error; |
445 | } | 406 | } |
446 | 407 | ||
@@ -461,11 +422,10 @@ static int dpm_suspend(pm_message_t state) | |||
461 | int error = 0; | 422 | int error = 0; |
462 | 423 | ||
463 | mutex_lock(&dpm_list_mtx); | 424 | mutex_lock(&dpm_list_mtx); |
464 | while (!list_empty(&dpm_locked)) { | 425 | while (!list_empty(&dpm_active)) { |
465 | struct list_head *entry = dpm_locked.prev; | 426 | struct list_head *entry = dpm_active.prev; |
466 | struct device *dev = to_device(entry); | 427 | struct device *dev = to_device(entry); |
467 | 428 | ||
468 | list_del_init(&dev->power.entry); | ||
469 | mutex_unlock(&dpm_list_mtx); | 429 | mutex_unlock(&dpm_list_mtx); |
470 | error = suspend_device(dev, state); | 430 | error = suspend_device(dev, state); |
471 | if (error) { | 431 | if (error) { |
@@ -476,14 +436,11 @@ static int dpm_suspend(pm_message_t state) | |||
476 | (error == -EAGAIN ? | 436 | (error == -EAGAIN ? |
477 | " (please convert to suspend_late)" : | 437 | " (please convert to suspend_late)" : |
478 | "")); | 438 | "")); |
479 | mutex_lock(&dpm_list_mtx); | ||
480 | if (list_empty(&dev->power.entry)) | ||
481 | list_add(&dev->power.entry, &dpm_locked); | ||
482 | break; | 439 | break; |
483 | } | 440 | } |
484 | mutex_lock(&dpm_list_mtx); | 441 | mutex_lock(&dpm_list_mtx); |
485 | if (list_empty(&dev->power.entry)) | 442 | if (!list_empty(&dev->power.entry)) |
486 | list_add(&dev->power.entry, &dpm_off); | 443 | list_move(&dev->power.entry, &dpm_off); |
487 | } | 444 | } |
488 | mutex_unlock(&dpm_list_mtx); | 445 | mutex_unlock(&dpm_list_mtx); |
489 | 446 | ||
@@ -491,36 +448,6 @@ static int dpm_suspend(pm_message_t state) | |||
491 | } | 448 | } |
492 | 449 | ||
493 | /** | 450 | /** |
494 | * lock_all_devices - Acquire every device's semaphore | ||
495 | * | ||
496 | * Go through the dpm_active list. Carefully lock each device's | ||
497 | * semaphore and put it in on the dpm_locked list. | ||
498 | */ | ||
499 | static void lock_all_devices(void) | ||
500 | { | ||
501 | mutex_lock(&dpm_list_mtx); | ||
502 | while (!list_empty(&dpm_active)) { | ||
503 | struct list_head *entry = dpm_active.next; | ||
504 | struct device *dev = to_device(entry); | ||
505 | |||
506 | /* Required locking order is dev->sem first, | ||
507 | * then dpm_list_mutex. Hence this awkward code. | ||
508 | */ | ||
509 | get_device(dev); | ||
510 | mutex_unlock(&dpm_list_mtx); | ||
511 | down(&dev->sem); | ||
512 | mutex_lock(&dpm_list_mtx); | ||
513 | |||
514 | if (list_empty(entry)) | ||
515 | up(&dev->sem); /* Device was removed */ | ||
516 | else | ||
517 | list_move_tail(entry, &dpm_locked); | ||
518 | put_device(dev); | ||
519 | } | ||
520 | mutex_unlock(&dpm_list_mtx); | ||
521 | } | ||
522 | |||
523 | /** | ||
524 | * device_suspend - Save state and stop all devices in system. | 451 | * device_suspend - Save state and stop all devices in system. |
525 | * @state: new power management state | 452 | * @state: new power management state |
526 | * | 453 | * |
@@ -533,7 +460,6 @@ int device_suspend(pm_message_t state) | |||
533 | 460 | ||
534 | might_sleep(); | 461 | might_sleep(); |
535 | down_write(&pm_sleep_rwsem); | 462 | down_write(&pm_sleep_rwsem); |
536 | lock_all_devices(); | ||
537 | error = dpm_suspend(state); | 463 | error = dpm_suspend(state); |
538 | if (error) | 464 | if (error) |
539 | device_resume(); | 465 | device_resume(); |