public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* Sync timeout mechanisms - Request for coordination
@ 2025-09-08  2:46 tuhaowen
  2025-09-09  2:00 ` Saravana Kannan
  0 siblings, 1 reply; 9+ messages in thread
From: tuhaowen @ 2025-09-08  2:46 UTC (permalink / raw)
  To: wusamuel, saravanak
  Cc: rafael, len.brown, pavel, linux-pm, linux-kernel, huangbibo

Hi Samuel and Saravana,

I hope this email finds you well. I'm reaching out regarding the sync
timeout mechanisms for suspend/hibernation that we've both been working on.

Rafael from the upstream kernel has indicated that he would prefer us to
coordinate our approaches rather than having two separate implementations.
He mentioned your patch series "PM: Support aborting suspend during
filesystem sync" and suggested we work together on a unified solution.

I would like to discuss how we can merge our approaches. Below is a
summary of my current implementation:

**My approach - Time-based timeout mechanism:**
- Introduces a configurable timeout for sync operations during both
  suspend and hibernation
- Uses kthread with wait_for_completion_timeout() to implement timeout
- Provides sysfs interface /sys/power/sleep_sync_timeout for runtime
  configuration  
- Default behavior unchanged (timeout disabled by default)
- Addresses scenarios where sync is excessively slow without wakeup events

You can see the detailed implementation and Rafael's feedback at:
https://lore.kernel.org/linux-pm/CAJZ5v0jBRy=CvZiWQQaorvc-zT+kkaB6+S2TErGmkaPAGmHLOQ@mail.gmail.com/

**Key differences I see between our approaches:**
1. Your solution focuses on aborting sync when wakeup events occur
2. My solution addresses cases where there are no wakeup events but sync
   is excessively slow (e.g., slow/faulty storage)
3. Your approach uses workqueue + completion, mine uses kthread + timeout
4. Both aim to prevent indefinite hangs but cover different scenarios

**Potential unified approach:**
I believe both mechanisms could complement each other:
- Event-driven abort (your approach) for responsive wakeup handling
- Time-based timeout (my approach) for proactive protection against
  slow storage
- Single, unified implementation in kernel/power/main.c

Would you be interested in discussing how we can combine these approaches?
I'm open to:
1. Merging the implementations into a single solution
2. Adopting your workqueue approach with added timeout functionality
3. Any other technical approach you think would work better

Looking forward to your thoughts and collaboration.

Best regards,
Haowen Tu

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH] PM: Add configurable sync timeout during suspend and hibernate to prevent hang
@ 2025-09-05  2:47 tuhaowen
  2025-09-05  9:24 ` [PATCH v2] PM: Add configurable sync timeout for suspend and hibernation tuhaowen
  0 siblings, 1 reply; 9+ messages in thread
From: tuhaowen @ 2025-09-05  2:47 UTC (permalink / raw)
  To: Rafael J . Wysocki, Len Brown, Pavel Machek
  Cc: linux-pm, linux-kernel, tuhaowen, huangbibo

When users initiate suspend (S3) or hibernate (S4) while large file
copy operations are in progress (especially to USB storage devices),
the system can appear to hang with a black screen for an extended period.
This occurs because the filesystem sync in the suspend/hibernate path
blocks until all pending I/O operations complete,
which can take several minutes for large file transfers or slow devices.

This patch introduces an optional, configurable timeout mechanism for
the sync operation during both suspend and hibernate.
If the timeout is reached, the operation is aborted,
and a clear error message is provided to the user, improving user
experience by preventing indefinite system hangs.

The timeout is disabled by default for both suspend and hibernate
to maintain backward compatibility. If enabled, the default
timeout value is 10000 ms (10 seconds), which can be changed
at runtime via module parameters:

- sync_on_suspend_timeout_enable (bool, default: false)
- sync_on_suspend_timeout_ms (uint, default: 10000)
- sync_on_hibernation_timeout_enable (bool, default: false)
- sync_on_hibernation_timeout_ms (uint, default: 10000)

Compared to [PATCH v3 0/3] PM: Support aborting suspend during
filesystem sync (see: https://lore.kernel.org/linux-pm/20250821004237.
2712312-1-wusamuel@google.com/), this patch addresses scenarios where
there may be no wakeup event, but the sync operation is excessively
slow (e.g., due to slow or faulty storage). By introducing a configurable
timeout, it proactively prevents indefinite hangs and improves user
experience in a wider range of real-world cases. The implementation
is also simpler and gives users or system integrators more flexibility
to tune behavior for different devices and requirements.

Signed-off-by: tuhaowen <tuhaowen@uniontech.com>
---
 kernel/power/hibernate.c | 55 ++++++++++++++++++++++++++++++++++++++-
 kernel/power/suspend.c   | 56 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 23c0f4e6c..2c173a0e5 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -33,10 +33,27 @@
 #include <linux/ktime.h>
 #include <linux/security.h>
 #include <linux/secretmem.h>
+#include <linux/moduleparam.h>
+#include <linux/completion.h>
+#include <linux/kthread.h>
+#include <linux/jiffies.h>
 #include <trace/events/power.h>
 
 #include "power.h"
 
+/* Sync timeout parameters for hibernation */
+static bool sync_on_hibernation_timeout_enable;
+module_param(sync_on_hibernation_timeout_enable, bool, 0644);
+MODULE_PARM_DESC(sync_on_hibernation_timeout_enable, "Enable sync timeout during hibernation (default: false)");
+
+static unsigned int sync_on_hibernation_timeout_ms = 10000;
+module_param(sync_on_hibernation_timeout_ms, uint, 0644);
+MODULE_PARM_DESC(sync_on_hibernation_timeout_ms, "Sync timeout in milliseconds during hibernation (default: 10000)");
+
+/* Sync timeout implementation for hibernation */
+static struct completion hibernation_sync_completion;
+static struct task_struct *hibernation_sync_task;
+
 
 static int nocompress;
 static int noresume;
@@ -97,6 +114,40 @@ bool hibernation_available(void)
 		!secretmem_active() && !cxl_mem_active();
 }
 
+static int hibernation_sync_thread_func(void *data)
+{
+	ksys_sync_helper();
+	complete(&hibernation_sync_completion);
+	return 0;
+}
+
+static int hibernation_sync_with_timeout(void)
+{
+	unsigned long timeout_jiffies;
+
+	if (!sync_on_hibernation_timeout_enable) {
+		ksys_sync_helper();
+		return 0;
+	}
+
+	init_completion(&hibernation_sync_completion);
+	hibernation_sync_task = kthread_run(hibernation_sync_thread_func, NULL, "hibernation_sync");
+	if (IS_ERR(hibernation_sync_task)) {
+		pr_warn("PM: hibernation: Failed to create sync thread, performing sync directly\n");
+		ksys_sync_helper();
+		return 0;
+	}
+
+	timeout_jiffies = msecs_to_jiffies(sync_on_hibernation_timeout_ms);
+	if (!wait_for_completion_timeout(&hibernation_sync_completion, timeout_jiffies)) {
+		pr_warn("PM: hibernation: Sync operation timed out after %u ms, aborting hibernation\n",
+				sync_on_hibernation_timeout_ms);
+		kthread_stop(hibernation_sync_task);
+		return -ETIMEDOUT;
+	}
+	return 0;
+}
+
 /**
  * hibernation_set_ops - Set the global hibernate operations.
  * @ops: Hibernation operations to use in subsequent hibernation transitions.
@@ -777,7 +828,9 @@ int hibernate(void)
 	if (error)
 		goto Restore;
 
-	ksys_sync_helper();
+	error = hibernation_sync_with_timeout();
+	if (error)
+		goto Exit;
 
 	error = freeze_processes();
 	if (error)
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 8eaec4ab1..144caf525 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -30,9 +30,25 @@
 #include <trace/events/power.h>
 #include <linux/compiler.h>
 #include <linux/moduleparam.h>
+#include <linux/completion.h>
+#include <linux/kthread.h>
+#include <linux/jiffies.h>
 
 #include "power.h"
 
+/* Sync timeout parameters for suspend */
+static bool sync_on_suspend_timeout_enable;
+module_param(sync_on_suspend_timeout_enable, bool, 0644);
+MODULE_PARM_DESC(sync_on_suspend_timeout_enable, "Enable sync timeout during suspend (default: false)");
+
+static unsigned int sync_on_suspend_timeout_ms = 10000;
+module_param(sync_on_suspend_timeout_ms, uint, 0644);
+MODULE_PARM_DESC(sync_on_suspend_timeout_ms, "Sync timeout in milliseconds during suspend (default: 10000)");
+
+/* Sync timeout implementation for suspend */
+static struct completion suspend_sync_completion;
+static struct task_struct *suspend_sync_task;
+
 const char * const pm_labels[] = {
 	[PM_SUSPEND_TO_IDLE] = "freeze",
 	[PM_SUSPEND_STANDBY] = "standby",
@@ -61,6 +77,40 @@ static DECLARE_SWAIT_QUEUE_HEAD(s2idle_wait_head);
 enum s2idle_states __read_mostly s2idle_state;
 static DEFINE_RAW_SPINLOCK(s2idle_lock);
 
+static int suspend_sync_thread_func(void *data)
+{
+	ksys_sync_helper();
+	complete(&suspend_sync_completion);
+	return 0;
+}
+
+static int suspend_sync_with_timeout(void)
+{
+	unsigned long timeout_jiffies;
+
+	if (!sync_on_suspend_timeout_enable) {
+		ksys_sync_helper();
+		return 0;
+	}
+
+	init_completion(&suspend_sync_completion);
+	suspend_sync_task = kthread_run(suspend_sync_thread_func, NULL, "suspend_sync");
+	if (IS_ERR(suspend_sync_task)) {
+		pr_warn("PM: suspend: Failed to create sync thread, performing sync directly\n");
+		ksys_sync_helper();
+		return 0;
+	}
+
+	timeout_jiffies = msecs_to_jiffies(sync_on_suspend_timeout_ms);
+	if (!wait_for_completion_timeout(&suspend_sync_completion, timeout_jiffies)) {
+		pr_warn("PM: suspend: Sync operation timed out after %u ms, aborting suspend\n",
+				sync_on_suspend_timeout_ms);
+		kthread_stop(suspend_sync_task);
+		return -ETIMEDOUT;
+	}
+	return 0;
+}
+
 /**
  * pm_suspend_default_s2idle - Check if suspend-to-idle is the default suspend.
  *
@@ -585,8 +635,12 @@ static int enter_state(suspend_state_t state)
 
 	if (sync_on_suspend_enabled) {
 		trace_suspend_resume(TPS("sync_filesystems"), 0, true);
-		ksys_sync_helper();
+		error = suspend_sync_with_timeout();
 		trace_suspend_resume(TPS("sync_filesystems"), 0, false);
+		if (error) {
+			pr_err("PM: Sync timeout, aborting suspend\n");
+			goto Unlock;
+		}
 	}
 
 	pm_pr_dbg("Preparing system for sleep (%s)\n", mem_sleep_labels[state]);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-09-09  8:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-08  2:46 Sync timeout mechanisms - Request for coordination tuhaowen
2025-09-09  2:00 ` Saravana Kannan
2025-09-09  2:45   ` [PATCH v2] PM: Add configurable sync timeout for suspend and hibernation tuhaowen
2025-09-09  6:18     ` Saravana Kannan
2025-09-09  7:13       ` Different approaches to sync timeout: Desktop vs Mobile use cases tuhaowen
2025-09-09  8:51         ` Oliver Neukum
  -- strict thread matches above, loose matches on Subject: below --
2025-09-05  2:47 [PATCH] PM: Add configurable sync timeout during suspend and hibernate to prevent hang tuhaowen
2025-09-05  9:24 ` [PATCH v2] PM: Add configurable sync timeout for suspend and hibernation tuhaowen
2025-09-05 19:27   ` Rafael J. Wysocki
2025-09-08  2:22     ` tuhaowen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox