Add some hooks to generic suspend code
Subsystems (3)
RFC 1/1 Add some hooks to generic suspend code
Hi !
While consolidating the powermac suspend to ram and suspend to disk implementations to properly use the new framework in kernel/power, among others, I ended up with the need of adding various callbacks to kernel/power/main.c. Here is a patch adding & documenting those.
The reasons I need them are:
/* Call before process freezing. If returns 0, then no freeze * should be done, if 1, freeze, negative -> error */ int (*pre_freeze)(suspend_state_t state);
I'm using that one for calling my "old style" notifiers (they are beeing phased out but I still have a couple of drivers using them). The reason I do that here is because that's how my APM emulation hooks, and that code interacts with userland (to properly signal things like X of the suspend process), so I need to do that before we freeze processes.
This call also allow me to decide not to freeze process (in my suspend-to-ram implementation) by returning 0 (the absence of the callback defaults to freeze of course, to not break existing code). So far, I have been using the freezer though, as it seem to be fast enough, but I still dislike relying on it for suspend to ram, it just hides bugs imho, and I've had a couple of problems with it where it failed to stop a process occasionally, or lost track of one on wakeup (and left it in D state in the refrigerator forever) for reasons I haven't yet explained.
/* called just before irqs are off and device second pass * and sysdevs are suspended. This function can on some archs * shut irqs off, in which case, they'll still be off when * finish_irqs() is called. */ int (*prepare_irqs)(suspend_state_t state);
I use that one to do some housekeeping that I want to do before the interrupts are switched off, that is after device_suspend() and before device_power_off(). That includes some mucking around with the interrupt controller (so that it doesn't try to deliver interrupts to a CPU that won't take them, that avoids some "issues" on wakeup), plus a couple of other things that don't quite fit in sysdev's and that I want to do before sysdev's get suspended.
Additionally, I shut interrupts down (with local_irq_disable) myself in that callback, which means that the "common" code which does local_irq_save/restore will end up not re-enabling them before it calls the "mirror" callback of that one (see below the explanation of finish_irqs), which I do need.
/* called after sysdevs and "irq off" devices have been * worken up, irqs have just been restored to whatever state * prepare_irqs() left them in. */ void (*finish_irqs)(suspend_state_t state);
This is the pending of the above callback. It gets called after sysdev's have been woken up but before normal devices have. It's called after the core has restored local interrupts to what they were upon exit of prepare_irqs(), so if you do nothing special in prepare_irqs(), you'll get entered with irqs re-enabled here, while if you exit prepare_irqs() with irqs off, you'll get here with irqs off as well (and thus become responsible for re-enabling them).
I want this callback to have finer control of re-enabling interrutps. The interrupt controller has been partially reconfigured earlier in arch code, but the CPU priority is only lowered here, so that it starts hitting the CPU again only now. There is also some code to properly wake up the CPU decrementer so it ticks right away, and to force taking a pseudo-interrupt (to sort-of "kick" the interrupt controller into life, seems to work around an issue that I think is related to a HW bug in the interrupt controller we use).
I also do some arch magic at this point, mirror of what was done in prepare_irqs(), that doesn't quite fit in a sysdev.
/* called after unfreezing userland */ void (*post_freeze)(suspend_state_t state);
That one is the mirror of pre-freeze, gets called after userland has been re-enabled, it also calls my old-style notifiers, which includes APM emulation, which is important for sending the APM wakeup events to things like X.
Please comment, I would like these patches in early 2.6.13 stages,
Thanks,
Ben.
include/linux/pm.h +30 -1
@@ -169,9 +169,38 @@struct pm_ops {suspend_disk_method_t pm_disk_mode; + +/* Call before process freezing. If returns 0, then no freeze +* should be done, if 1, freeze, negative -> error +*/ +int (*pre_freeze)(suspend_state_t state); + +/* called before devices are suspended */ int (*prepare)(suspend_state_t state); + +/* called just before irqs are off and device second pass +* and sysdevs are suspended. This function can on some archs +* shut irqs off, in which case, they'll still be off when +* finish_irqs() is called. +*/ +int (*prepare_irqs)(suspend_state_t state); + +/* called for entering the actual suspend state. Exits with +* machine worken up and interrupts off +*/ int (*enter)(suspend_state_t state); -int (*finish)(suspend_state_t state); + +/* called after sysdevs and "irq off" devices have been +* worken up, irqs have just been restored to whatever state +* prepare_irqs() left them in. +*/ +void (*finish_irqs)(suspend_state_t state); + +/* called after all devices are woken up, processes still frozen */ +void (*finish)(suspend_state_t state); + +/* called after unfreezing userland */ +void (*post_freeze)(suspend_state_t state); };extern void pm_set_ops (struct pm_ops *);
kernel/power/main.c +34 -4
@@ -23,6 +23,7 @@struct pm_ops * pm_ops =NULL ;suspend_disk_method_t pm_disk_mode =PM_DISK_SHUTDOWN ; +static int pm_process_frozen;/** * pm_set_ops - Set the global power method table.
@@ -49,32 +50,53 @@static int suspend_prepare (suspend_state_t state) {int error =0 ; +int freeze =1 ;if (!pm_ops || !pm_ops->enter)return -EPERM ;pm_prepare_console (); -if (freeze_processes ()) { +if (pm_ops->pre_freeze) + freeze = pm_ops->pre_freeze (state); +if (freeze <0 ) + goto Console; + +if (freeze &&freeze_processes ()) { error = -EAGAIN ; goto Thaw; } + pm_process_frozen = freeze;if (pm_ops->prepare) { +pr_debug ("preparing arch...\n" );if ((error = pm_ops->prepare (state))) goto Thaw; } +pr_debug ("suspending devices...\n" );if ((error =device_suspend (PMSG_SUSPEND ))) {printk (KERN_ERR "Some devices failed to suspend\n" ); goto Finish; } + +if (pm_ops->prepare_irqs) { +pr_debug ("preparing arch irqs...\n" ); +if ((error = pm_ops->prepare_irqs (state))) + goto Finish; + } +return 0 ; Finish:if (pm_ops->finish) pm_ops->finish (state); Thaw: -thaw_processes (); +if (freeze) +thaw_processes (); + +if (pm_ops->post_freeze) + pm_ops->post_freeze (state); + Console:pm_restore_console ();return error; }
@@ -109,10 +131,18 @@static void suspend_finish (suspend_state_t state) { +if (!pm_ops) +return ; + +if (pm_ops->finish_irqs) + pm_ops->finish_irqs (state);device_resume (); -if (pm_ops && pm_ops->finish) +if (pm_ops->finish) pm_ops->finish (state); -thaw_processes (); +if (pm_process_frozen) +thaw_processes (); +if (pm_ops->post_freeze) + pm_ops->post_freeze (state);pm_restore_console (); }
Hi!
Quoted 12 lines
While consolidating the powermac suspend to ram and suspend to disk implementations to properly use the new framework in kernel/power, among others, I ended up with the need of adding various callbacks to
kernel/power/main.c. Here is a patch adding & documenting those.The reasons I need them are:
/* Call before process freezing. If returns 0, then no freeze * should be done, if 1, freeze, negative -> error */ int (*pre_freeze)(suspend_state_t state);
Quoted 5 lines
I'm using that one for calling my "old style" notifiers (they are beeing phased out but I still have a couple of drivers using them). The reason I do that here is because that's how my APM emulation hooks, and that code interacts with userland (to properly signal things like X of the suspend process), so I need to do that before we freeze processes.
This should not be needed in future, right? Could it be marked deprecated or something?
Quoted 13 lines
/* called after sysdevs and "irq off" devices have been * worken up, irqs have just been restored to whatever state * prepare_irqs() left them in. */ void (*finish_irqs)(suspend_state_t state);This is the pending of the above callback. It gets called after sysdev's have been woken up but before normal devices have. It's called after the core has restored local interrupts to what they were upon exit of
prepare_irqs(), so if you do nothing special inprepare_irqs(), you'll get entered with irqs re-enabled here, while if you exitprepare_irqs()with irqs off, you'll get here with irqs off as well (and thus become responsible for re-enabling them).
Quoted 7 lines
I want this callback to have finer control of re-enabling interrutps. The interrupt controller has been partially reconfigured earlier in arch code, but the CPU priority is only lowered here, so that it starts hitting the CPU again only now. There is also some code to properly wake up the CPU decrementer so it ticks right away, and to force taking a pseudo-interrupt (to sort-of "kick" the interrupt controller into life, seems to work around an issue that I think is related to a HW bug in the interrupt controller we use).
Could you simply reconfigure interrupt controller fully in earlier arch code?
Quoted 6 lines
/* called after unfreezing userland */ void (*post_freeze)(suspend_state_t state);That one is the mirror of pre-freeze, gets called after userland has been re-enabled, it also calls my old-style notifiers, which includes APM emulation, which is important for sending the APM wakeup events to things like X.
Could this be marked deprecated, too?
Alternatively, proper way of notifying X (etc) should be created, and done from generic code....
Pavel
Quoted 14 lines · Pavel Machek
Hi!
Quoted 12 lines
While consolidating the powermac suspend to ram and suspend to disk implementations to properly use the new framework in kernel/power, among others, I ended up with the need of adding various callbacks to
kernel/power/main.c. Here is a patch adding & documenting those.The reasons I need them are:
/* Call before process freezing. If returns 0, then no freeze * should be done, if 1, freeze, negative -> error */ int (*pre_freeze)(suspend_state_t state);
Quoted 8 lines
Quoted 5 lines
I'm using that one for calling my "old style" notifiers (they are beeing phased out but I still have a couple of drivers using them). The reason I do that here is because that's how my APM emulation hooks, and that code interacts with userland (to properly signal things like X of the suspend process), so I need to do that before we freeze processes.
This should not be needed in future, right? Could it be marked deprecated or something?
Not really ... I need to notify userland before we freeze processes.
Quoted 21 lines
Quoted 13 lines
/* called after sysdevs and "irq off" devices have been * worken up, irqs have just been restored to whatever state * prepare_irqs() left them in. */ void (*finish_irqs)(suspend_state_t state);This is the pending of the above callback. It gets called after sysdev's have been woken up but before normal devices have. It's called after the core has restored local interrupts to what they were upon exit of
prepare_irqs(), so if you do nothing special inprepare_irqs(), you'll get entered with irqs re-enabled here, while if you exitprepare_irqs()with irqs off, you'll get here with irqs off as well (and thus become responsible for re-enabling them).Quoted 7 lines
I want this callback to have finer control of re-enabling interrutps. The interrupt controller has been partially reconfigured earlier in arch code, but the CPU priority is only lowered here, so that it starts hitting the CPU again only now. There is also some code to properly wake up the CPU decrementer so it ticks right away, and to force taking a pseudo-interrupt (to sort-of "kick" the interrupt controller into life, seems to work around an issue that I think is related to a HW bug in the interrupt controller we use).
Quoted 1 line
Could you simply reconfigure interrupt controller fully in earlier arch code?
Nope, it has to happen between device_suspend, ant device_power_off. Basically, I need control around the time we switch irqs off.
Quoted 11 lines
Quoted 6 lines
/* called after unfreezing userland */ void (*post_freeze)(suspend_state_t state);That one is the mirror of pre-freeze, gets called after userland has been re-enabled, it also calls my old-style notifiers, which includes APM emulation, which is important for sending the APM wakeup events to things like X.
Could this be marked deprecated, too?
Alternatively, proper way of notifying X (etc) should be created, and done from generic code....
Sure, ideally. However, existing X knows how to deal with APM events, and thus APM emulation is an important thing to get something that works. Pne thing I should do is consolidate PPC APM emu with ARM one as I think Russell improve my stuff significantly.
Ben.
Hi!
Quoted 18 lines
Quoted 18 lines
Quoted 17 lines
While consolidating the powermac suspend to ram and suspend to disk implementations to properly use the new framework in kernel/power, among others, I ended up with the need of adding various callbacks to
kernel/power/main.c. Here is a patch adding & documenting those.The reasons I need them are:
/* Call before process freezing. If returns 0, then no freeze * should be done, if 1, freeze, negative -> error */ int (*pre_freeze)(suspend_state_t state);I'm using that one for calling my "old style" notifiers (they are beeing phased out but I still have a couple of drivers using them). The reason I do that here is because that's how my APM emulation hooks, and that code interacts with userland (to properly signal things like X of the suspend process), so I need to do that before we freeze processes.
Quoted 4 lines
Quoted 2 lines
This should not be needed in future, right? Could it be marked deprecated or something?
Not really ... I need to notify userland before we freeze processes.
Why do you need it? Do you initiate suspend without userland asking you to?
Anyway, it should not be arch-dependend. We need one good mechanism of notifying userland, not one per architecture.
Quoted 9 lines
Quoted 9 lines
Quoted 6 lines
/* called after unfreezing userland */ void (*post_freeze)(suspend_state_t state);That one is the mirror of pre-freeze, gets called after userland has been re-enabled, it also calls my old-style notifiers, which includes APM emulation, which is important for sending the APM wakeup events to things like X.
Could this be marked deprecated, too?
Quoted 7 lines
Quoted 2 lines
Alternatively, proper way of notifying X (etc) should be created, and done from generic code....
Sure, ideally. However, existing X knows how to deal with APM events, and thus APM emulation is an important thing to get something that works. Pne thing I should do is consolidate PPC APM emu with ARM one as I think Russell improve my stuff significantly.
Perhaps we need apm emulation on i386, too?
Pavel
Quoted 3 lines
Why do you need it? Do you initiate suspend without userland asking you to?
Because there is an existing API, via /dev/apm_bios, and that's all X understands ! And because I've always done that ;)
Quoted 2 lines
Anyway, it should not be arch-dependend. We need one good mechanism of notifying userland, not one per architecture.
We need to define a new mecanism, I think. In the meantime, my APM emulation works though and I won't drop it.
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
Quoted 6 lines
/* called after unfreezing userland */ void (*post_freeze)(suspend_state_t state);That one is the mirror of pre-freeze, gets called after userland has been re-enabled, it also calls my old-style notifiers, which includes APM emulation, which is important for sending the APM wakeup events to things like X.
Could this be marked deprecated, too?
Alternatively, proper way of notifying X (etc) should be created, and done from generic code....
Quoted 6 lines
Quoted 4 lines
Sure, ideally. However, existing X knows how to deal with APM events, and thus APM emulation is an important thing to get something that works. Pne thing I should do is consolidate PPC APM emu with ARM one as I think Russell improve my stuff significantly.
Perhaps we need apm emulation on i386, too?
Maybe. It may help in some cases.
Ben.
Hi!
Quoted 5 lines
Quoted 2 lines
Why do you need it? Do you initiate suspend without userland asking you to?
Because there is an existing API, via
/dev/apm_bios, and that's all X understands ! And because I've always done that ;)
Try stopping doing that ;-).
[On i386, we do not emulate apm, and it still works. Reason is that we switch to other console before suspend, so X has to give up framebuffer control, anyway.]
Pavel
Quoted 9 lines · Pavel Machek
Hi!
Quoted 5 lines
Quoted 2 lines
Why do you need it? Do you initiate suspend without userland asking you to?
Because there is an existing API, via
/dev/apm_bios, and that's all X understands ! And because I've always done that ;)Try stopping doing that ;-).
Certainly not short-term. Again, it would be nice to have something better, but heh, you need to go step by step. I have this big rework where I re-implement most of the pmac suspend code on top of the generic code (cleans up a lot of stuff) but I don't want to touch the userland ABI for now, that would be too much of a chance. And /dev/apm_bios X notofication stuff seems to actually fix problems for some users.
Quoted 3 lines
[On i386, we do not emulate apm, and it still works. Reason is that we switch to other console before suspend, so X has to give up framebuffer control, anyway.]
Well, I sort-of work :) I have reported cases of X locking the machine up under some circumstances. Note that historically, I was not switching consoles in the pmac PM code, though I'm doing it nowadays.
There are other uses of those "events" in /dev/apm_bios. Some people run scripts on resume triggered by these for example, etc...
I'd rather not break an existing and relied upon userland interface now, at least not until we have a well accepted replacement that has been around for some time.
I do agree however that it may be nice to make the APM emulation code more generic & shared between architectures. That's something I intend to look into next. But I would like my current stuff to get in after 2.6.12 is released.
Ben.
Hi!
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines
Why do you need it? Do you initiate suspend without userland asking you to?
Because there is an existing API, via
/dev/apm_bios, and that's all X understands ! And because I've always done that ;)
Quoted 8 lines
Quoted 1 line
Try stopping doing that ;-).
Certainly not short-term. Again, it would be nice to have something better, but heh, you need to go step by step. I have this big rework where I re-implement most of the pmac suspend code on top of the generic code (cleans up a lot of stuff) but I don't want to touch the userland ABI for now, that would be too much of a chance. And
/dev/apm_biosX notofication stuff seems to actually fix problems for some users.
Ok.
Quoted 8 lines
I'd rather not break an existing and relied upon userland interface now, at least not until we have a well accepted replacement that has been around for some time.
I do agree however that it may be nice to make the APM emulation code more generic & shared between architectures. That's something I intend to look into next. But I would like my current stuff to get in after 2.6.12 is released.
Well, but that means that we can get those "please don't use these callbacks if you can avoid it" messages, right :-).
Seems like lots of stuff is going to happen in pm post-2.6.12: I'd like to finally fix pm_message_t, too...
Pavel
Hi.
Quoted 2 lines · Pavel Machek
Seems like lots of stuff is going to happen in pm post-2.6.12: I'd like to finally fix
pm_message_t, too...
Speaking of which, how do you want to move forward with the refrigerator patches I sent you a while back?
Regards,
Nigel
Hi!
Quoted 5 lines
Quoted 2 lines
Seems like lots of stuff is going to happen in pm post-2.6.12: I'd like to finally fix
pm_message_t, too...Speaking of which, how do you want to move forward with the refrigerator patches I sent you a while back?
Can you send them again? I'll either comment on them or integrate them into tree for linus (linux-good).
Pavel
Quoted 3 lines
Well, but that means that we can get those "please don't use these callbacks if you can avoid it" messages, right :-).
Heh, well, I'll look if I can do something more specific to APM emu there, maybe I'll find time to just switch over to a generic implementation.
Quoted 2 lines
Seems like lots of stuff is going to happen in pm post-2.6.12: I'd like to finally fix
pm_message_t, too...
Yup. I have a couple of driver patches fixing things in this area too.
Ben.
Quoted 2 lines · Pavel Machek
Anyway, it should not be arch-dependend. We need one good mechanism of notifying userland, not one per architecture.
Yes.
Quoted 6 lines
Quoted 4 lines
Sure, ideally. However, existing X knows how to deal with APM events, and thus APM emulation is an important thing to get something that works. Pne thing I should do is consolidate PPC APM emu with ARM one as I think Russell improve my stuff significantly.
Perhaps we need apm emulation on i386, too?
No. This is too ugly for words IMO. If we have one good mechanism of notifying userland, X can use this mechanism. Let's kill APM, not keep it alive.
Signature · 3 lines
Stefan Seyfried QA / R&D Team Mobile Devices | "Any ideas, John?" SUSE LINUX Products GmbH, Nürnberg | "Well, surrounding them's out."
Quoted 10 lines · Stefan Seyfried
Quoted 6 lines
Quoted 4 lines
Sure, ideally. However, existing X knows how to deal with APM events, and thus APM emulation is an important thing to get something that works. Pne thing I should do is consolidate PPC APM emu with ARM one as I think Russell improve my stuff significantly.
Perhaps we need apm emulation on i386, too?
No. This is too ugly for words IMO. If we have one good mechanism of notifying userland, X can use this mechanism. Let's kill APM, not keep it alive.
Euh... maybe but I still think we need to keep this userland interface alive for a little while. At least ARM and PPC have existing stuff that rely on it.
Ben.