* [PATCH v1 0/2] PM: hibernate: Fix GFP mask restoration issue and consolidate code
@ 2025-09-26 16:37 Rafael J. Wysocki
2025-09-26 16:40 ` [PATCH v1 1/2] PM: hibernate: Restrict GFP mask in power_down() Rafael J. Wysocki
2025-09-26 16:41 ` [PATCH v1 2/2] PM: hibernate: Combine return paths " Rafael J. Wysocki
0 siblings, 2 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 16:37 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Mario Limonciello
Hi All,
Unfortunately, there is another issue related to the pm_restrict_gfp_mask() and
pm_restore_gfp_mask() calls that needs to be addressed (patch [1/2]) and on
top of that, some code in power_down() can be consolidated to avoid duplication
and improve clarity (patch [2/2]).
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 1/2] PM: hibernate: Restrict GFP mask in power_down()
2025-09-26 16:37 [PATCH v1 0/2] PM: hibernate: Fix GFP mask restoration issue and consolidate code Rafael J. Wysocki
@ 2025-09-26 16:40 ` Rafael J. Wysocki
2025-09-26 16:47 ` Mario Limonciello
2025-09-26 16:41 ` [PATCH v1 2/2] PM: hibernate: Combine return paths " Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 16:40 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Mario Limonciello
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Commit 12ffc3b1513e ("PM: Restrict swap use to later in the
suspend sequence") caused hibernation_platform_enter() to call
pm_restore_gfp_mask() via dpm_resume_end(), so when power_down()
returns after aborting hibernation_platform_enter(), it needs
to match the pm_restore_gfp_mask() call in hibernate() that will
occur subsequently.
Address this by adding a pm_restrict_gfp_mask() call to the relevant
error path in power_down().
Fixes: 12ffc3b1513e ("PM: Restrict swap use to later in the suspend sequence")
Cc: 6.16+ <stable@vger.kernel.org> # 6.16+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
kernel/power/hibernate.c | 2 ++
1 file changed, 2 insertions(+)
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -733,6 +733,8 @@ static void power_down(void)
case HIBERNATION_PLATFORM:
error = hibernation_platform_enter();
if (error == -EAGAIN || error == -EBUSY) {
+ /* Match pm_restore_gfp_mask() in hibernate(). */
+ pm_restrict_gfp_mask();
swsusp_unmark();
events_check_enabled = false;
pr_info("Wakeup event detected during hibernation, rolling back.\n");
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] PM: hibernate: Combine return paths in power_down()
2025-09-26 16:37 [PATCH v1 0/2] PM: hibernate: Fix GFP mask restoration issue and consolidate code Rafael J. Wysocki
2025-09-26 16:40 ` [PATCH v1 1/2] PM: hibernate: Restrict GFP mask in power_down() Rafael J. Wysocki
@ 2025-09-26 16:41 ` Rafael J. Wysocki
2025-09-26 18:29 ` Mario Limonciello
1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 16:41 UTC (permalink / raw)
To: Linux PM; +Cc: LKML, Mario Limonciello
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
To avoid code duplication and improve clarity, combine the code
paths in power_down() leading to a return from that function.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
kernel/power/hibernate.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -708,21 +708,11 @@ static void power_down(void)
if (hibernation_mode == HIBERNATION_SUSPEND) {
pm_restore_gfp_mask();
error = suspend_devices_and_enter(mem_sleep_current);
- if (error) {
- hibernation_mode = hibernation_ops ?
- HIBERNATION_PLATFORM :
- HIBERNATION_SHUTDOWN;
- } else {
- /* Match pm_restore_gfp_mask() call in hibernate() */
- pm_restrict_gfp_mask();
+ if (!error)
+ goto rollback;
- /* Restore swap signature. */
- error = swsusp_unmark();
- if (error)
- pr_err("Swap will be unusable! Try swapon -a.\n");
-
- return;
- }
+ hibernation_mode = hibernation_ops ? HIBERNATION_PLATFORM :
+ HIBERNATION_SHUTDOWN;
}
#endif
@@ -733,12 +723,9 @@ static void power_down(void)
case HIBERNATION_PLATFORM:
error = hibernation_platform_enter();
if (error == -EAGAIN || error == -EBUSY) {
- /* Match pm_restore_gfp_mask() in hibernate(). */
- pm_restrict_gfp_mask();
- swsusp_unmark();
events_check_enabled = false;
pr_info("Wakeup event detected during hibernation, rolling back.\n");
- return;
+ goto rollback;
}
fallthrough;
case HIBERNATION_SHUTDOWN:
@@ -757,6 +744,15 @@ static void power_down(void)
pr_crit("Power down manually\n");
while (1)
cpu_relax();
+
+rollback:
+ /* Match the pm_restore_gfp_mask() call in hibernate(). */
+ pm_restrict_gfp_mask();
+
+ /* Restore swap signature. */
+ error = swsusp_unmark();
+ if (error)
+ pr_err("Swap will be unusable! Try swapon -a.\n");
}
static int load_image_and_restore(void)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] PM: hibernate: Restrict GFP mask in power_down()
2025-09-26 16:40 ` [PATCH v1 1/2] PM: hibernate: Restrict GFP mask in power_down() Rafael J. Wysocki
@ 2025-09-26 16:47 ` Mario Limonciello
0 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2025-09-26 16:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML
On 9/26/25 11:40 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit 12ffc3b1513e ("PM: Restrict swap use to later in the
> suspend sequence") caused hibernation_platform_enter() to call
> pm_restore_gfp_mask() via dpm_resume_end(), so when power_down()
> returns after aborting hibernation_platform_enter(), it needs
> to match the pm_restore_gfp_mask() call in hibernate() that will
> occur subsequently.
>
> Address this by adding a pm_restrict_gfp_mask() call to the relevant
> error path in power_down().
>
> Fixes: 12ffc3b1513e ("PM: Restrict swap use to later in the suspend sequence")
> Cc: 6.16+ <stable@vger.kernel.org> # 6.16+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>> ---
> kernel/power/hibernate.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -733,6 +733,8 @@ static void power_down(void)
> case HIBERNATION_PLATFORM:
> error = hibernation_platform_enter();
> if (error == -EAGAIN || error == -EBUSY) {
> + /* Match pm_restore_gfp_mask() in hibernate(). */
> + pm_restrict_gfp_mask();
> swsusp_unmark();
> events_check_enabled = false;
> pr_info("Wakeup event detected during hibernation, rolling back.\n");
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] PM: hibernate: Combine return paths in power_down()
2025-09-26 16:41 ` [PATCH v1 2/2] PM: hibernate: Combine return paths " Rafael J. Wysocki
@ 2025-09-26 18:29 ` Mario Limonciello
2025-09-26 18:31 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Mario Limonciello @ 2025-09-26 18:29 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM; +Cc: LKML
On 9/26/25 11:41 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> To avoid code duplication and improve clarity, combine the code
> paths in power_down() leading to a return from that function.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The actual change looks fine to me.
Since it's used in "both" a success and failure path would you consider
using "wakeup" for the label instead of "rollback"? Or anything else
you can think of that doesn't have a connotation of failure.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> kernel/power/hibernate.c | 32 ++++++++++++++------------------
> 1 file changed, 14 insertions(+), 18 deletions(-)
>
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -708,21 +708,11 @@ static void power_down(void)
> if (hibernation_mode == HIBERNATION_SUSPEND) {
> pm_restore_gfp_mask();
> error = suspend_devices_and_enter(mem_sleep_current);
> - if (error) {
> - hibernation_mode = hibernation_ops ?
> - HIBERNATION_PLATFORM :
> - HIBERNATION_SHUTDOWN;
> - } else {
> - /* Match pm_restore_gfp_mask() call in hibernate() */
> - pm_restrict_gfp_mask();
> + if (!error)
> + goto rollback;
>
> - /* Restore swap signature. */
> - error = swsusp_unmark();
> - if (error)
> - pr_err("Swap will be unusable! Try swapon -a.\n");
> -
> - return;
> - }
> + hibernation_mode = hibernation_ops ? HIBERNATION_PLATFORM :
> + HIBERNATION_SHUTDOWN;
> }
> #endif
>
> @@ -733,12 +723,9 @@ static void power_down(void)
> case HIBERNATION_PLATFORM:
> error = hibernation_platform_enter();
> if (error == -EAGAIN || error == -EBUSY) {
> - /* Match pm_restore_gfp_mask() in hibernate(). */
> - pm_restrict_gfp_mask();
> - swsusp_unmark();
> events_check_enabled = false;
> pr_info("Wakeup event detected during hibernation, rolling back.\n");
> - return;
> + goto rollback;
> }
> fallthrough;
> case HIBERNATION_SHUTDOWN:
> @@ -757,6 +744,15 @@ static void power_down(void)
> pr_crit("Power down manually\n");
> while (1)
> cpu_relax();
> +
> +rollback:
> + /* Match the pm_restore_gfp_mask() call in hibernate(). */
> + pm_restrict_gfp_mask();
> +
> + /* Restore swap signature. */
> + error = swsusp_unmark();
> + if (error)
> + pr_err("Swap will be unusable! Try swapon -a.\n");
> }
>
> static int load_image_and_restore(void)
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] PM: hibernate: Combine return paths in power_down()
2025-09-26 18:29 ` Mario Limonciello
@ 2025-09-26 18:31 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 18:31 UTC (permalink / raw)
To: Mario Limonciello; +Cc: Rafael J. Wysocki, Linux PM, LKML
On Fri, Sep 26, 2025 at 8:29 PM Mario Limonciello <superm1@kernel.org> wrote:
>
> On 9/26/25 11:41 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > To avoid code duplication and improve clarity, combine the code
> > paths in power_down() leading to a return from that function.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The actual change looks fine to me.
>
> Since it's used in "both" a success and failure path would you consider
> using "wakeup" for the label instead of "rollback"? Or anything else
> you can think of that doesn't have a connotation of failure.
Sure, "exit" comes to mind for instance.
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Thanks!
> > ---
> > kernel/power/hibernate.c | 32 ++++++++++++++------------------
> > 1 file changed, 14 insertions(+), 18 deletions(-)
> >
> > --- a/kernel/power/hibernate.c
> > +++ b/kernel/power/hibernate.c
> > @@ -708,21 +708,11 @@ static void power_down(void)
> > if (hibernation_mode == HIBERNATION_SUSPEND) {
> > pm_restore_gfp_mask();
> > error = suspend_devices_and_enter(mem_sleep_current);
> > - if (error) {
> > - hibernation_mode = hibernation_ops ?
> > - HIBERNATION_PLATFORM :
> > - HIBERNATION_SHUTDOWN;
> > - } else {
> > - /* Match pm_restore_gfp_mask() call in hibernate() */
> > - pm_restrict_gfp_mask();
> > + if (!error)
> > + goto rollback;
> >
> > - /* Restore swap signature. */
> > - error = swsusp_unmark();
> > - if (error)
> > - pr_err("Swap will be unusable! Try swapon -a.\n");
> > -
> > - return;
> > - }
> > + hibernation_mode = hibernation_ops ? HIBERNATION_PLATFORM :
> > + HIBERNATION_SHUTDOWN;
> > }
> > #endif
> >
> > @@ -733,12 +723,9 @@ static void power_down(void)
> > case HIBERNATION_PLATFORM:
> > error = hibernation_platform_enter();
> > if (error == -EAGAIN || error == -EBUSY) {
> > - /* Match pm_restore_gfp_mask() in hibernate(). */
> > - pm_restrict_gfp_mask();
> > - swsusp_unmark();
> > events_check_enabled = false;
> > pr_info("Wakeup event detected during hibernation, rolling back.\n");
> > - return;
> > + goto rollback;
> > }
> > fallthrough;
> > case HIBERNATION_SHUTDOWN:
> > @@ -757,6 +744,15 @@ static void power_down(void)
> > pr_crit("Power down manually\n");
> > while (1)
> > cpu_relax();
> > +
> > +rollback:
> > + /* Match the pm_restore_gfp_mask() call in hibernate(). */
> > + pm_restrict_gfp_mask();
> > +
> > + /* Restore swap signature. */
> > + error = swsusp_unmark();
> > + if (error)
> > + pr_err("Swap will be unusable! Try swapon -a.\n");
> > }
> >
> > static int load_image_and_restore(void)
> >
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-26 18:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-26 16:37 [PATCH v1 0/2] PM: hibernate: Fix GFP mask restoration issue and consolidate code Rafael J. Wysocki
2025-09-26 16:40 ` [PATCH v1 1/2] PM: hibernate: Restrict GFP mask in power_down() Rafael J. Wysocki
2025-09-26 16:47 ` Mario Limonciello
2025-09-26 16:41 ` [PATCH v1 2/2] PM: hibernate: Combine return paths " Rafael J. Wysocki
2025-09-26 18:29 ` Mario Limonciello
2025-09-26 18:31 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox