[PATCH v4] PM: Support aborting sleep during filesystem sync

Samuel Wu posted 1 patch 2 weeks, 6 days ago
drivers/base/power/wakeup.c |  8 +++++
include/linux/suspend.h     |  4 +++
kernel/power/hibernate.c    |  5 ++-
kernel/power/main.c         | 70 +++++++++++++++++++++++++++++++++++++
kernel/power/suspend.c      |  7 ++--
5 files changed, 91 insertions(+), 3 deletions(-)
[PATCH v4] PM: Support aborting sleep during filesystem sync
Posted by Samuel Wu 2 weeks, 6 days ago
At the start of suspend and hibernate, filesystems will sync to save the
current state of the device. However, the long tail of the filesystem
sync can take upwards of 25 seconds. If during this filesystem sync
there is some wakeup or abort signal, it will not be processed until the
sync is complete; from a user's perspective, this looks like the device
is unresponsive to any form of input.

This patch adds functionality to handle a sleep abort signal when in
the filesystem sync phase of suspend or hibernate. This topic was first
discussed specifically for suspend by Saravana Kannan at LPC 2024 [1],
where the general consensus was to allow filesystem sync on a parallel
thread. The same logic applies to both suspend and hibernate code paths.

There is extra care needed to account for back-to-back sleeps while
still maintaining functionality to immediately abort during the
filesystem sync stage.

This patch handles this by serializing the filesystem sync sequence with
an invariant; a subsequent sleep's filesystem sync operation will only
start when the previous sleep's filesystem sync has finished. While
waiting for the previous sleep's filesystem sync to finish, the
subsequent sleep will still abort early if a wakeup event is triggered,
solving the original issue of filesystem sync blocking abort.

[1]: https://lpc.events/event/18/contributions/1845/

Suggested-by: Saravana Kannan <saravanak@google.com>
Signed-off-by: Samuel Wu <wusamuel@google.com>
---
Changes in v4:
- Removed patch 1/3 of v3 as it is already picked up on linux-pm
- Squashed patches 2/3 and 3/3 from v3 into this single patch
- Added abort during fs_sync functionality to hibernate in addition to suspend
- Moved variables and functions for abort from power/suspend.c to power/main.c
- Renamed suspend_fs_sync_with_abort() to pm_sleep_fs_sync()
- Renamed suspend_abort_fs_sync() to abort_sleep_during_fs_sync()
- v3 link: https://lore.kernel.org/all/20250821004237.2712312-1-wusamuel@google.com/

Changes in v3:
- Split v2 patch into 3 patches
- Moved pm_wakeup_clear() outside of if(sync_on_suspend_enabled) condition
- Updated documentation and comments within kernel/power/suspend.c
- v2 link: https://lore.kernel.org/all/20250812232126.1814253-1-wusamuel@google.com/

Changes in v2:
- Added documentation for suspend_abort_fs_sync()
- Made suspend_fs_sync_lock and suspend_fs_sync_complete declaration static
- v1 link: https://lore.kernel.org/all/20250815004635.3684650-1-wusamuel@google.com

 drivers/base/power/wakeup.c |  8 +++++
 include/linux/suspend.h     |  4 +++
 kernel/power/hibernate.c    |  5 ++-
 kernel/power/main.c         | 70 +++++++++++++++++++++++++++++++++++++
 kernel/power/suspend.c      |  7 ++--
 5 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index d1283ff1080b..daf07ab7ac3f 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -570,6 +570,13 @@ static void wakeup_source_activate(struct wakeup_source *ws)
 
 	/* Increment the counter of events in progress. */
 	cec = atomic_inc_return(&combined_event_count);
+	/*
+	 * wakeup_source_activate() aborts sleep only if events_check_enabled
+	 * is set (see pm_wakeup_pending()). Similarly, abort sleep during
+	 * fs_sync only if events_check_enabled is set.
+	 */
+	if (events_check_enabled)
+		abort_sleep_during_fs_sync();
 
 	trace_wakeup_source_activate(ws->name, cec);
 }
@@ -899,6 +906,7 @@ EXPORT_SYMBOL_GPL(pm_wakeup_pending);
 void pm_system_wakeup(void)
 {
 	atomic_inc(&pm_abort_suspend);
+	abort_sleep_during_fs_sync();
 	s2idle_wake();
 }
 EXPORT_SYMBOL_GPL(pm_system_wakeup);
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 317ae31e89b3..c961bdb00bb6 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -444,6 +444,8 @@ void restore_processor_state(void);
 extern int register_pm_notifier(struct notifier_block *nb);
 extern int unregister_pm_notifier(struct notifier_block *nb);
 extern void ksys_sync_helper(void);
+extern void abort_sleep_during_fs_sync(void);
+extern int pm_sleep_fs_sync(void);
 extern void pm_report_hw_sleep_time(u64 t);
 extern void pm_report_max_hw_sleep(u64 t);
 void pm_restrict_gfp_mask(void);
@@ -499,6 +501,8 @@ static inline void pm_restrict_gfp_mask(void) {}
 static inline void pm_restore_gfp_mask(void) {}
 
 static inline void ksys_sync_helper(void) {}
+static inline abort_sleep_during_fs_sync(void) {}
+static inline int pm_sleep_fs_sync(void) {}
 
 #define pm_notifier(fn, pri)	do { (void)(fn); } while (0)
 
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 2f66ab453823..651dcd768644 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -811,7 +811,10 @@ int hibernate(void)
 	if (error)
 		goto Restore;
 
-	ksys_sync_helper();
+	error = pm_sleep_fs_sync();
+	if (error)
+		goto Restore;
+
 	if (filesystem_freeze_enabled)
 		filesystems_freeze();
 
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 3cf2d7e72567..38b1de295cfe 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -570,6 +570,76 @@ bool pm_sleep_transition_in_progress(void)
 {
 	return pm_suspend_in_progress() || hibernation_in_progress();
 }
+
+static bool pm_sleep_fs_sync_queued;
+static DEFINE_SPINLOCK(pm_sleep_fs_sync_lock);
+static DECLARE_COMPLETION(pm_sleep_fs_sync_complete);
+
+/**
+ * abort_sleep_during_fs_sync - Abort fs_sync to abort sleep early
+ *
+ * This function aborts the fs_sync stage of suspend/hibernate so that
+ * suspend/hibernate itself can be aborted early.
+ */
+void abort_sleep_during_fs_sync(void)
+{
+	spin_lock(&pm_sleep_fs_sync_lock);
+	complete(&pm_sleep_fs_sync_complete);
+	spin_unlock(&pm_sleep_fs_sync_lock);
+}
+
+static void sync_filesystems_fn(struct work_struct *work)
+{
+	ksys_sync_helper();
+
+	spin_lock(&pm_sleep_fs_sync_lock);
+	pm_sleep_fs_sync_queued = false;
+	complete(&pm_sleep_fs_sync_complete);
+	spin_unlock(&pm_sleep_fs_sync_lock);
+}
+static DECLARE_WORK(sync_filesystems, sync_filesystems_fn);
+
+/**
+ * pm_sleep_fs_sync - Trigger fs_sync with ability to abort
+ *
+ * Return 0 on successful file system sync, otherwise returns -EBUSY if file
+ * system sync was aborted.
+ */
+int pm_sleep_fs_sync(void)
+{
+	bool need_pm_sleep_fs_sync_requeue;
+
+Start_fs_sync:
+	spin_lock(&pm_sleep_fs_sync_lock);
+	reinit_completion(&pm_sleep_fs_sync_complete);
+	/*
+	 * Handle the case where a sleep immediately follows a previous sleep
+	 * that was aborted during fs_sync. In this case, wait for the previous
+	 * filesystem sync to finish. Then do another filesystem sync so any
+	 * subsequent filesystem changes are synced before sleeping.
+	 */
+	if (pm_sleep_fs_sync_queued) {
+		need_pm_sleep_fs_sync_requeue = true;
+	} else {
+		need_pm_sleep_fs_sync_requeue = false;
+		pm_sleep_fs_sync_queued = true;
+		schedule_work(&sync_filesystems);
+	}
+	spin_unlock(&pm_sleep_fs_sync_lock);
+
+	/*
+	 * Completion is triggered by fs_sync finishing or an abort sleep
+	 * signal, whichever comes first
+	 */
+	wait_for_completion(&pm_sleep_fs_sync_complete);
+	if (pm_wakeup_pending())
+		return -EBUSY;
+	if (need_pm_sleep_fs_sync_requeue)
+		goto Start_fs_sync;
+
+	return 0;
+}
+
 #endif /* CONFIG_PM_SLEEP */
 
 #ifdef CONFIG_PM_SLEEP_DEBUG
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 4bb4686c1c08..c019a4396c1f 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -31,6 +31,7 @@
 #include <linux/compiler.h>
 #include <linux/moduleparam.h>
 #include <linux/fs.h>
+#include <linux/workqueue.h>
 
 #include "power.h"
 
@@ -588,14 +589,16 @@ static int enter_state(suspend_state_t state)
 	if (state == PM_SUSPEND_TO_IDLE)
 		s2idle_begin();
 
+	pm_wakeup_clear(0);
 	if (sync_on_suspend_enabled) {
 		trace_suspend_resume(TPS("sync_filesystems"), 0, true);
-		ksys_sync_helper();
+		error = pm_sleep_fs_sync();
 		trace_suspend_resume(TPS("sync_filesystems"), 0, false);
+		if (error)
+			goto Unlock;
 	}
 
 	pm_pr_dbg("Preparing system for sleep (%s)\n", mem_sleep_labels[state]);
-	pm_wakeup_clear(0);
 	pm_suspend_clear_flags();
 	error = suspend_prepare(state);
 	if (error)
-- 
2.51.0.384.g4c02a37b29-goog
Re: [PATCH v4] PM: Support aborting sleep during filesystem sync
Posted by Samuel Wu 1 day, 23 hours ago
Hi Rafael,

Just a friendly ping on this patch. Please let me know if there's any
feedback or if you'd like me to make any changes.

Thanks,
Sam

On Thu, Sep 11, 2025 at 11:53 AM Samuel Wu <wusamuel@google.com> wrote:
>
> At the start of suspend and hibernate, filesystems will sync to save the
> current state of the device. However, the long tail of the filesystem
> sync can take upwards of 25 seconds. If during this filesystem sync
> there is some wakeup or abort signal, it will not be processed until the
> sync is complete; from a user's perspective, this looks like the device
> is unresponsive to any form of input.
>
> This patch adds functionality to handle a sleep abort signal when in
> the filesystem sync phase of suspend or hibernate. This topic was first
> discussed specifically for suspend by Saravana Kannan at LPC 2024 [1],
> where the general consensus was to allow filesystem sync on a parallel
> thread. The same logic applies to both suspend and hibernate code paths.
>
> There is extra care needed to account for back-to-back sleeps while
> still maintaining functionality to immediately abort during the
> filesystem sync stage.
>
> This patch handles this by serializing the filesystem sync sequence with
> an invariant; a subsequent sleep's filesystem sync operation will only
> start when the previous sleep's filesystem sync has finished. While
> waiting for the previous sleep's filesystem sync to finish, the
> subsequent sleep will still abort early if a wakeup event is triggered,
> solving the original issue of filesystem sync blocking abort.
>
> [1]: https://lpc.events/event/18/contributions/1845/
>
> Suggested-by: Saravana Kannan <saravanak@google.com>
> Signed-off-by: Samuel Wu <wusamuel@google.com>
> ---
> Changes in v4:
> - Removed patch 1/3 of v3 as it is already picked up on linux-pm
> - Squashed patches 2/3 and 3/3 from v3 into this single patch
> - Added abort during fs_sync functionality to hibernate in addition to suspend
> - Moved variables and functions for abort from power/suspend.c to power/main.c
> - Renamed suspend_fs_sync_with_abort() to pm_sleep_fs_sync()
> - Renamed suspend_abort_fs_sync() to abort_sleep_during_fs_sync()
> - v3 link: https://lore.kernel.org/all/20250821004237.2712312-1-wusamuel@google.com/
>
> Changes in v3:
> - Split v2 patch into 3 patches
> - Moved pm_wakeup_clear() outside of if(sync_on_suspend_enabled) condition
> - Updated documentation and comments within kernel/power/suspend.c
> - v2 link: https://lore.kernel.org/all/20250812232126.1814253-1-wusamuel@google.com/
>
> Changes in v2:
> - Added documentation for suspend_abort_fs_sync()
> - Made suspend_fs_sync_lock and suspend_fs_sync_complete declaration static
> - v1 link: https://lore.kernel.org/all/20250815004635.3684650-1-wusamuel@google.com
>
>  drivers/base/power/wakeup.c |  8 +++++
>  include/linux/suspend.h     |  4 +++
>  kernel/power/hibernate.c    |  5 ++-
>  kernel/power/main.c         | 70 +++++++++++++++++++++++++++++++++++++
>  kernel/power/suspend.c      |  7 ++--
>  5 files changed, 91 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index d1283ff1080b..daf07ab7ac3f 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -570,6 +570,13 @@ static void wakeup_source_activate(struct wakeup_source *ws)
>
>         /* Increment the counter of events in progress. */
>         cec = atomic_inc_return(&combined_event_count);
> +       /*
> +        * wakeup_source_activate() aborts sleep only if events_check_enabled
> +        * is set (see pm_wakeup_pending()). Similarly, abort sleep during
> +        * fs_sync only if events_check_enabled is set.
> +        */
> +       if (events_check_enabled)
> +               abort_sleep_during_fs_sync();
>
>         trace_wakeup_source_activate(ws->name, cec);
>  }
> @@ -899,6 +906,7 @@ EXPORT_SYMBOL_GPL(pm_wakeup_pending);
>  void pm_system_wakeup(void)
>  {
>         atomic_inc(&pm_abort_suspend);
> +       abort_sleep_during_fs_sync();
>         s2idle_wake();
>  }
>  EXPORT_SYMBOL_GPL(pm_system_wakeup);
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 317ae31e89b3..c961bdb00bb6 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -444,6 +444,8 @@ void restore_processor_state(void);
>  extern int register_pm_notifier(struct notifier_block *nb);
>  extern int unregister_pm_notifier(struct notifier_block *nb);
>  extern void ksys_sync_helper(void);
> +extern void abort_sleep_during_fs_sync(void);
> +extern int pm_sleep_fs_sync(void);
>  extern void pm_report_hw_sleep_time(u64 t);
>  extern void pm_report_max_hw_sleep(u64 t);
>  void pm_restrict_gfp_mask(void);
> @@ -499,6 +501,8 @@ static inline void pm_restrict_gfp_mask(void) {}
>  static inline void pm_restore_gfp_mask(void) {}
>
>  static inline void ksys_sync_helper(void) {}
> +static inline abort_sleep_during_fs_sync(void) {}
> +static inline int pm_sleep_fs_sync(void) {}
>
>  #define pm_notifier(fn, pri)   do { (void)(fn); } while (0)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 2f66ab453823..651dcd768644 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -811,7 +811,10 @@ int hibernate(void)
>         if (error)
>                 goto Restore;
>
> -       ksys_sync_helper();
> +       error = pm_sleep_fs_sync();
> +       if (error)
> +               goto Restore;
> +
>         if (filesystem_freeze_enabled)
>                 filesystems_freeze();
>
> diff --git a/kernel/power/main.c b/kernel/power/main.c
> index 3cf2d7e72567..38b1de295cfe 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -570,6 +570,76 @@ bool pm_sleep_transition_in_progress(void)
>  {
>         return pm_suspend_in_progress() || hibernation_in_progress();
>  }
> +
> +static bool pm_sleep_fs_sync_queued;
> +static DEFINE_SPINLOCK(pm_sleep_fs_sync_lock);
> +static DECLARE_COMPLETION(pm_sleep_fs_sync_complete);
> +
> +/**
> + * abort_sleep_during_fs_sync - Abort fs_sync to abort sleep early
> + *
> + * This function aborts the fs_sync stage of suspend/hibernate so that
> + * suspend/hibernate itself can be aborted early.
> + */
> +void abort_sleep_during_fs_sync(void)
> +{
> +       spin_lock(&pm_sleep_fs_sync_lock);
> +       complete(&pm_sleep_fs_sync_complete);
> +       spin_unlock(&pm_sleep_fs_sync_lock);
> +}
> +
> +static void sync_filesystems_fn(struct work_struct *work)
> +{
> +       ksys_sync_helper();
> +
> +       spin_lock(&pm_sleep_fs_sync_lock);
> +       pm_sleep_fs_sync_queued = false;
> +       complete(&pm_sleep_fs_sync_complete);
> +       spin_unlock(&pm_sleep_fs_sync_lock);
> +}
> +static DECLARE_WORK(sync_filesystems, sync_filesystems_fn);
> +
> +/**
> + * pm_sleep_fs_sync - Trigger fs_sync with ability to abort
> + *
> + * Return 0 on successful file system sync, otherwise returns -EBUSY if file
> + * system sync was aborted.
> + */
> +int pm_sleep_fs_sync(void)
> +{
> +       bool need_pm_sleep_fs_sync_requeue;
> +
> +Start_fs_sync:
> +       spin_lock(&pm_sleep_fs_sync_lock);
> +       reinit_completion(&pm_sleep_fs_sync_complete);
> +       /*
> +        * Handle the case where a sleep immediately follows a previous sleep
> +        * that was aborted during fs_sync. In this case, wait for the previous
> +        * filesystem sync to finish. Then do another filesystem sync so any
> +        * subsequent filesystem changes are synced before sleeping.
> +        */
> +       if (pm_sleep_fs_sync_queued) {
> +               need_pm_sleep_fs_sync_requeue = true;
> +       } else {
> +               need_pm_sleep_fs_sync_requeue = false;
> +               pm_sleep_fs_sync_queued = true;
> +               schedule_work(&sync_filesystems);
> +       }
> +       spin_unlock(&pm_sleep_fs_sync_lock);
> +
> +       /*
> +        * Completion is triggered by fs_sync finishing or an abort sleep
> +        * signal, whichever comes first
> +        */
> +       wait_for_completion(&pm_sleep_fs_sync_complete);
> +       if (pm_wakeup_pending())
> +               return -EBUSY;
> +       if (need_pm_sleep_fs_sync_requeue)
> +               goto Start_fs_sync;
> +
> +       return 0;
> +}
> +
>  #endif /* CONFIG_PM_SLEEP */
>
>  #ifdef CONFIG_PM_SLEEP_DEBUG
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 4bb4686c1c08..c019a4396c1f 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -31,6 +31,7 @@
>  #include <linux/compiler.h>
>  #include <linux/moduleparam.h>
>  #include <linux/fs.h>
> +#include <linux/workqueue.h>
>
>  #include "power.h"
>
> @@ -588,14 +589,16 @@ static int enter_state(suspend_state_t state)
>         if (state == PM_SUSPEND_TO_IDLE)
>                 s2idle_begin();
>
> +       pm_wakeup_clear(0);
>         if (sync_on_suspend_enabled) {
>                 trace_suspend_resume(TPS("sync_filesystems"), 0, true);
> -               ksys_sync_helper();
> +               error = pm_sleep_fs_sync();
>                 trace_suspend_resume(TPS("sync_filesystems"), 0, false);
> +               if (error)
> +                       goto Unlock;
>         }
>
>         pm_pr_dbg("Preparing system for sleep (%s)\n", mem_sleep_labels[state]);
> -       pm_wakeup_clear(0);
>         pm_suspend_clear_flags();
>         error = suspend_prepare(state);
>         if (error)
> --
> 2.51.0.384.g4c02a37b29-goog
>
Re: [PATCH v4] PM: Support aborting sleep during filesystem sync
Posted by Rafael J. Wysocki 1 day, 23 hours ago
Hi,

On Tue, Sep 30, 2025 at 8:30 PM Samuel Wu <wusamuel@google.com> wrote:
>
> Hi Rafael,
>
> Just a friendly ping on this patch. Please let me know if there's any
> feedback or if you'd like me to make any changes.

Have you seen https://lore.kernel.org/all/20250909065836.32534-1-tuhaowen@uniontech.com/
?

If so, what do you think about it?

If not, please see it.

Thanks!
Re: [PATCH v4] PM: Support aborting sleep during filesystem sync
Posted by Samuel Wu 1 day, 19 hours ago
On Tue, Sep 30, 2025 at 11:51 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> Hi,
>
> On Tue, Sep 30, 2025 at 8:30 PM Samuel Wu <wusamuel@google.com> wrote:
> >
> > Hi Rafael,
> >
> > Just a friendly ping on this patch. Please let me know if there's any
> > feedback or if you'd like me to make any changes.
>
> Have you seen https://lore.kernel.org/all/20250909065836.32534-1-tuhaowen@uniontech.com/
> ?
>
> If so, what do you think about it?

I was following this chain
(https://lore.kernel.org/all/20250908024655.14636-1-tuhaowen@uniontech.com/),
where there is some ongoing discussion on converging the solution.

Our changes aren't mutually exclusive, and tuhaowen can build changes
on top of ours, even indicating:
> I'm happy to work on this as a follow-up patch series after your changes land, or we could explore a unified solution that handles both scenarios.

These patchsets don't negate each other, so could we decouple these
two patchsets since they address different issues?

Thanks,
Sam
>
> If not, please see it.
>
> Thanks!
Re: [PATCH v4] PM: Support aborting sleep during filesystem sync
Posted by Rafael J. Wysocki 23 hours ago
On Wed, Oct 1, 2025 at 12:37 AM Samuel Wu <wusamuel@google.com> wrote:
>
> On Tue, Sep 30, 2025 at 11:51 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > Hi,
> >
> > On Tue, Sep 30, 2025 at 8:30 PM Samuel Wu <wusamuel@google.com> wrote:
> > >
> > > Hi Rafael,
> > >
> > > Just a friendly ping on this patch. Please let me know if there's any
> > > feedback or if you'd like me to make any changes.
> >
> > Have you seen https://lore.kernel.org/all/20250909065836.32534-1-tuhaowen@uniontech.com/
> > ?
> >
> > If so, what do you think about it?
>
> I was following this chain
> (https://lore.kernel.org/all/20250908024655.14636-1-tuhaowen@uniontech.com/),
> where there is some ongoing discussion on converging the solution.
>
> Our changes aren't mutually exclusive, and tuhaowen can build changes
> on top of ours, even indicating:
> > I'm happy to work on this as a follow-up patch series after your changes land, or we could explore a unified solution that handles both scenarios.

That's fair.

> These patchsets don't negate each other, so could we decouple these
> two patchsets since they address different issues?

Well, I'm not sure if they are really different.  After all, this is
all about avoiding having to wait on an excessively long filesystem
sync during suspend.

I'm also not sure why it is being pursued to be honest.  Is setting
/sys/power/sync_on_suspend to 0 not regarded as a viable option?
Re: [PATCH v4] PM: Support aborting sleep during filesystem sync
Posted by Saravana Kannan 20 hours ago
On Wed, Oct 1, 2025 at 11:22 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Oct 1, 2025 at 12:37 AM Samuel Wu <wusamuel@google.com> wrote:
> >
> > On Tue, Sep 30, 2025 at 11:51 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > Hi,
> > >
> > > On Tue, Sep 30, 2025 at 8:30 PM Samuel Wu <wusamuel@google.com> wrote:
> > > >
> > > > Hi Rafael,
> > > >
> > > > Just a friendly ping on this patch. Please let me know if there's any
> > > > feedback or if you'd like me to make any changes.
> > >
> > > Have you seen https://lore.kernel.org/all/20250909065836.32534-1-tuhaowen@uniontech.com/
> > > ?
> > >
> > > If so, what do you think about it?
> >
> > I was following this chain
> > (https://lore.kernel.org/all/20250908024655.14636-1-tuhaowen@uniontech.com/),
> > where there is some ongoing discussion on converging the solution.
> >
> > Our changes aren't mutually exclusive, and tuhaowen can build changes
> > on top of ours, even indicating:
> > > I'm happy to work on this as a follow-up patch series after your changes land, or we could explore a unified solution that handles both scenarios.
>
> That's fair.
>
> > These patchsets don't negate each other, so could we decouple these
> > two patchsets since they address different issues?
>
> Well, I'm not sure if they are really different.  After all, this is
> all about avoiding having to wait on an excessively long filesystem
> sync during suspend.

No, it's different. We don't mind a long filesystem sync if we don't
have a need to abort a suspend. If it takes 25 seconds to sync the
filesystem but there's no need to abort it, that's totally fine. So,
this patch is just about allowing abort to happen without waiting for
file system sync to finish.

The other patch's requirement is to always abort if suspend takes 25
seconds (or whatever the timeout is). IIRC, in his case, it's because
of a bad disk or say a USB disk getting unplugged. I'm not convinced a
suspend timeout is the right thing to do, but I'm not going to nack
it. But to implement his requirement, he can put a patch on top of
ours where he sets a timer and then aborts suspends if it fires.

> I'm also not sure why it is being pursued to be honest.  Is setting
> /sys/power/sync_on_suspend to 0 not regarded as a viable option?

We do want to sync on every suspend. So, we don't want to turn it off
completely.

Thanks,
Saravana