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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b71b3e35-fc76-4397-9d60-20778685e37c@collabora.com>
Date: Tue, 25 Nov 2025 14:53:40 +0500
From: Muhammad Usama Anjum <usama.anjum@...labora.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: usama.anjum@...labora.com, 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

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].

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)?

[1] 56a232d93cea ("PM: sleep: Make pm_wakeup_clear() call more clear") - Aug 20
[2] 79816d4b9e9b ("Revert "PM: sleep: Make pm_wakeup_clear() call more clear"") - Oct 22

---
Thanks,
Usama