PM: Suspend and hibernate error paths fixes
Subsystems (3)
cover PM: Suspend and hibernate error paths fixes
Hi,
The following three patches are intended to fix a few bugs spotted recently in the suspend/hibernation core code.
Since they are located in error paths, hopefully they have no chance to trigger to often. ;-)
Rafael
PATCH 1/3 PM / Hibernate: Fix hibernation_platform_enter()
From: Rafael J. Wysocki <[email protected]>
The hibernation_platform_enter() function calls dpm_suspend_noirq() instead of dpm_resume_noirq() by mistake. Fix this.
--- kernel/power/hibernate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/power/hibernate.c +1 -1
@@ -530,7 +530,7 @@ int hibernation_platform_enter(void) Platform_finish: hibernation_ops->finish (); -dpm_suspend_noirq (PMSG_RESTORE ); +dpm_resume_noirq (PMSG_RESTORE ); Resume_devices: entering_platform_hibernation = false;
PATCH 1/3 PM / Hibernate: Fix hibernation_platform_enter()
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
The hibernation_platform_enter() function calls dpm_suspend_noirq() instead of dpm_resume_noirq() by mistake. Fix this.
--- kernel/power/hibernate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/power/hibernate.c +1 -1
@@ -530,7 +530,7 @@ int hibernation_platform_enter(void) Platform_finish: hibernation_ops->finish (); -dpm_suspend_noirq (PMSG_RESTORE ); +dpm_resume_noirq (PMSG_RESTORE ); Resume_devices: entering_platform_hibernation = false;
PATCH 2/3 PM / Hibernate: Fix snapshot error code path
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
There is an inconsistency between hibernation_platform_enter() and hibernation_snapshot(), because the latter calls hibernation_ops->end() after failing hibernation_ops->begin(), while the former doesn't do that. Make hibernation_snapshot() behave in the same way as hibernation_platform_enter() in that respect.
--- kernel/power/hibernate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/power/hibernate.c +1 -1
@@ -330,7 +330,7 @@ int hibernation_snapshot(int platform_mo error =platform_begin (platform_mode);if (error) -return error; + goto Close;/* Preallocate image memory before shutting down devices. */ error =hibernate_preallocate_memory ();
PATCH 2/3 PM / Hibernate: Fix snapshot error code path
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
There is an inconsistency between hibernation_platform_enter() and hibernation_snapshot(), because the latter calls hibernation_ops->end() after failing hibernation_ops->begin(), while the former doesn't do that. Make hibernation_snapshot() behave in the same way as hibernation_platform_enter() in that respect.
--- kernel/power/hibernate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/power/hibernate.c +1 -1
@@ -330,7 +330,7 @@ int hibernation_snapshot(int platform_mo error =platform_begin (platform_mode);if (error) -return error; + goto Close;/* Preallocate image memory before shutting down devices. */ error =hibernate_preallocate_memory ();
PATCH 3/3 PM / Suspend: Fix ordering of calls in suspend error paths
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
The ACPI suspend code calls suspend_nvs_free() at a wrong place, which may lead to a memory leak if there's an error executing acpi_pm_prepare(), because acpi_pm_finish() will not be called in that case. However, the root cause of this problem is the apparently confusing ordering of calls in suspend error paths that needs to be fixed.
In addition to that, fix a typo in a label name in suspend.c.
--- include/linux/suspend.h | 10 ++++++---- kernel/power/suspend.c | 9 ++++----- 2 files changed, 10 insertions(+), 9 deletions(-)
include/linux/suspend.h +6 -4
@@ -61,14 +61,15 @@ typedef int __bitwise suspend_state_t;* before device drivers' late suspend callbacks are executed. It returns * 0 on success or a negative error code otherwise, in which case the * system cannot enter the desired sleep state (@prepare_late(), @enter(), -* @wake(), and @finish() will not be called in that case). +* and @wake() will not be called in that case). * * @prepare_late: Finish preparing the platform for entering the system sleep * state indicated by @begin(). * @prepare_late is called before disabling nonboot CPUs and after * device drivers' late suspend callbacks have been executed. It returns * 0 on success or a negative error code otherwise, in which case the -* system cannot enter the desired sleep state (@enter() and @wake()). +* system cannot enter the desired sleep state (@enter() will not be +* executed). * * @enter: Enter the system sleep state indicated by @begin() or represented by * the argument if @begin() is not implemented.
@@ -81,14 +82,15 @@ typedef int __bitwise suspend_state_t;* resume callbacks are executed. * This callback is optional, but should be implemented by the platforms * that implement @prepare_late(). If implemented, it is always called -* after @enter(), even if @enter() fails. +* after @prepare_late and @enter(), even if one of them fails. * * @finish: Finish wake-up of the platform. * @finish is called right prior to calling device drivers' regular suspend * callbacks. * This callback is optional, but should be implemented by the platforms * that implement @prepare(). If implemented, it is always called after -* @enter() and @wake(), if implemented, even if any of them fails. +* @enter() and @wake(), even if any of them fails. It is executed after +* a failing @prepare. * * @end: Called by the PM core right after resuming devices, to indicate to * the platform that the system has returned to the working state or
kernel/power/suspend.c +4 -5
@@ -145,19 +145,19 @@ static int suspend_enter(suspend_state_tif (suspend_ops->prepare) { error = suspend_ops->prepare ();if (error) -return error; + goto Platform_finish; } error =dpm_suspend_noirq (PMSG_SUSPEND );if (error) {printk (KERN_ERR "PM: Some devices failed to power down\n" ); - goto Platfrom_finish; + goto Platform_finish; }if (suspend_ops->prepare_late) { error = suspend_ops->prepare_late ();if (error) - goto Power_up_devices; + goto Platform_wake; }if (suspend_test (TEST_PLATFORM ))
@@ -189,10 +189,9 @@ static int suspend_enter(suspend_state_tif (suspend_ops->wake) suspend_ops->wake (); - Power_up_devices:dpm_resume_noirq (PMSG_RESUME ); - Platfrom_finish: + Platform_finish:if (suspend_ops->finish) suspend_ops->finish ();
PATCH 3/3 PM / Suspend: Fix ordering of calls in suspend error paths
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
The ACPI suspend code calls suspend_nvs_free() at a wrong place, which may lead to a memory leak if there's an error executing acpi_pm_prepare(), because acpi_pm_finish() will not be called in that case. However, the root cause of this problem is the apparently confusing ordering of calls in suspend error paths that needs to be fixed.
In addition to that, fix a typo in a label name in suspend.c.
--- include/linux/suspend.h | 10 ++++++---- kernel/power/suspend.c | 9 ++++----- 2 files changed, 10 insertions(+), 9 deletions(-)
include/linux/suspend.h +6 -4
@@ -61,14 +61,15 @@ typedef int __bitwise suspend_state_t;* before device drivers' late suspend callbacks are executed. It returns * 0 on success or a negative error code otherwise, in which case the * system cannot enter the desired sleep state (@prepare_late(), @enter(), -* @wake(), and @finish() will not be called in that case). +* and @wake() will not be called in that case). * * @prepare_late: Finish preparing the platform for entering the system sleep * state indicated by @begin(). * @prepare_late is called before disabling nonboot CPUs and after * device drivers' late suspend callbacks have been executed. It returns * 0 on success or a negative error code otherwise, in which case the -* system cannot enter the desired sleep state (@enter() and @wake()). +* system cannot enter the desired sleep state (@enter() will not be +* executed). * * @enter: Enter the system sleep state indicated by @begin() or represented by * the argument if @begin() is not implemented.
@@ -81,14 +82,15 @@ typedef int __bitwise suspend_state_t;* resume callbacks are executed. * This callback is optional, but should be implemented by the platforms * that implement @prepare_late(). If implemented, it is always called -* after @enter(), even if @enter() fails. +* after @prepare_late and @enter(), even if one of them fails. * * @finish: Finish wake-up of the platform. * @finish is called right prior to calling device drivers' regular suspend * callbacks. * This callback is optional, but should be implemented by the platforms * that implement @prepare(). If implemented, it is always called after -* @enter() and @wake(), if implemented, even if any of them fails. +* @enter() and @wake(), even if any of them fails. It is executed after +* a failing @prepare. * * @end: Called by the PM core right after resuming devices, to indicate to * the platform that the system has returned to the working state or
kernel/power/suspend.c +4 -5
@@ -145,19 +145,19 @@ static int suspend_enter(suspend_state_tif (suspend_ops->prepare) { error = suspend_ops->prepare ();if (error) -return error; + goto Platform_finish; } error =dpm_suspend_noirq (PMSG_SUSPEND );if (error) {printk (KERN_ERR "PM: Some devices failed to power down\n" ); - goto Platfrom_finish; + goto Platform_finish; }if (suspend_ops->prepare_late) { error = suspend_ops->prepare_late ();if (error) - goto Power_up_devices; + goto Platform_wake; }if (suspend_test (TEST_PLATFORM ))
@@ -189,10 +189,9 @@ static int suspend_enter(suspend_state_tif (suspend_ops->wake) suspend_ops->wake (); - Power_up_devices:dpm_resume_noirq (PMSG_RESUME ); - Platfrom_finish: + Platform_finish:if (suspend_ops->finish) suspend_ops->finish ();
Quoted 7 lines · Rafael J. Wysocki
Hi,
The following three patches are intended to fix a few bugs spotted recently in the suspend/hibernation core code.
Since they are located in error paths, hopefully they have no chance to trigger to often. ;-)
So, I assume the lack of responses implies the lack of objections. :-)
Rafael
Quoted 7 lines · Rafael J. Wysocki
Hi,
The following three patches are intended to fix a few bugs spotted recently in the suspend/hibernation core code.
Since they are located in error paths, hopefully they have no chance to trigger to often. ;-)
So, I assume the lack of responses implies the lack of objections. :-)
Rafael
I assume you'll send these up via your suspend tree.
thanks,
Len Brown, Intel Open Source Technology Center
I assume you'll send these up via your suspend tree.
thanks,
Len Brown, Intel Open Source Technology Center
Quoted 3 lines · Len Brown
Acked-by Len Brown <[email protected]>I assume you'll send these up via your suspend tree.
Yes, I will.
Thanks,
Rafael
Quoted 3 lines · Len Brown
Acked-by Len Brown <[email protected]>I assume you'll send these up via your suspend tree.
Yes, I will.
Thanks,
Rafael