[lkml]   [2005]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectFYI: device_suspend(...) in kernel_power_off().

    Early in the 2.6.13 process my kexec related patches were introduced
    into the reboot path, and under the rule you touch it you fix it
    it I have been involved in tracking quite a few regressions
    on the reboot path.

    Recently with Benjamin Herrenschmidt's removal of
    device_suppend(PMSG_SUPPEND) from kernel_power_off(),
    it seems the last of the issues went away. I asked
    for confirmation that reintroducing the patch below
    would break systems and I received it.

    The experimental evidence then is that calling
    device_suspend(PMSG_SUSPEND) in kernel_power_off
    when performing an acpi_power_off is actively harmful.

    There as been a fair amount of consensus that calling
    device_suspend(...) in the reboot path was inappropriate now, because
    the device suspend code was too immature. With this latest
    piece of evidence it seems to me that introducing device_suspend(...)
    in kernel_power_off, kernel_halt, kernel_reboot, or kernel_kexec
    can never be appropriate. I am including as many people as
    I can on this so we in our collective wisdom don't forget this

    What lead us to this situation was a real problem, and a desire
    to fix it. Currently linux has a proliferation of interfaces
    to place devices in different states. The reboot notifiers,
    device_suspend(...), device_shutdown+system_state, remove_one,
    module_exit, and probably a few I'm not aware of. Each interface
    introduced by a different author wanting to solve a different problem.
    Just writing this list of interfaces is confusing. The implementation
    task for device driver writers appears even harder as simultaneously
    implementing all of these interfaces correctly seems not to happen.

    The lesson: fixing this problem is heavy lifting, and that
    device_suspend() in the reboot path is not the answer.


    This is the patch that subtly and mysterously broke things.
    > diff --git a/kernel/sys.c b/kernel/sys.c
    > --- a/kernel/sys.c
    > +++ b/kernel/sys.c
    > @@ -404,6 +404,7 @@ void kernel_halt(void)
    > {
    > notifier_call_chain(&reboot_notifier_list, SYS_HALT, NULL);
    > system_state = SYSTEM_HALT;
    > + device_suspend(PMSG_SUSPEND);
    > device_shutdown();
    > printk(KERN_EMERG "System halted.\n");
    > machine_halt();
    > @@ -414,6 +415,7 @@ void kernel_power_off(void)
    > {
    > notifier_call_chain(&reboot_notifier_list, SYS_POWER_OFF, NULL);
    > system_state = SYSTEM_POWER_OFF;
    > + device_suspend(PMSG_SUSPEND);
    > device_shutdown();
    > printk(KERN_EMERG "Power down.\n");
    > machine_power_off();

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-08-07 14:52    [W:0.024 / U:1.212 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site