linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support
@ 2025-11-07 18:44 Muhammad Usama Anjum
  2025-11-07 18:44 ` [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress() Muhammad Usama Anjum
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-07 18:44 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input
  Cc: Muhammad Usama Anjum, kernel, superm1

On a normal laptop/PC, the hibernation takes 15-20 seconds which is
considerable time. Once hibernation is triggered from command line or by
some GUI option, the hibernation cannot be cancelled until completed.
Its not a blocker, but poor user experience.

When power button is pressed during hibernation, it generates interrupt
and then the event is routed to userspace. If systemd is being used, the
logind handles these events and performs the specific action.

During hibernation, the first stage is to freeze the userspace. Hence
even if the power button is pressed, it doesn't aborts the hibernation
as user space daemon is frozen.

My device takes ~19 seconds to hibernate. When I was testing hibernation
using rtcwake with timeout of 10 seconds, I found out that hibernation
gets canceled around 10 seconds mark when the interrupt fires.

In this series, the idea is to find a way to cancel the hibernation.
With this series applied, the hibernation gets cancelled gracefully.

The hibernation cancellation support isn't present in very last stage
just before power_down(). I've not been able to handle the error paths
correctly there yet as logs aren't available from that stage. I'll send
that patch separately when it works.

Cc: rafael@kernel.org
Cc: superm1@kernel.org
---
Changes since RFC:
- Update description of patches
- Use pm_sleep_transition_in_progress() instead of
  hibernation_in_progress()

Muhammad Usama Anjum (4):
  PM: hibernate: export pm_sleep_transition_in_progress()
  ACPI: button: Cancel hibernation if button is pressed during
    hibernation
  Input: Ignore the KEY_POWER events if hibernation is in progress
  PM: sleep: clear pm_abort_suspend at suspend

 drivers/acpi/button.c     | 12 +++++++++---
 drivers/base/power/main.c |  2 ++
 drivers/input/input.c     |  6 ++++++
 kernel/cpu.c              |  1 +
 kernel/power/hibernate.c  |  5 ++++-
 kernel/power/main.c       |  1 +
 kernel/power/process.c    |  1 +
 7 files changed, 24 insertions(+), 4 deletions(-)

-- 
2.47.3


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress()
  2025-11-07 18:44 [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
@ 2025-11-07 18:44 ` Muhammad Usama Anjum
  2025-11-24 17:01   ` Greg Kroah-Hartman
  2025-11-07 18:44 ` [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation Muhammad Usama Anjum
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-07 18:44 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input
  Cc: Muhammad Usama Anjum, kernel, superm1

Export pm_sleep_transition_in_progress() to be used by other
modules.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 kernel/power/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/power/main.c b/kernel/power/main.c
index 33a47ed15994f..ff3354b5be150 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -570,6 +570,7 @@ bool pm_sleep_transition_in_progress(void)
 {
 	return pm_suspend_in_progress() || hibernation_in_progress();
 }
+EXPORT_SYMBOL_GPL(pm_sleep_transition_in_progress);
 #endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_PM_SLEEP_DEBUG
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-07 18:44 [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
  2025-11-07 18:44 ` [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress() Muhammad Usama Anjum
@ 2025-11-07 18:44 ` Muhammad Usama Anjum
  2025-11-24 17:03   ` Greg Kroah-Hartman
  2025-11-24 18:42   ` Rafael J. Wysocki
  2025-11-07 18:44 ` [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress Muhammad Usama Anjum
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-07 18:44 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input
  Cc: Muhammad Usama Anjum, kernel, superm1

acpi_pm_wakeup_event() is called from acpi_button_notify() which is
called when power button is pressed. The system is worken up from s2idle
in this case by setting hard parameter to pm_wakeup_dev_event().

Call acpi_pm_wakeup_event() if power button is pressed and hibernation
is in progress. Set the hard parameter such that pm_system_wakeup()
gets called which increments pm_abort_suspend counter. The explicit call
to acpi_pm_wakeup_event() is necessary as ACPI button device has the
wakeup source. Hence call to input_report_key() with input device
doesn't call pm_system_wakeup() as it doesn't have wakeup source
registered.

Hence hibernation would be cancelled as in hibernation path, this counter
is checked if it should be aborted.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since RFC:
- Use pm_sleep_transition_in_progress()
- Update descriptin why explicit call to acpi_pm_wakeup_event() is
  necessary
---
 drivers/acpi/button.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 3c6dd9b4ba0ad..e4be5c763edaf 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -20,6 +20,7 @@
 #include <linux/acpi.h>
 #include <linux/dmi.h>
 #include <acpi/button.h>
+#include <linux/suspend.h>
 
 #define ACPI_BUTTON_CLASS		"button"
 #define ACPI_BUTTON_FILE_STATE		"state"
@@ -458,11 +459,16 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
 	acpi_pm_wakeup_event(&device->dev);
 
 	button = acpi_driver_data(device);
-	if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
-		return;
-
 	input = button->input;
 	keycode = test_bit(KEY_SLEEP, input->keybit) ? KEY_SLEEP : KEY_POWER;
+	if (event == ACPI_BUTTON_NOTIFY_STATUS && keycode == KEY_POWER &&
+	    pm_sleep_transition_in_progress()) {
+		pm_wakeup_dev_event(&device->dev, 0, true);
+		return;
+	}
+
+	if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
+		return;
 
 	input_report_key(input, keycode, 1);
 	input_sync(input);
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress
  2025-11-07 18:44 [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
  2025-11-07 18:44 ` [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress() Muhammad Usama Anjum
  2025-11-07 18:44 ` [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation Muhammad Usama Anjum
@ 2025-11-07 18:44 ` Muhammad Usama Anjum
  2025-11-24 18:50   ` Rafael J. Wysocki
  2025-11-07 18:44 ` [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend Muhammad Usama Anjum
  2025-11-24 13:03 ` [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
  4 siblings, 1 reply; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-07 18:44 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input
  Cc: Muhammad Usama Anjum, kernel, superm1

Input (Serio) drivers call input_handle_event(). Although the serio
drivers have duplicate events, they have separate code path and call
input_handle_event(). Ignore the KEY_POWER such that this event isn't
sent to the userspace if hibernation is in progress.

Abort the hibernation by calling pm_wakeup_dev_event(). In case of serio,
doesn't have wakeup source registered, this call doesn't do anything.
But there may be other input drivers which will require this.

Without this, the event is sent to the userspace and it suspends the
device after hibernation cancellation.

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since RFC:
- Use pm_sleep_transition_in_progress()
- Update description
---
 drivers/input/input.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index a500e1e276c21..7939bd9e47668 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -26,6 +26,7 @@
 #include <linux/kstrtox.h>
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/suspend.h>
 #include "input-compat.h"
 #include "input-core-private.h"
 #include "input-poller.h"
@@ -362,6 +363,11 @@ void input_handle_event(struct input_dev *dev,
 
 	lockdep_assert_held(&dev->event_lock);
 
+	if (code == KEY_POWER && pm_sleep_transition_in_progress()) {
+		pm_wakeup_dev_event(&dev->dev, 0, true);
+		return;
+	}
+
 	disposition = input_get_disposition(dev, type, code, &value);
 	if (disposition != INPUT_IGNORE_EVENT) {
 		if (type != EV_SYN)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend
  2025-11-07 18:44 [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
                   ` (2 preceding siblings ...)
  2025-11-07 18:44 ` [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress Muhammad Usama Anjum
@ 2025-11-07 18:44 ` Muhammad Usama Anjum
  2025-11-24 18:54   ` Rafael J. Wysocki
  2025-11-24 13:03 ` [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
  4 siblings, 1 reply; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-07 18:44 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input
  Cc: Muhammad Usama Anjum, kernel, superm1

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@collabora.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;
 		}
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support
  2025-11-07 18:44 [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
                   ` (3 preceding siblings ...)
  2025-11-07 18:44 ` [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend Muhammad Usama Anjum
@ 2025-11-24 13:03 ` Muhammad Usama Anjum
  4 siblings, 0 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-24 13:03 UTC (permalink / raw)
  To: Rafael J. Wysocki, superm1
  Cc: usama.anjum, kernel, Len Brown, linux-pm, Danilo Krummrich,
	Pavel Machek, Thomas Gleixner, Peter Zijlstra, Greg Kroah-Hartman,
	linux-input, Dmitry Torokhov, linux-acpi, linux-kernel

Hi,

Please review the patches when you have time. 

On 11/7/25 11:44 PM, Muhammad Usama Anjum wrote:
> On a normal laptop/PC, the hibernation takes 15-20 seconds which is
> considerable time. Once hibernation is triggered from command line or by
> some GUI option, the hibernation cannot be cancelled until completed.
> Its not a blocker, but poor user experience.
> 
> When power button is pressed during hibernation, it generates interrupt
> and then the event is routed to userspace. If systemd is being used, the
> logind handles these events and performs the specific action.
> 
> During hibernation, the first stage is to freeze the userspace. Hence
> even if the power button is pressed, it doesn't aborts the hibernation
> as user space daemon is frozen.
> 
> My device takes ~19 seconds to hibernate. When I was testing hibernation
> using rtcwake with timeout of 10 seconds, I found out that hibernation
> gets canceled around 10 seconds mark when the interrupt fires.
> 
> In this series, the idea is to find a way to cancel the hibernation.
> With this series applied, the hibernation gets cancelled gracefully.
> 
> The hibernation cancellation support isn't present in very last stage
> just before power_down(). I've not been able to handle the error paths
> correctly there yet as logs aren't available from that stage. I'll send
> that patch separately when it works.
> 
> Cc: rafael@kernel.org
> Cc: superm1@kernel.org
> ---
> Changes since RFC:
> - Update description of patches
> - Use pm_sleep_transition_in_progress() instead of
>   hibernation_in_progress()
> 
> Muhammad Usama Anjum (4):
>   PM: hibernate: export pm_sleep_transition_in_progress()
>   ACPI: button: Cancel hibernation if button is pressed during
>     hibernation
>   Input: Ignore the KEY_POWER events if hibernation is in progress
>   PM: sleep: clear pm_abort_suspend at suspend
> 
>  drivers/acpi/button.c     | 12 +++++++++---
>  drivers/base/power/main.c |  2 ++
>  drivers/input/input.c     |  6 ++++++
>  kernel/cpu.c              |  1 +
>  kernel/power/hibernate.c  |  5 ++++-
>  kernel/power/main.c       |  1 +
>  kernel/power/process.c    |  1 +
>  7 files changed, 24 insertions(+), 4 deletions(-)
> 


-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress()
  2025-11-07 18:44 ` [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress() Muhammad Usama Anjum
@ 2025-11-24 17:01   ` Greg Kroah-Hartman
  2025-11-25 16:44     ` Muhammad Usama Anjum
  0 siblings, 1 reply; 26+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-24 17:01 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Danilo Krummrich,
	Dmitry Torokhov, Thomas Gleixner, Peter Zijlstra, linux-acpi,
	linux-kernel, linux-pm, linux-input, kernel, superm1

On Fri, Nov 07, 2025 at 11:44:28PM +0500, Muhammad Usama Anjum wrote:
> Export pm_sleep_transition_in_progress() to be used by other
> modules.
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  kernel/power/main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 33a47ed15994f..ff3354b5be150 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -570,6 +570,7 @@ bool pm_sleep_transition_in_progress(void)
>  {
>  	return pm_suspend_in_progress() || hibernation_in_progress();
>  }
> +EXPORT_SYMBOL_GPL(pm_sleep_transition_in_progress);

The problem is, you can not rely on the result of this call as it could
change right after you call it, right?

So who needs to call this and why?  What new workflow requires this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-07 18:44 ` [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation Muhammad Usama Anjum
@ 2025-11-24 17:03   ` Greg Kroah-Hartman
  2025-11-25 11:12     ` Muhammad Usama Anjum
  2025-11-24 18:42   ` Rafael J. Wysocki
  1 sibling, 1 reply; 26+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-24 17:03 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Danilo Krummrich,
	Dmitry Torokhov, Thomas Gleixner, Peter Zijlstra, linux-acpi,
	linux-kernel, linux-pm, linux-input, kernel, superm1

On Fri, Nov 07, 2025 at 11:44:29PM +0500, Muhammad Usama Anjum wrote:
> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
> called when power button is pressed. The system is worken up from s2idle
> in this case by setting hard parameter to pm_wakeup_dev_event().
> 
> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
> is in progress. Set the hard parameter such that pm_system_wakeup()
> gets called which increments pm_abort_suspend counter. The explicit call
> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
> wakeup source. Hence call to input_report_key() with input device
> doesn't call pm_system_wakeup() as it doesn't have wakeup source
> registered.
> 
> Hence hibernation would be cancelled as in hibernation path, this counter
> is checked if it should be aborted.
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

This could be dangerous, as this is not what happens today, are you sure
that people aren't just used to pressing the button multiple times until
the system is hibernated?  If so, that would now break with this change
as it's hard to determine what is going on.

And why does hibernate take so long?  Why not fix that up instead?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-07 18:44 ` [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation Muhammad Usama Anjum
  2025-11-24 17:03   ` Greg Kroah-Hartman
@ 2025-11-24 18:42   ` Rafael J. Wysocki
  2025-11-28 14:17     ` Muhammad Usama Anjum
  1 sibling, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-24 18:42 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
>
> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
> called when power button is pressed. The system is worken up from s2idle
> in this case by setting hard parameter to pm_wakeup_dev_event().

Right, so presumably you want to set it for hibernation too.

> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
> is in progress.

Well, this is not what the code does after the change.

> Set the hard parameter such that pm_system_wakeup()
> gets called which increments pm_abort_suspend counter. The explicit call
> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
> wakeup source. Hence call to input_report_key() with input device
> doesn't call pm_system_wakeup() as it doesn't have wakeup source
> registered.
>
> Hence hibernation would be cancelled as in hibernation path, this counter
> is checked if it should be aborted.
>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since RFC:
> - Use pm_sleep_transition_in_progress()
> - Update descriptin why explicit call to acpi_pm_wakeup_event() is
>   necessary
> ---
>  drivers/acpi/button.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 3c6dd9b4ba0ad..e4be5c763edaf 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -20,6 +20,7 @@
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
>  #include <acpi/button.h>
> +#include <linux/suspend.h>
>
>  #define ACPI_BUTTON_CLASS              "button"
>  #define ACPI_BUTTON_FILE_STATE         "state"
> @@ -458,11 +459,16 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
>         acpi_pm_wakeup_event(&device->dev);

The above is what you want to change, as this already reports the
event.  Reporting it twice is unnecessary and potentially confusing.

>         button = acpi_driver_data(device);
> -       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
> -               return;
> -
>         input = button->input;
>         keycode = test_bit(KEY_SLEEP, input->keybit) ? KEY_SLEEP : KEY_POWER;
> +       if (event == ACPI_BUTTON_NOTIFY_STATUS && keycode == KEY_POWER &&
> +           pm_sleep_transition_in_progress()) {
> +               pm_wakeup_dev_event(&device->dev, 0, true);
> +               return;
> +       }

First, this will affect suspend too.

Second, this reports an already reported wakeup event.

Next, why KEY_POWER only?  Is KEY_SLEEP not expected to wake up?

And why event == ACPI_BUTTON_NOTIFY_STATUS?  Isn't this what
ACPI_BUTTON_NOTIFY_WAKE is for?

> +
> +       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
> +               return;
>
>         input_report_key(input, keycode, 1);
>         input_sync(input);
> --

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress
  2025-11-07 18:44 ` [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress Muhammad Usama Anjum
@ 2025-11-24 18:50   ` Rafael J. Wysocki
  2025-11-25 10:22     ` Muhammad Usama Anjum
  0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-24 18:50 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
>
> Input (Serio) drivers call input_handle_event(). Although the serio
> drivers have duplicate events, they have separate code path and call
> input_handle_event(). Ignore the KEY_POWER such that this event isn't
> sent to the userspace if hibernation is in progress.

Your change affects suspend too.

Also, what's the goal you want to achieve?

> Abort the hibernation by calling pm_wakeup_dev_event(). In case of serio,
> doesn't have wakeup source registered, this call doesn't do anything.
> But there may be other input drivers which will require this.
>
> Without this, the event is sent to the userspace and it suspends the
> device after hibernation cancellation.

I think that's because user space handles it this way, isn't it?

> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since RFC:
> - Use pm_sleep_transition_in_progress()
> - Update description
> ---
>  drivers/input/input.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index a500e1e276c21..7939bd9e47668 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -26,6 +26,7 @@
>  #include <linux/kstrtox.h>
>  #include <linux/mutex.h>
>  #include <linux/rcupdate.h>
> +#include <linux/suspend.h>
>  #include "input-compat.h"
>  #include "input-core-private.h"
>  #include "input-poller.h"
> @@ -362,6 +363,11 @@ void input_handle_event(struct input_dev *dev,
>
>         lockdep_assert_held(&dev->event_lock);
>
> +       if (code == KEY_POWER && pm_sleep_transition_in_progress()) {
> +               pm_wakeup_dev_event(&dev->dev, 0, true);
> +               return;
> +       }
> +
>         disposition = input_get_disposition(dev, type, code, &value);
>         if (disposition != INPUT_IGNORE_EVENT) {
>                 if (type != EV_SYN)
> --

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend
  2025-11-07 18:44 ` [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend Muhammad Usama Anjum
@ 2025-11-24 18:54   ` Rafael J. Wysocki
  2025-11-25  9:53     ` Muhammad Usama Anjum
  0 siblings, 1 reply; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-24 18:54 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
<usama.anjum@collabora.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@collabora.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?

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend
  2025-11-24 18:54   ` Rafael J. Wysocki
@ 2025-11-25  9:53     ` Muhammad Usama Anjum
  2025-11-25 12:59       ` Rafael J. Wysocki
  0 siblings, 1 reply; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-25  9:53 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: usama.anjum, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

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@collabora.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@collabora.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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress
  2025-11-24 18:50   ` Rafael J. Wysocki
@ 2025-11-25 10:22     ` Muhammad Usama Anjum
  2025-11-25 12:25       ` Rafael J. Wysocki
  0 siblings, 1 reply; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-25 10:22 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: usama.anjum, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On 11/24/25 11:50 PM, Rafael J. Wysocki wrote:
> On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> Input (Serio) drivers call input_handle_event(). Although the serio
>> drivers have duplicate events, they have separate code path and call
>> input_handle_event(). Ignore the KEY_POWER such that this event isn't
>> sent to the userspace if hibernation is in progress.
> 
> Your change affects suspend too.
> 
> Also, what's the goal you want to achieve?
Two goals:
* Don't send event to userspace
* Set pm_wakeup for hibernation cancellation for non-acpi devices (This api
  call should be tested on non-acpi devices such as arm board to see if it
  helps. I don't have an arm board in hand)

> 
>> Abort the hibernation by calling pm_wakeup_dev_event(). In case of serio,
>> doesn't have wakeup source registered, this call doesn't do anything.
>> But there may be other input drivers which will require this.
>>
>> Without this, the event is sent to the userspace and it suspends the
>> device after hibernation cancellation.
> 
> I think that's because user space handles it this way, isn't it?
Yes, it depends on how userspace handles such events. There are different settings
configured for systemd-logind when power event is received. The purpose is to consume
this event to cancel the hibernation without letting userspace know about it.

Thinking more about it, I wasn't sure if all of such events are compulsory to be
delivered to userspace. But then I found an example: In acpi_button_notify(), all
such events are not sent to userspace if button is suspended. So it seems okay to
not send this as well and just consume in the kernel.

> 
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since RFC:
>> - Use pm_sleep_transition_in_progress()
>> - Update description
>> ---
>>  drivers/input/input.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/input/input.c b/drivers/input/input.c
>> index a500e1e276c21..7939bd9e47668 100644
>> --- a/drivers/input/input.c
>> +++ b/drivers/input/input.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/kstrtox.h>
>>  #include <linux/mutex.h>
>>  #include <linux/rcupdate.h>
>> +#include <linux/suspend.h>
>>  #include "input-compat.h"
>>  #include "input-core-private.h"
>>  #include "input-poller.h"
>> @@ -362,6 +363,11 @@ void input_handle_event(struct input_dev *dev,
>>
>>         lockdep_assert_held(&dev->event_lock);
>>
>> +       if (code == KEY_POWER && pm_sleep_transition_in_progress()) {
>> +               pm_wakeup_dev_event(&dev->dev, 0, true);
>> +               return;
>> +       }
>> +
>>         disposition = input_get_disposition(dev, type, code, &value);
>>         if (disposition != INPUT_IGNORE_EVENT) {
>>                 if (type != EV_SYN)
>> --


-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-24 17:03   ` Greg Kroah-Hartman
@ 2025-11-25 11:12     ` Muhammad Usama Anjum
  2025-11-25 11:47       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-25 11:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: usama.anjum, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

Hi Greg,

Thank you for the review.

On 11/24/25 10:03 PM, Greg Kroah-Hartman wrote:
> On Fri, Nov 07, 2025 at 11:44:29PM +0500, Muhammad Usama Anjum wrote:
>> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
>> called when power button is pressed. The system is worken up from s2idle
>> in this case by setting hard parameter to pm_wakeup_dev_event().
>>
>> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
>> is in progress. Set the hard parameter such that pm_system_wakeup()
>> gets called which increments pm_abort_suspend counter. The explicit call
>> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
>> wakeup source. Hence call to input_report_key() with input device
>> doesn't call pm_system_wakeup() as it doesn't have wakeup source
>> registered.
>>
>> Hence hibernation would be cancelled as in hibernation path, this counter
>> is checked if it should be aborted.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>
My thinking is that people don't press power button after triggering
hibernation. They will only press power button if they want to cancel the
hibernation or resume from hibernation a bit later when hibernation completes. 
> This could be dangerous, as this is not what happens today, are you sure
> that people aren't just used to pressing the button multiple times until
> the system is hibernated? If so, that would now break with this change
> as it's hard to determine what is going on.
Yes, its possible. Previously the device wouldn't cancel hibernation on power
button press; while now it'll cancel.

So should we put this cancellation under some config option to avoid breaking
the default behavior?

> 
> And why does hibernate take so long?  Why not fix that up instead?
Hibernation is inherently slow: it must freeze devices, copy and
compress/encrypt memory, then resume storage devices to write the image to
disk.

While I've thought about increasing the speed, I've no concrete ideas yet.
The main problem is that its sequential in nature.

> 
> thanks,
> 
> greg k-h


-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-25 11:12     ` Muhammad Usama Anjum
@ 2025-11-25 11:47       ` Greg Kroah-Hartman
  2025-11-25 16:41         ` Muhammad Usama Anjum
  0 siblings, 1 reply; 26+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-25 11:47 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Danilo Krummrich,
	Dmitry Torokhov, Thomas Gleixner, Peter Zijlstra, linux-acpi,
	linux-kernel, linux-pm, linux-input, kernel, superm1

On Tue, Nov 25, 2025 at 04:12:54PM +0500, Muhammad Usama Anjum wrote:
> Hi Greg,
> 
> Thank you for the review.
> 
> On 11/24/25 10:03 PM, Greg Kroah-Hartman wrote:
> > On Fri, Nov 07, 2025 at 11:44:29PM +0500, Muhammad Usama Anjum wrote:
> >> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
> >> called when power button is pressed. The system is worken up from s2idle
> >> in this case by setting hard parameter to pm_wakeup_dev_event().
> >>
> >> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
> >> is in progress. Set the hard parameter such that pm_system_wakeup()
> >> gets called which increments pm_abort_suspend counter. The explicit call
> >> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
> >> wakeup source. Hence call to input_report_key() with input device
> >> doesn't call pm_system_wakeup() as it doesn't have wakeup source
> >> registered.
> >>
> >> Hence hibernation would be cancelled as in hibernation path, this counter
> >> is checked if it should be aborted.
> >>
> >> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> >
> My thinking is that people don't press power button after triggering
> hibernation. They will only press power button if they want to cancel the
> hibernation or resume from hibernation a bit later when hibernation completes. 
> > This could be dangerous, as this is not what happens today, are you sure
> > that people aren't just used to pressing the button multiple times until
> > the system is hibernated? If so, that would now break with this change
> > as it's hard to determine what is going on.
> Yes, its possible. Previously the device wouldn't cancel hibernation on power
> button press; while now it'll cancel.
> 
> So should we put this cancellation under some config option to avoid breaking
> the default behavior?

Do not add another config option, that way lies madness.  As proof, what
would your distro select for this, in order to preserve old behavior?  :)

> > And why does hibernate take so long?  Why not fix that up instead?
> Hibernation is inherently slow: it must freeze devices, copy and
> compress/encrypt memory, then resume storage devices to write the image to
> disk.
> 
> While I've thought about increasing the speed, I've no concrete ideas yet.
> The main problem is that its sequential in nature.

Then fix that?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress
  2025-11-25 10:22     ` Muhammad Usama Anjum
@ 2025-11-25 12:25       ` Rafael J. Wysocki
  2025-11-25 16:05         ` Muhammad Usama Anjum
  2025-11-28 17:00         ` Muhammad Usama Anjum
  0 siblings, 2 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-25 12:25 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On Tue, Nov 25, 2025 at 11:23 AM Muhammad Usama Anjum
<usama.anjum@collabora.com> wrote:
>
> On 11/24/25 11:50 PM, Rafael J. Wysocki wrote:
> > On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
> > <usama.anjum@collabora.com> wrote:
> >>
> >> Input (Serio) drivers call input_handle_event(). Although the serio
> >> drivers have duplicate events, they have separate code path and call
> >> input_handle_event(). Ignore the KEY_POWER such that this event isn't
> >> sent to the userspace if hibernation is in progress.
> >
> > Your change affects suspend too.
> >
> > Also, what's the goal you want to achieve?
> Two goals:
> * Don't send event to userspace
> * Set pm_wakeup for hibernation cancellation for non-acpi devices (This api
>   call should be tested on non-acpi devices such as arm board to see if it
>   helps. I don't have an arm board in hand)
>
> >
> >> Abort the hibernation by calling pm_wakeup_dev_event(). In case of serio,
> >> doesn't have wakeup source registered, this call doesn't do anything.
> >> But there may be other input drivers which will require this.
> >>
> >> Without this, the event is sent to the userspace and it suspends the
> >> device after hibernation cancellation.
> >
> > I think that's because user space handles it this way, isn't it?
>
> Yes, it depends on how userspace handles such events. There are different settings
> configured for systemd-logind when power event is received. The purpose is to consume
> this event to cancel the hibernation without letting userspace know about it.
>
> Thinking more about it, I wasn't sure if all of such events are compulsory to be
> delivered to userspace. But then I found an example: In acpi_button_notify(), all
> such events are not sent to userspace if button is suspended. So it seems okay to
> not send this as well and just consume in the kernel.

You want the given key (and it doesn't matter whether or not this is
KEY_POWER or something else) to play two roles.  One of them is to
send a specific key code to user space and let it handle the keypress
as it wants.  This is how it works most of the time.  The second one
is to wake up the system from sleep (and I'm not sure why you want to
limit this to hibernation) in which case the key code will not be sent
to user space.

For this to work, you need to switch between the two modes quite
precisely and checking things like pm_sleep_transition_in_progress()
(which is only used for debug and its raciness is not relevant there)
is not sufficient for this purpose.  That's what the "suspended" flag
in the ACPI button driver is for.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend
  2025-11-25  9:53     ` Muhammad Usama Anjum
@ 2025-11-25 12:59       ` Rafael J. Wysocki
  0 siblings, 0 replies; 26+ messages in thread
From: Rafael J. Wysocki @ 2025-11-25 12:59 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On Tue, Nov 25, 2025 at 10:54 AM Muhammad Usama Anjum
<usama.anjum@collabora.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@collabora.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@collabora.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).

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress
  2025-11-25 12:25       ` Rafael J. Wysocki
@ 2025-11-25 16:05         ` Muhammad Usama Anjum
  2025-11-28 17:00         ` Muhammad Usama Anjum
  1 sibling, 0 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-25 16:05 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: usama.anjum, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On 11/25/25 5:25 PM, Rafael J. Wysocki wrote:
> On Tue, Nov 25, 2025 at 11:23 AM Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> On 11/24/25 11:50 PM, Rafael J. Wysocki wrote:
>>> On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
>>> <usama.anjum@collabora.com> wrote:
>>>>
>>>> Input (Serio) drivers call input_handle_event(). Although the serio
>>>> drivers have duplicate events, they have separate code path and call
>>>> input_handle_event(). Ignore the KEY_POWER such that this event isn't
>>>> sent to the userspace if hibernation is in progress.
>>>
>>> Your change affects suspend too.
>>>
>>> Also, what's the goal you want to achieve?
>> Two goals:
>> * Don't send event to userspace
>> * Set pm_wakeup for hibernation cancellation for non-acpi devices (This api
>>   call should be tested on non-acpi devices such as arm board to see if it
>>   helps. I don't have an arm board in hand)
>>
>>>
>>>> Abort the hibernation by calling pm_wakeup_dev_event(). In case of serio,
>>>> doesn't have wakeup source registered, this call doesn't do anything.
>>>> But there may be other input drivers which will require this.
>>>>
>>>> Without this, the event is sent to the userspace and it suspends the
>>>> device after hibernation cancellation.
>>>
>>> I think that's because user space handles it this way, isn't it?
>>
>> Yes, it depends on how userspace handles such events. There are different settings
>> configured for systemd-logind when power event is received. The purpose is to consume
>> this event to cancel the hibernation without letting userspace know about it.
>>
>> Thinking more about it, I wasn't sure if all of such events are compulsory to be
>> delivered to userspace. But then I found an example: In acpi_button_notify(), all
>> such events are not sent to userspace if button is suspended. So it seems okay to
>> not send this as well and just consume in the kernel.
> 
> You want the given key (and it doesn't matter whether or not this is
> KEY_POWER or something else) to play two roles.  One of them is to
> send a specific key code to user space and let it handle the keypress
> as it wants.  This is how it works most of the time.  The second one
> is to wake up the system from sleep (and I'm not sure why you want to
> limit this to hibernation) in which case the key code will not be sent
> to user space.
> 
> For this to work, you need to switch between the two modes quite
> precisely and checking things like pm_sleep_transition_in_progress()
> (which is only used for debug and its raciness is not relevant there)
> is not sufficient for this purpose.  That's what the "suspended" flag
> in the ACPI button driver is for.
I see. I'll add a suspended flag just like the acpi button and use it here.
Thank you so much for explaining.

-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-25 11:47       ` Greg Kroah-Hartman
@ 2025-11-25 16:41         ` Muhammad Usama Anjum
  2025-11-26  7:38           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-25 16:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: usama.anjum, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On 11/25/25 4:47 PM, Greg Kroah-Hartman wrote:
> On Tue, Nov 25, 2025 at 04:12:54PM +0500, Muhammad Usama Anjum wrote:
>> Hi Greg,
>>
>> Thank you for the review.
>>
>> On 11/24/25 10:03 PM, Greg Kroah-Hartman wrote:
>>> On Fri, Nov 07, 2025 at 11:44:29PM +0500, Muhammad Usama Anjum wrote:
>>>> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
>>>> called when power button is pressed. The system is worken up from s2idle
>>>> in this case by setting hard parameter to pm_wakeup_dev_event().
>>>>
>>>> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
>>>> is in progress. Set the hard parameter such that pm_system_wakeup()
>>>> gets called which increments pm_abort_suspend counter. The explicit call
>>>> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
>>>> wakeup source. Hence call to input_report_key() with input device
>>>> doesn't call pm_system_wakeup() as it doesn't have wakeup source
>>>> registered.
>>>>
>>>> Hence hibernation would be cancelled as in hibernation path, this counter
>>>> is checked if it should be aborted.
>>>>
>>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>>
>> My thinking is that people don't press power button after triggering
>> hibernation. They will only press power button if they want to cancel the
>> hibernation or resume from hibernation a bit later when hibernation completes. 
>>> This could be dangerous, as this is not what happens today, are you sure
>>> that people aren't just used to pressing the button multiple times until
>>> the system is hibernated? If so, that would now break with this change
>>> as it's hard to determine what is going on.
>> Yes, its possible. Previously the device wouldn't cancel hibernation on power
>> button press; while now it'll cancel.
>>
>> So should we put this cancellation under some config option to avoid breaking
>> the default behavior?
> 
> Do not add another config option, that way lies madness.  As proof, what
> would your distro select for this, in order to preserve old behavior?  :)
I think, the new behavior would be desirable by most distros. They don't care
about the old behavior. But its just my thinking. What do you think is the way forward?

Even if there are users which used to pressing power button during hibernation,
will not press it after a few tries if they really want the hibernation to complete.

> 
>>> And why does hibernate take so long?  Why not fix that up instead?
>> Hibernation is inherently slow: it must freeze devices, copy and
>> compress/encrypt memory, then resume storage devices to write the image to
>> disk.
>>
>> While I've thought about increasing the speed, I've no concrete ideas yet.
>> The main problem is that its sequential in nature.
> 
> Then fix that?
That's in the plan. But who knows when we get time to attempt that. 

First I need a board/machine with serial console access to view all logs in real
time. :)

---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress()
  2025-11-24 17:01   ` Greg Kroah-Hartman
@ 2025-11-25 16:44     ` Muhammad Usama Anjum
  0 siblings, 0 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-25 16:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: usama.anjum, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On 11/24/25 10:01 PM, Greg Kroah-Hartman wrote:
> On Fri, Nov 07, 2025 at 11:44:28PM +0500, Muhammad Usama Anjum wrote:
>> Export pm_sleep_transition_in_progress() to be used by other
>> modules.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>  kernel/power/main.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/power/main.c b/kernel/power/main.c
>> index 33a47ed15994f..ff3354b5be150 100644
>> --- a/kernel/power/main.c
>> +++ b/kernel/power/main.c
>> @@ -570,6 +570,7 @@ bool pm_sleep_transition_in_progress(void)
>>  {
>>  	return pm_suspend_in_progress() || hibernation_in_progress();
>>  }
>> +EXPORT_SYMBOL_GPL(pm_sleep_transition_in_progress);
> 
> The problem is, you can not rely on the result of this call as it could
> change right after you call it, right?
> 
> So who needs to call this and why?  What new workflow requires this?
Yeah, Rafael just mentioned as well that this API isn't dependable. I need
to use better flag for detecting if driver is suspended. I'm testing if
suspended flag in ACPI button driver is enough and similar flag can be added
in input driver.

> 
> thanks,
> 
> greg k-h


-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-25 16:41         ` Muhammad Usama Anjum
@ 2025-11-26  7:38           ` Greg Kroah-Hartman
  2025-11-26 12:55             ` Mario Limonciello
  2025-11-26 13:41             ` Muhammad Usama Anjum
  0 siblings, 2 replies; 26+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-26  7:38 UTC (permalink / raw)
  To: Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Danilo Krummrich,
	Dmitry Torokhov, Thomas Gleixner, Peter Zijlstra, linux-acpi,
	linux-kernel, linux-pm, linux-input, kernel, superm1

On Tue, Nov 25, 2025 at 09:41:22PM +0500, Muhammad Usama Anjum wrote:
> >> While I've thought about increasing the speed, I've no concrete ideas yet.
> >> The main problem is that its sequential in nature.
> > 
> > Then fix that?
> That's in the plan. But who knows when we get time to attempt that. 

Take the time to fix this properly first, don't paper over the issue by
changing user/system interactions that will not be needed in the future
when the real problem is resolved.

> First I need a board/machine with serial console access to view all logs in real
> time. :)

usb debug cables might be your solution.

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-26  7:38           ` Greg Kroah-Hartman
@ 2025-11-26 12:55             ` Mario Limonciello
  2025-11-26 13:26               ` Muhammad Usama Anjum
  2025-11-26 13:41             ` Muhammad Usama Anjum
  1 sibling, 1 reply; 26+ messages in thread
From: Mario Limonciello @ 2025-11-26 12:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Muhammad Usama Anjum
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Danilo Krummrich,
	Dmitry Torokhov, Thomas Gleixner, Peter Zijlstra, linux-acpi,
	linux-kernel, linux-pm, linux-input, kernel, Askar Safin

>> First I need a board/machine with serial console access to view all logs in real
>> time. :)
> 
> usb debug cables might be your solution.
> 
Just a word of warning before you go too far down this path to get a 
console working from XHCI debug.

This is probably a Hal changing a light bulb problem [1]. Last time I 
tried XHCI debug cables on some modern AMD systems I ran into a problem 
that the BAR is too big for early_ioremap().  Looking through LKML it's 
come up a few times in the past too [2] [3].

Link: https://youtu.be/5W4NFcamRhM?si=qOFrCTzvK6-H-4AX [1]
Link: https://lore.kernel.org/linux-usb/ZCOq3PUBCtHkwdnw@mail-itl/ [2]
Link: 
https://lore.kernel.org/linux-usb/CAAcb1K_bezseTM8DrOrzVUi_W+nZoE2N0CO4k3AQWPw7=7pyjw@mail.gmail.com/ 
[3]

The other obvious idea is to use netconsole, but you need a PCIe 
Ethernet controller, but I think you'll have more success in development 
using KVM as these are generic architectural problems.

To help you get started with this I may point out something that was 
shared to me for another hibernate bug [4].

Link: 
https://lore.kernel.org/linux-pm/20251105180506.137448-1-safinaskar@gmail.com/ 
[4]

Askar Safin (CC'ed) produced a script that does a very minimal kernel 
build, sets up a VM with the right sizes of disks/swap/etc.  It's 
trivial to make kernel changes and re-run the script, and you can  also 
attach a debugger to the KVM instance.  Maybe you can adapt something 
like that.  You can wrap it with 'time' calls to actually measure 
performance for any ideas and prove them out too.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-26 12:55             ` Mario Limonciello
@ 2025-11-26 13:26               ` Muhammad Usama Anjum
  0 siblings, 0 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-26 13:26 UTC (permalink / raw)
  To: Mario Limonciello, Greg Kroah-Hartman
  Cc: usama.anjum, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, Askar Safin

On 11/26/25 5:55 PM, Mario Limonciello wrote:
>>> First I need a board/machine with serial console access to view all logs in real
>>> time. :)
>>
>> usb debug cables might be your solution.
>>
> Just a word of warning before you go too far down this path to get a console working from XHCI debug.
> 
> This is probably a Hal changing a light bulb problem [1]. Last time I tried XHCI debug cables on some modern AMD systems I ran into a problem that the BAR is too big for early_ioremap().  Looking through LKML it's come up a few times in the past too [2] [3].
> 
> Link: https://youtu.be/5W4NFcamRhM?si=qOFrCTzvK6-H-4AX [1]
> Link: https://lore.kernel.org/linux-usb/ZCOq3PUBCtHkwdnw@mail-itl/ [2]
> Link: https://lore.kernel.org/linux-usb/CAAcb1K_bezseTM8DrOrzVUi_W+nZoE2N0CO4k3AQWPw7=7pyjw@mail.gmail.com/ [3]
> 
> The other obvious idea is to use netconsole, but you need a PCIe Ethernet controller, but I think you'll have more success in development using KVM as these are generic architectural problems.
> 
> To help you get started with this I may point out something that was shared to me for another hibernate bug [4].
> 
> Link: https://lore.kernel.org/linux-pm/20251105180506.137448-1-safinaskar@gmail.com/ [4]
> 
> Askar Safin (CC'ed) produced a script that does a very minimal kernel build, sets up a VM with the right sizes of disks/swap/etc.  It's trivial to make kernel changes and re-run the script, and you can  also attach a debugger to the KVM instance.  Maybe you can adapt something like that.  You can wrap it with 'time' calls to actually measure performance for any ideas and prove them out too.
I was just going to try it. Thank you so much for double confirming. I'll
test and see.

-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-26  7:38           ` Greg Kroah-Hartman
  2025-11-26 12:55             ` Mario Limonciello
@ 2025-11-26 13:41             ` Muhammad Usama Anjum
  1 sibling, 0 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-26 13:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: usama.anjum, Rafael J. Wysocki, Len Brown, Pavel Machek,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On 11/26/25 12:38 PM, Greg Kroah-Hartman wrote:
> On Tue, Nov 25, 2025 at 09:41:22PM +0500, Muhammad Usama Anjum wrote:
>>>> While I've thought about increasing the speed, I've no concrete ideas yet.
>>>> The main problem is that its sequential in nature.
>>>
>>> Then fix that?
>> That's in the plan. But who knows when we get time to attempt that. 
> 
> Take the time to fix this properly first, don't paper over the issue by
> changing user/system interactions that will not be needed in the future
> when the real problem is resolved.
You're absolutely right, and I share the same philosophy.

However, I think the hibernation cancellation feature has standalone value
regardless of how much we optimize hibernation time. Even if we achieve
significant improvements (5-30% or more), there will still be scenarios where
users want to abort an in-progress hibernation.

I'm focusing on making this series more concise for next revision.

> 
>> First I need a board/machine with serial console access to view all logs in real
>> time. :)
> 
> usb debug cables might be your solution.
> 
> good luck!
> 
> greg k-h


-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation
  2025-11-24 18:42   ` Rafael J. Wysocki
@ 2025-11-28 14:17     ` Muhammad Usama Anjum
  0 siblings, 0 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-28 14:17 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: usama.anjum, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On 11/24/25 11:42 PM, Rafael J. Wysocki wrote:
> On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
>> called when power button is pressed. The system is worken up from s2idle
>> in this case by setting hard parameter to pm_wakeup_dev_event().
> 
> Right, so presumably you want to set it for hibernation too.
> 
>> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
>> is in progress.
> 
> Well, this is not what the code does after the change.
> 
>> Set the hard parameter such that pm_system_wakeup()
>> gets called which increments pm_abort_suspend counter. The explicit call
>> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
>> wakeup source. Hence call to input_report_key() with input device
>> doesn't call pm_system_wakeup() as it doesn't have wakeup source
>> registered.
>>
>> Hence hibernation would be cancelled as in hibernation path, this counter
>> is checked if it should be aborted.
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since RFC:
>> - Use pm_sleep_transition_in_progress()
>> - Update descriptin why explicit call to acpi_pm_wakeup_event() is
>>   necessary
>> ---
>>  drivers/acpi/button.c | 12 +++++++++---
>>  1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
>> index 3c6dd9b4ba0ad..e4be5c763edaf 100644
>> --- a/drivers/acpi/button.c
>> +++ b/drivers/acpi/button.c
>> @@ -20,6 +20,7 @@
>>  #include <linux/acpi.h>
>>  #include <linux/dmi.h>
>>  #include <acpi/button.h>
>> +#include <linux/suspend.h>
>>
>>  #define ACPI_BUTTON_CLASS              "button"
>>  #define ACPI_BUTTON_FILE_STATE         "state"
>> @@ -458,11 +459,16 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
>>         acpi_pm_wakeup_event(&device->dev);
> 
> The above is what you want to change, as this already reports the
> event.  Reporting it twice is unnecessary and potentially confusing.
Thanks for mentioning. This will be fixed easily by adding newer
version of acpi_pm_wakeup_event() which takes hard parameter.

> 
>>         button = acpi_driver_data(device);
>> -       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
>> -               return;
>> -
>>         input = button->input;
>>         keycode = test_bit(KEY_SLEEP, input->keybit) ? KEY_SLEEP : KEY_POWER;
>> +       if (event == ACPI_BUTTON_NOTIFY_STATUS && keycode == KEY_POWER &&
>> +           pm_sleep_transition_in_progress()) {
>> +               pm_wakeup_dev_event(&device->dev, 0, true);
>> +               return;
>> +       }
> 
> First, this will affect suspend too.
I'll fix terminologies in entire series.

> 
> Second, this reports an already reported wakeup event.
As mentioned above, I'll update to only 1 call to pm_wakeup_dev_event().> 
> Next, why KEY_POWER only?  Is KEY_SLEEP not expected to wake up?
Yes, this is true. This will be fixed in next version.

> 
> And why event == ACPI_BUTTON_NOTIFY_STATUS?  Isn't this what
> ACPI_BUTTON_NOTIFY_WAKE is for?
While device is hibernating, at that if power button is pressed, 
ACPI_BUTTON_NOTIFY_STATUS event is received. 

ACPI_BUTTON_NOTIFY_WAKE is received when we turn on the device from sleeping state
(suspend) or turned off state (hibernated).

> 
>> +
>> +       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
>> +               return;
>>
>>         input_report_key(input, keycode, 1);
>>         input_sync(input);
>> --


-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress
  2025-11-25 12:25       ` Rafael J. Wysocki
  2025-11-25 16:05         ` Muhammad Usama Anjum
@ 2025-11-28 17:00         ` Muhammad Usama Anjum
  1 sibling, 0 replies; 26+ messages in thread
From: Muhammad Usama Anjum @ 2025-11-28 17:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: usama.anjum, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Danilo Krummrich, Dmitry Torokhov, Thomas Gleixner,
	Peter Zijlstra, linux-acpi, linux-kernel, linux-pm, linux-input,
	kernel, superm1

On 11/25/25 5:25 PM, Rafael J. Wysocki wrote:
> On Tue, Nov 25, 2025 at 11:23 AM Muhammad Usama Anjum
> <usama.anjum@collabora.com> wrote:
>>
>> On 11/24/25 11:50 PM, Rafael J. Wysocki wrote:
>>> On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
>>> <usama.anjum@collabora.com> wrote:
>>>>
>>>> Input (Serio) drivers call input_handle_event(). Although the serio
>>>> drivers have duplicate events, they have separate code path and call
>>>> input_handle_event(). Ignore the KEY_POWER such that this event isn't
>>>> sent to the userspace if hibernation is in progress.
>>>
>>> Your change affects suspend too.
>>>
>>> Also, what's the goal you want to achieve?
>> Two goals:
>> * Don't send event to userspace
>> * Set pm_wakeup for hibernation cancellation for non-acpi devices (This api
>>   call should be tested on non-acpi devices such as arm board to see if it
>>   helps. I don't have an arm board in hand)
>>
>>>
>>>> Abort the hibernation by calling pm_wakeup_dev_event(). In case of serio,
>>>> doesn't have wakeup source registered, this call doesn't do anything.
>>>> But there may be other input drivers which will require this.
>>>>
>>>> Without this, the event is sent to the userspace and it suspends the
>>>> device after hibernation cancellation.
>>>
>>> I think that's because user space handles it this way, isn't it?
>>
>> Yes, it depends on how userspace handles such events. There are different settings
>> configured for systemd-logind when power event is received. The purpose is to consume
>> this event to cancel the hibernation without letting userspace know about it.
>>
>> Thinking more about it, I wasn't sure if all of such events are compulsory to be
>> delivered to userspace. But then I found an example: In acpi_button_notify(), all
>> such events are not sent to userspace if button is suspended. So it seems okay to
>> not send this as well and just consume in the kernel.
> 
> You want the given key (and it doesn't matter whether or not this is
> KEY_POWER or something else) to play two roles.  One of them is to
> send a specific key code to user space and let it handle the keypress
> as it wants.  This is how it works most of the time.  The second one
> is to wake up the system from sleep (and I'm not sure why you want to
> limit this to hibernation) in which case the key code will not be sent
> to user space.
> 
> For this to work, you need to switch between the two modes quite
> precisely and checking things like pm_sleep_transition_in_progress()
> (which is only used for debug and its raciness is not relevant there)
> is not sufficient for this purpose.  That's what the "suspended" flag
> in the ACPI button driver is for.
I've been testing and trying out `suspended` flag. But this flag gets set very late.
If we depend on it, we'll not be able to wakeup in time after cancelling hibernation.

Initially we were using hibernation_in_progress() in RFC and we switched to
pm_sleep_transition_in_progress() in order to cancel the sleep as well (which wasn't 
the original intention).

The sleep detection isn't working through pm_suspend_target_state or pm_suspend_in_progress()
as it is set very late in suspend process. While hibernation_in_progress() gets set in
start of hibernation.

Then as you said, they are unreliable. I'm thinking what other options. But I've not
found any. Please share ideas what other way can work instead of suspended flag better? 

-- 
---
Thanks,
Usama

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2025-11-28 17:01 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07 18:44 [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum
2025-11-07 18:44 ` [PATCH 1/4] PM: hibernate: export pm_sleep_transition_in_progress() Muhammad Usama Anjum
2025-11-24 17:01   ` Greg Kroah-Hartman
2025-11-25 16:44     ` Muhammad Usama Anjum
2025-11-07 18:44 ` [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation Muhammad Usama Anjum
2025-11-24 17:03   ` Greg Kroah-Hartman
2025-11-25 11:12     ` Muhammad Usama Anjum
2025-11-25 11:47       ` Greg Kroah-Hartman
2025-11-25 16:41         ` Muhammad Usama Anjum
2025-11-26  7:38           ` Greg Kroah-Hartman
2025-11-26 12:55             ` Mario Limonciello
2025-11-26 13:26               ` Muhammad Usama Anjum
2025-11-26 13:41             ` Muhammad Usama Anjum
2025-11-24 18:42   ` Rafael J. Wysocki
2025-11-28 14:17     ` Muhammad Usama Anjum
2025-11-07 18:44 ` [PATCH 3/4] Input: Ignore the KEY_POWER events if hibernation is in progress Muhammad Usama Anjum
2025-11-24 18:50   ` Rafael J. Wysocki
2025-11-25 10:22     ` Muhammad Usama Anjum
2025-11-25 12:25       ` Rafael J. Wysocki
2025-11-25 16:05         ` Muhammad Usama Anjum
2025-11-28 17:00         ` Muhammad Usama Anjum
2025-11-07 18:44 ` [PATCH 4/4] PM: sleep: clear pm_abort_suspend at suspend Muhammad Usama Anjum
2025-11-24 18:54   ` Rafael J. Wysocki
2025-11-25  9:53     ` Muhammad Usama Anjum
2025-11-25 12:59       ` Rafael J. Wysocki
2025-11-24 13:03 ` [PATCH 0/4] PM: Hibernate: Add hibernation cancellation support Muhammad Usama Anjum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).