linux-kernel - Re: [PATCH 4/4] PM: sleep: clear pm_abort

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0icvqaL1YwBGYrF9K_hKwPjwYmP50Js61Ru3pphpjmBwg@mail.gmail.com>
Date: Tue, 25 Nov 2025 13:59:29 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Muhammad Usama Anjum <usama.anjum@...labora.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, Pavel Machek <pavel@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Danilo Krummrich <dakr@...nel.org>, 
	Dmitry Torokhov <dmitry.torokhov@...il.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Peter Zijlstra <peterz@...radead.org>, linux-acpi@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, 
	linux-input@...r.kernel.org, kernel@...labora.com, superm1@...nel.org
Subject: Re: [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend

On Tue, Nov 25, 2025 at 10:54 AM Muhammad Usama Anjum
<usama.anjum@...labora.com> wrote:
>
> Hi Rafael,
>
> Thank you for reviewing.
>
> On 11/24/25 11:54 PM, Rafael J. Wysocki wrote:
> > On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
> > <usama.anjum@...labora.com> wrote:
> >>
> >> Clear pm_abort_suspend counter in case a wakeup is detected during
> >> hibernation process. If this counter isn't reset, it'll affect the
> >> next hibernation cycle and next time hibernation will not happen as
> >> pm_abort_suspend is still positive.
> >>
> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> >> ---
> >>  drivers/base/power/main.c | 2 ++
> >>  kernel/cpu.c              | 1 +
> >>  kernel/power/hibernate.c  | 5 ++++-
> >>  kernel/power/process.c    | 1 +
> >>  4 files changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> >> index 5760abb25b591..84e76f8df1e02 100644
> >> --- a/drivers/base/power/main.c
> >> +++ b/drivers/base/power/main.c
> >> @@ -1642,6 +1642,7 @@ static void device_suspend_late(struct device *dev, pm_message_t state, bool asy
> >>                 goto Complete;
> >>
> >>         if (pm_wakeup_pending()) {
> >> +               pm_wakeup_clear(0);
> >>                 WRITE_ONCE(async_error, -EBUSY);
> >>                 goto Complete;
> >>         }
> >> @@ -1887,6 +1888,7 @@ static void device_suspend(struct device *dev, pm_message_t state, bool async)
> >>
> >>         if (pm_wakeup_pending()) {
> >>                 dev->power.direct_complete = false;
> >> +               pm_wakeup_clear(0);
> >>                 WRITE_ONCE(async_error, -EBUSY);
> >>                 goto Complete;
> >>         }
> >> diff --git a/kernel/cpu.c b/kernel/cpu.c
> >> index db9f6c539b28c..74c9f6b4947dd 100644
> >> --- a/kernel/cpu.c
> >> +++ b/kernel/cpu.c
> >> @@ -1921,6 +1921,7 @@ int freeze_secondary_cpus(int primary)
> >>
> >>                 if (pm_wakeup_pending()) {
> >>                         pr_info("Wakeup pending. Abort CPU freeze\n");
> >> +                       pm_wakeup_clear(0);
> >>                         error = -EBUSY;
> >>                         break;
> >>                 }
> >> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> >> index e15907f28c4cd..1f6b60df45d34 100644
> >> --- a/kernel/power/hibernate.c
> >> +++ b/kernel/power/hibernate.c
> >> @@ -349,8 +349,10 @@ static int create_image(int platform_mode)
> >>                 goto Enable_irqs;
> >>         }
> >>
> >> -       if (hibernation_test(TEST_CORE) || pm_wakeup_pending())
> >> +       if (hibernation_test(TEST_CORE) || pm_wakeup_pending()) {
> >> +               pm_wakeup_clear(0);
> >>                 goto Power_up;
> >> +       }
> >>
> >>         in_suspend = 1;
> >>         save_processor_state();
> >> @@ -660,6 +662,7 @@ int hibernation_platform_enter(void)
> >>                 goto Enable_irqs;
> >>
> >>         if (pm_wakeup_pending()) {
> >> +               pm_wakeup_clear(0);
> >>                 error = -EAGAIN;
> >>                 goto Power_up;
> >>         }
> >> diff --git a/kernel/power/process.c b/kernel/power/process.c
> >> index dc0dfc349f22b..e935b27a04ae0 100644
> >> --- a/kernel/power/process.c
> >> +++ b/kernel/power/process.c
> >> @@ -67,6 +67,7 @@ static int try_to_freeze_tasks(bool user_only)
> >>                         break;
> >>
> >>                 if (pm_wakeup_pending()) {
> >> +                       pm_wakeup_clear(0);
> >>                         wakeup = true;
> >>                         break;
> >>                 }
> >> --
> >
> > I don't think pm_wakeup_clear() needs to be called in so many places.
> >
> > Any why isn't it sufficient to call it in freeze_processes()?  For
> > suspend, it is sufficient, so what's different about hibernation in
> > that respect?
>
> It seems this patch was written by me when [1] was added which removed the
> unconditional call pm_wakeup_clear(0) from freeze_processes(). It was later
> reverted [2].

OK, I see.

> I've removed this patch and tested again to find out:
> - try_to_freeze_tasks() gets called from freeze_process() after
>   unconditional clearing of pm_wakeup. So pm_wakeup doesn't get cleared
>   until next hibernation or any other similar operation. So for hibernation
>   cancellation this patch isn't required. I'll drop it.
>
> But shouldn't this wakeup event be consumed without waiting for next hibernation
> (or similar operation to happen)?

I'm not sure what you mean.

Consuming an event is not related to calling pm_wakeup_clear().

pm_wakeup_clear() is related to wakeup IRQ handling, see pm_system_irq_wakeup().

This takes place after IRQs have been suspended (that's what the
"noirq" suspend phase is about).