Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

PM: Do not hold dpm_list_mtx while disabling/enabling nonboot CPUs

We shouldn't hold dpm_list_mtx while executing
[disable|enable]_nonboot_cpus(), because theoretically this may lead
to a deadlock as shown by the following example (provided by Johannes
Berg):

CPU 3 CPU 2 CPU 1
suspend/hibernate
something:
rtnl_lock() device_pm_lock()
-> mutex_lock(&dpm_list_mtx)

mutex_lock(&dpm_list_mtx)

linkwatch_work
-> rtnl_lock()
disable_nonboot_cpus()
-> flush CPU 3 workqueue

Fortunately, device drivers are supposed to stop any activities that
might lead to the registration of new device objects way before
disable_nonboot_cpus() is called, so it shouldn't be necessary to
hold dpm_list_mtx over the entire late part of device suspend and
early part of device resume.

Thus, during the late suspend and the early resume of devices acquire
dpm_list_mtx only when dpm_list is going to be traversed and release
it right after that.

This patch is reported to fix the regressions tracked as
http://bugzilla.kernel.org/show_bug.cgi?id=13245.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Miles Lane <miles.lane@gmail.com>
Tested-by: Ming Lei <tom.leiming@gmail.com>

+8 -26
+4
drivers/base/power/main.c
··· 357 357 { 358 358 struct device *dev; 359 359 360 + mutex_lock(&dpm_list_mtx); 360 361 list_for_each_entry(dev, &dpm_list, power.entry) 361 362 if (dev->power.status > DPM_OFF) { 362 363 int error; ··· 367 366 if (error) 368 367 pm_dev_err(dev, state, " early", error); 369 368 } 369 + mutex_unlock(&dpm_list_mtx); 370 370 } 371 371 372 372 /** ··· 616 614 int error = 0; 617 615 618 616 suspend_device_irqs(); 617 + mutex_lock(&dpm_list_mtx); 619 618 list_for_each_entry_reverse(dev, &dpm_list, power.entry) { 620 619 error = suspend_device_noirq(dev, state); 621 620 if (error) { ··· 625 622 } 626 623 dev->power.status = DPM_OFF_IRQ; 627 624 } 625 + mutex_unlock(&dpm_list_mtx); 628 626 if (error) 629 627 device_power_up(resume_event(state)); 630 628 return error;
-2
kernel/kexec.c
··· 1451 1451 error = device_suspend(PMSG_FREEZE); 1452 1452 if (error) 1453 1453 goto Resume_console; 1454 - device_pm_lock(); 1455 1454 /* At this point, device_suspend() has been called, 1456 1455 * but *not* device_power_down(). We *must* 1457 1456 * device_power_down() now. Otherwise, drivers for ··· 1488 1489 enable_nonboot_cpus(); 1489 1490 device_power_up(PMSG_RESTORE); 1490 1491 Resume_devices: 1491 - device_pm_unlock(); 1492 1492 device_resume(PMSG_RESTORE); 1493 1493 Resume_console: 1494 1494 resume_console();
+3 -18
kernel/power/disk.c
··· 215 215 if (error) 216 216 return error; 217 217 218 - device_pm_lock(); 219 - 220 218 /* At this point, device_suspend() has been called, but *not* 221 219 * device_power_down(). We *must* call device_power_down() now. 222 220 * Otherwise, drivers for some devices (e.g. interrupt controllers) ··· 225 227 if (error) { 226 228 printk(KERN_ERR "PM: Some devices failed to power down, " 227 229 "aborting hibernation\n"); 228 - goto Unlock; 230 + return error; 229 231 } 230 232 231 233 error = platform_pre_snapshot(platform_mode); ··· 277 279 278 280 device_power_up(in_suspend ? 279 281 (error ? PMSG_RECOVER : PMSG_THAW) : PMSG_RESTORE); 280 - 281 - Unlock: 282 - device_pm_unlock(); 283 282 284 283 return error; 285 284 } ··· 339 344 { 340 345 int error; 341 346 342 - device_pm_lock(); 343 - 344 347 error = device_power_down(PMSG_QUIESCE); 345 348 if (error) { 346 349 printk(KERN_ERR "PM: Some devices failed to power down, " 347 350 "aborting resume\n"); 348 - goto Unlock; 351 + return error; 349 352 } 350 353 351 354 error = platform_pre_restore(platform_mode); ··· 395 402 platform_restore_cleanup(platform_mode); 396 403 397 404 device_power_up(PMSG_RECOVER); 398 - 399 - Unlock: 400 - device_pm_unlock(); 401 405 402 406 return error; 403 407 } ··· 454 464 goto Resume_devices; 455 465 } 456 466 457 - device_pm_lock(); 458 - 459 467 error = device_power_down(PMSG_HIBERNATE); 460 468 if (error) 461 - goto Unlock; 469 + goto Resume_devices; 462 470 463 471 error = hibernation_ops->prepare(); 464 472 if (error) ··· 480 492 hibernation_ops->finish(); 481 493 482 494 device_power_up(PMSG_RESTORE); 483 - 484 - Unlock: 485 - device_pm_unlock(); 486 495 487 496 Resume_devices: 488 497 entering_platform_hibernation = false;
+1 -6
kernel/power/main.c
··· 289 289 { 290 290 int error; 291 291 292 - device_pm_lock(); 293 - 294 292 if (suspend_ops->prepare) { 295 293 error = suspend_ops->prepare(); 296 294 if (error) 297 - goto Done; 295 + return error; 298 296 } 299 297 300 298 error = device_power_down(PMSG_SUSPEND); ··· 340 342 Platfrom_finish: 341 343 if (suspend_ops->finish) 342 344 suspend_ops->finish(); 343 - 344 - Done: 345 - device_pm_unlock(); 346 345 347 346 return error; 348 347 }