| Message ID | 1386169618-6417-1-git-send-email-bjorn@mork.no (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Wednesday, December 04, 2013 04:06:58 PM Bjørn Mork wrote: > The system can continue running after the image is created, for > example because userspace cancelled the hibernation or only > saved the image for a dual mode disk + ram suspend. We must > restore the platform to the pre_snapshot state in this case. No, this is not what is happening. :-) The problem is that the enable_nonboot_cpus() is run with EC transactions blocked if in_suspend is 1 and that causes the failure below to happen. So the changelog needs to be rewritten. That said, looking at what acpi_hibernation_leave() does I think we can do the change that you're proposing without any significant side effects. I'll queue up this change as a fix for 3.13 or 3.14, but I'll write a better changelog for it. Thanks! > This fixes a long standing bug in the acpi_cpufreq driver > which would fail after a cancelled hibernate because the > system then was running with the EC blocked: > > cpufreq: adding CPU 1 > acpi_cpufreq_cpu_init > cpufreq: FREQ: 1401000 - CPU: 0 > ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for [EmbeddedControl] (20130725/evregion-287) > ACPI Error: Method parse/execution failed [\_SB_.PCI0.LPC_.EC__.LPMD] (Node ffff88023249ab28), AE_BAD_PARAMETER (20130725/psparse-536) > ACPI Error: Method parse/execution failed [\_PR_.CPU0._PPC] (Node ffff88023270e3f8), AE_BAD_PARAMETER (20130725/psparse-536) > ACPI Error: Method parse/execution failed [\_PR_.CPU1._PPC] (Node ffff88023270e290), AE_BAD_PARAMETER (20130725/psparse-536) > ACPI Exception: AE_BAD_PARAMETER, Evaluating _PPC (20130725/processor_perflib-140) > cpufreq: initialization failed > CPU1 is up > > Signed-off-by: Bjørn Mork <bjorn@mork.no> > --- > This is posted as RFC because I am unsure whether this is the correct > way to fix the problem. We definitely need to undo some of the > pre_snapshot work to have a fully functional system when create_image > returns, but I guess there can be unwanted side effects of unconditionally > calling platform_leave() like this. > > FWIW, this works for me. > > Testing and verifying the original acpi_cpufreq problem should be easy on > any system using that driver and having an EC, which should include most > laptops: > - Start a userspace hibernation and cancel it, for example by using > s2disk from uswsusp and pressing backspace. > > The cpufreq device will disappear from all non-boot cores, and the above > ACPI exceptions will be logged. > > Comments and/or alternative solutions are appreciated. > > Note that the final solution to this problem should be backported to all > maintained stable kernels. I've verified that the problem exists and that > this proposed patch applies to 3.2, 3.4, 3.10 and 3.12 > > > Bjørn > > kernel/power/hibernate.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index 0121dab83f43..59fb7e82f818 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -293,10 +293,10 @@ static int create_image(int platform_mode) > error); > /* Restore control flow magically appears here */ > restore_processor_state(); > - if (!in_suspend) { > + if (!in_suspend) > events_check_enabled = false; > - platform_leave(platform_mode); > - } > + > + platform_leave(platform_mode); > > Power_up: > syscore_resume(); >
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index 0121dab83f43..59fb7e82f818 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -293,10 +293,10 @@ static int create_image(int platform_mode) error); /* Restore control flow magically appears here */ restore_processor_state(); - if (!in_suspend) { + if (!in_suspend) events_check_enabled = false; - platform_leave(platform_mode); - } + + platform_leave(platform_mode); Power_up: syscore_resume();
The system can continue running after the image is created, for example because userspace cancelled the hibernation or only saved the image for a dual mode disk + ram suspend. We must restore the platform to the pre_snapshot state in this case. This fixes a long standing bug in the acpi_cpufreq driver which would fail after a cancelled hibernate because the system then was running with the EC blocked: cpufreq: adding CPU 1 acpi_cpufreq_cpu_init cpufreq: FREQ: 1401000 - CPU: 0 ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for [EmbeddedControl] (20130725/evregion-287) ACPI Error: Method parse/execution failed [\_SB_.PCI0.LPC_.EC__.LPMD] (Node ffff88023249ab28), AE_BAD_PARAMETER (20130725/psparse-536) ACPI Error: Method parse/execution failed [\_PR_.CPU0._PPC] (Node ffff88023270e3f8), AE_BAD_PARAMETER (20130725/psparse-536) ACPI Error: Method parse/execution failed [\_PR_.CPU1._PPC] (Node ffff88023270e290), AE_BAD_PARAMETER (20130725/psparse-536) ACPI Exception: AE_BAD_PARAMETER, Evaluating _PPC (20130725/processor_perflib-140) cpufreq: initialization failed CPU1 is up Signed-off-by: Bjørn Mork <bjorn@mork.no> --- This is posted as RFC because I am unsure whether this is the correct way to fix the problem. We definitely need to undo some of the pre_snapshot work to have a fully functional system when create_image returns, but I guess there can be unwanted side effects of unconditionally calling platform_leave() like this. FWIW, this works for me. Testing and verifying the original acpi_cpufreq problem should be easy on any system using that driver and having an EC, which should include most laptops: - Start a userspace hibernation and cancel it, for example by using s2disk from uswsusp and pressing backspace. The cpufreq device will disappear from all non-boot cores, and the above ACPI exceptions will be logged. Comments and/or alternative solutions are appreciated. Note that the final solution to this problem should be backported to all maintained stable kernels. I've verified that the problem exists and that this proposed patch applies to 3.2, 3.4, 3.10 and 3.12 Bjørn kernel/power/hibernate.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)