linux-kernel - Re: [PATCH v1 2/2] PM: hibernate: Combine return paths in power

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJZ5v0h3r8PKU6Q1bL0yr1seuVH3HRrdBa9-+x5eQUuU0CsYkQ@mail.gmail.com>
Date: Fri, 26 Sep 2025 20:31:48 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Mario Limonciello <superm1@...nel.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Linux PM <linux-pm@...r.kernel.org>, 
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] PM: hibernate: Combine return paths in power_down()

On Fri, Sep 26, 2025 at 8:29 PM Mario Limonciello <superm1@...nel.org> wrote:
>
> On 9/26/25 11:41 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.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@...el.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@...nel.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)
> >
> >
> >
>