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

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

[Samuel Wu]

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 by Saravana Kannan at LPC 2024 [1], where the general
consensus was to allow filesystem sync on a parallel thread. In case of
abort, the suspend process will stop waiting on an in-progress
filesystem sync, and continue by aborting suspend before the filesystem
sync is complete.

Additionally, there is extra care needed to account for back-to-back
sleeps while maintaining functionality to immediately abort during the
filesystem sync stage. This patch handles this by serializing the
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 v5:
- Update spin_lock() to spin_lock_irqsave() since abort can be in IRQ context
- Updated changelog description to be more precise regarding continuing abort
  sleep before fs_sync() is complete
- Rename abort_sleep_during_fs_sync() to pm_stop_waiting_for_fs_sync()
- Simplify from a goto to do-while in pm_sleep_fs_sync()
- v4 link: https://lore.kernel.org/all/20250911185314.2377124-1-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         | 75 +++++++++++++++++++++++++++++++++++++
 kernel/power/suspend.c      |  7 +++-
 5 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index d1283ff1080b..689c16b08b38 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)
+		pm_stop_waiting_for_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);
+	pm_stop_waiting_for_fs_sync();
 	s2idle_wake();
 }
 EXPORT_SYMBOL_GPL(pm_system_wakeup);
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index b02876f1ae38..dc6829b3836f 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -450,6 +450,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 pm_stop_waiting_for_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);
@@ -505,6 +507,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 pm_stop_waiting_for_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 14e85ff23551..9c8db4b3c114 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -824,7 +824,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..81a53d833358 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -570,6 +570,81 @@ 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);
+
+/**
+ * pm_stop_waiting_for_fs_sync - Abort fs_sync to abort sleep early
+ *
+ * This function causes the suspend process to stop waiting on an in-progress
+ * filesystem sync, such that the suspend process can be aborted before the
+ * filesystem sync is complete.
+ */
+void pm_stop_waiting_for_fs_sync(void)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
+	complete(&pm_sleep_fs_sync_complete);
+	spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
+}
+
+static void sync_filesystems_fn(struct work_struct *work)
+{
+	unsigned long flags;
+
+	ksys_sync_helper();
+	spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
+	pm_sleep_fs_sync_queued = false;
+	complete(&pm_sleep_fs_sync_complete);
+	spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
+}
+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;
+	unsigned long flags;
+
+	do {
+		spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
+		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_irqrestore(&pm_sleep_fs_sync_lock, flags);
+
+		/*
+		 * 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;
+	} while (need_pm_sleep_fs_sync_requeue);
+
+	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.858.gf9c4a03a3a-goog

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Hillf Danton]

On Fri, 17 Oct 2025 23:39:06 +0000 Samuel Wu wrote:
> +/**
> + * 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;
> +	unsigned long flags;
> +
> +	do {
> +		spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> +		reinit_completion(&pm_sleep_fs_sync_complete);

Given difficulty following up here, can you specify why reinit is needed?
> +		/*
> +		 * 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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> +
> +		/*
> +		 * 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;
> +	} while (need_pm_sleep_fs_sync_requeue);
> +
> +	return 0;
> +}
> +

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[kernel test robot]

Hi Samuel,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/bleeding-edge amd-pstate/linux-next amd-pstate/bleeding-edge linus/master v6.18-rc1 next-20251017]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Samuel-Wu/PM-Support-aborting-sleep-during-filesystem-sync/20251018-091422
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20251017233907.2305303-1-wusamuel%40google.com
patch subject: [PATCH v5] PM: Support aborting sleep during filesystem sync
config: x86_64-buildonly-randconfig-001-20251018 (https://download.01.org/0day-ci/archive/20251018/202510182014.I0BvTZvE-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251018/202510182014.I0BvTZvE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510182014.I0BvTZvE-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from arch/x86/kernel/asm-offsets.c:14:
>> include/linux/suspend.h:510:15: error: return type defaults to 'int' [-Wimplicit-int]
     510 | static inline pm_stop_waiting_for_fs_sync(void) {}
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/suspend.h: In function 'pm_sleep_fs_sync':
>> include/linux/suspend.h:511:1: warning: no return statement in function returning non-void [-Wreturn-type]
     511 | static inline int pm_sleep_fs_sync(void) {}
         | ^~~~~~
   make[3]: *** [scripts/Makefile.build:182: arch/x86/kernel/asm-offsets.s] Error 1 shuffle=496665200
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1280: prepare0] Error 2 shuffle=496665200
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2 shuffle=496665200
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2 shuffle=496665200
   make: Target 'prepare' not remade because of errors.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for I2C_K1
   Depends on [n]: I2C [=y] && HAS_IOMEM [=y] && (ARCH_SPACEMIT || COMPILE_TEST [=y]) && OF [=n]
   Selected by [y]:
   - MFD_SPACEMIT_P1 [=y] && HAS_IOMEM [=y] && (ARCH_SPACEMIT || COMPILE_TEST [=y]) && I2C [=y]


vim +/int +510 include/linux/suspend.h

   508	
   509	static inline void ksys_sync_helper(void) {}
 > 510	static inline pm_stop_waiting_for_fs_sync(void) {}
 > 511	static inline int pm_sleep_fs_sync(void) {}
   512	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[kernel test robot]

Hi Samuel,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/bleeding-edge amd-pstate/linux-next amd-pstate/bleeding-edge linus/master v6.18-rc1 next-20251017]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Samuel-Wu/PM-Support-aborting-sleep-during-filesystem-sync/20251018-091422
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20251017233907.2305303-1-wusamuel%40google.com
patch subject: [PATCH v5] PM: Support aborting sleep during filesystem sync
config: i386-randconfig-012-20251018 (https://download.01.org/0day-ci/archive/20251019/202510190215.ppx4apvg-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251019/202510190215.ppx4apvg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510190215.ppx4apvg-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from arch/x86/kernel/asm-offsets.c:14:
>> include/linux/suspend.h:510:15: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
     510 | static inline pm_stop_waiting_for_fs_sync(void) {}
         | ~~~~~~~~~~~~~ ^
         | int
   1 error generated.
   make[3]: *** [scripts/Makefile.build:182: arch/x86/kernel/asm-offsets.s] Error 1 shuffle=1011955029
   make[3]: Target 'prepare' not remade because of errors.
   make[2]: *** [Makefile:1280: prepare0] Error 2 shuffle=1011955029
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:248: __sub-make] Error 2 shuffle=1011955029
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:248: __sub-make] Error 2 shuffle=1011955029
   make: Target 'prepare' not remade because of errors.


vim +/int +510 include/linux/suspend.h

   508	
   509	static inline void ksys_sync_helper(void) {}
 > 510	static inline pm_stop_waiting_for_fs_sync(void) {}
   511	static inline int pm_sleep_fs_sync(void) {}
   512	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Samuel Wu]

On Sat, Oct 18, 2025 at 11:32 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Samuel,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on rafael-pm/linux-next]
> [also build test ERROR on rafael-pm/bleeding-edge amd-pstate/linux-next amd-pstate/bleeding-edge linus/master v6.18-rc1 next-20251017]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Samuel-Wu/PM-Support-aborting-sleep-during-filesystem-sync/20251018-091422
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> patch link:    https://lore.kernel.org/r/20251017233907.2305303-1-wusamuel%40google.com
> patch subject: [PATCH v5] PM: Support aborting sleep during filesystem sync
> config: i386-randconfig-012-20251018 (https://download.01.org/0day-ci/archive/20251019/202510190215.ppx4apvg-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251019/202510190215.ppx4apvg-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202510190215.ppx4apvg-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>    In file included from arch/x86/kernel/asm-offsets.c:14:
> >> include/linux/suspend.h:510:15: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
>      510 | static inline pm_stop_waiting_for_fs_sync(void) {}
>          | ~~~~~~~~~~~~~ ^
>          | int
>    1 error generated.
>    make[3]: *** [scripts/Makefile.build:182: arch/x86/kernel/asm-offsets.s] Error 1 shuffle=1011955029
>    make[3]: Target 'prepare' not remade because of errors.
>    make[2]: *** [Makefile:1280: prepare0] Error 2 shuffle=1011955029
>    make[2]: Target 'prepare' not remade because of errors.
>    make[1]: *** [Makefile:248: __sub-make] Error 2 shuffle=1011955029
>    make[1]: Target 'prepare' not remade because of errors.
>    make: *** [Makefile:248: __sub-make] Error 2 shuffle=1011955029
>    make: Target 'prepare' not remade because of errors.
>
>
> vim +/int +510 include/linux/suspend.h
>
>    508
>    509  static inline void ksys_sync_helper(void) {}
>  > 510  static inline pm_stop_waiting_for_fs_sync(void) {}
>    511  static inline int pm_sleep_fs_sync(void) {}
>    512
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

Oversight on my part- I'll fix this in v6 along with other feedback

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Samuel Wu]

On Fri, Oct 17, 2025 at 5:17 PM Hillf Danton <hdanton@sina.com> wrote:
>
> On Fri, 17 Oct 2025 23:39:06 +0000 Samuel Wu wrote:
> > +/**
> > + * 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;
> > +     unsigned long flags;
> > +
> > +     do {
> > +             spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > +             reinit_completion(&pm_sleep_fs_sync_complete);
>
> Given difficulty following up here, can you specify why reinit is needed?

There are two possibilities that make reinit_completion() necessary:
1. Suspend abort triggers completion, but is canceled before
pm_wakeup_pending(), so need reinit to restart the
wait_for_completion() process.
2. Handling back-to-back suspend attempts: after a subsequent suspend
attempt finishes waiting for a previous suspend's fs_sync to finish,
we need the reinit to start the wait_for_completion() process of the
subsequent suspend's fs_sync.

> > +             /*
> > +              * 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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > +
> > +             /*
> > +              * 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;
> > +     } while (need_pm_sleep_fs_sync_requeue);
> > +
> > +     return 0;
> > +}
> > +

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Hillf Danton]

On Tue, 21 Oct 2025 13:13:39 -0700 Samuel Wu wrote:
> On Fri, Oct 17, 2025 at 5:17 PM Hillf Danton <hdanton@sina.com> wrote:
> > On Fri, 17 Oct 2025 23:39:06 +0000 Samuel Wu wrote:
> > > +/**
> > > + * 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;
> > > +     unsigned long flags;
> > > +
> > > +     do {
> > > +             spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > > +             reinit_completion(&pm_sleep_fs_sync_complete);
> >
> > Given difficulty following up here, can you specify why reinit is needed?
> 
> There are two possibilities that make reinit_completion() necessary:
> 1. Suspend abort triggers completion, but is canceled before
> pm_wakeup_pending(), so need reinit to restart the
> wait_for_completion() process.
> 2. Handling back-to-back suspend attempts: after a subsequent suspend
> attempt finishes waiting for a previous suspend's fs_sync to finish,
> we need the reinit to start the wait_for_completion() process of the
> subsequent suspend's fs_sync.
> 
If 1. and 2. matches the comment for wait_for_completion() below,

	static DECLARE_COMPLETION(foo);

	waiter		waker1		waker2
	---		---		---
	for (;;) {
	  reinit_completion(&foo)
	  do anything
	  wait_for_completion(&foo)
			do bar1 	do bar2
			complete(&foo) 	complete(&foo)
	  if (end)
		break;
	}

the chance for reinit to drop one wakeup is not zero.
If drop makes sense, for what do you wait after receiving two wakeups? 

> > > +             /*
> > > +              * 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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > > +
> > > +             /*
> > > +              * 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;
> > > +     } while (need_pm_sleep_fs_sync_requeue);
> > > +
> > > +     return 0;
> > > +}

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Samuel Wu]

On Tue, Oct 21, 2025 at 6:16 PM Hillf Danton <hdanton@sina.com> wrote:
>
> On Tue, 21 Oct 2025 13:13:39 -0700 Samuel Wu wrote:
> > On Fri, Oct 17, 2025 at 5:17 PM Hillf Danton <hdanton@sina.com> wrote:
> > > On Fri, 17 Oct 2025 23:39:06 +0000 Samuel Wu wrote:
> > > > +/**
> > > > + * 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;
> > > > +     unsigned long flags;
> > > > +
> > > > +     do {
> > > > +             spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > > > +             reinit_completion(&pm_sleep_fs_sync_complete);
> > >
> > > Given difficulty following up here, can you specify why reinit is needed?
> >
> > There are two possibilities that make reinit_completion() necessary:
> > 1. Suspend abort triggers completion, but is canceled before
> > pm_wakeup_pending(), so need reinit to restart the
> > wait_for_completion() process.
> > 2. Handling back-to-back suspend attempts: after a subsequent suspend
> > attempt finishes waiting for a previous suspend's fs_sync to finish,
> > we need the reinit to start the wait_for_completion() process of the
> > subsequent suspend's fs_sync.
> >
> If 1. and 2. matches the comment for wait_for_completion() below,
>
>         static DECLARE_COMPLETION(foo);
>
>         waiter          waker1          waker2
>         ---             ---             ---
>         for (;;) {
>           reinit_completion(&foo)
>           do anything
>           wait_for_completion(&foo)
>                         do bar1         do bar2
>                         complete(&foo)  complete(&foo)
>           if (end)
>                 break;
>         }
>
> the chance for reinit to drop one wakeup is not zero.
> If drop makes sense, for what do you wait after receiving two wakeups?
>

If I understand correctly, you are referring to the case where
multiple wakers trigger wait_for_complete() simultaneously, hence
having at least one waker's complete() being ignored?

If so, I see two possibilities with multiple wakers:
1. fs_sync finishing + suspend abort1 + ... + suspend abortN
2. suspend abort1 + ... + suspend abortN

Simplifying, if two wakers come in simultaneously, while one of the
wakers may have its complete() ignored, the state of that waker is
still checked after wait_for_completion(), with
if(pm_wakeup_pending()) and while(need_pm_sleep_fs_sync_requeue) for
suspend aborts and fs_sync finishing respectively.

> > > > +             /*
> > > > +              * 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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > > > +
> > > > +             /*
> > > > +              * 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;
> > > > +     } while (need_pm_sleep_fs_sync_requeue);
> > > > +
> > > > +     return 0;
> > > > +}
>
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Hillf Danton]

On Wed, 22 Oct 2025 11:41:37 -0700 Samuel Wu wrote:
> On Tue, Oct 21, 2025 at 6:16 PM Hillf Danton <hdanton@sina.com> wrote:
> > On Tue, 21 Oct 2025 13:13:39 -0700 Samuel Wu wrote:
> > > On Fri, Oct 17, 2025 at 5:17 PM Hillf Danton <hdanton@sina.com> wrote:
> > > > On Fri, 17 Oct 2025 23:39:06 +0000 Samuel Wu wrote:
> > > > > +/**
> > > > > + * 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;
> > > > > +     unsigned long flags;
> > > > > +
> > > > > +     do {
> > > > > +             spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > > > > +             reinit_completion(&pm_sleep_fs_sync_complete);
> > > >
> > > > Given difficulty following up here, can you specify why reinit is needed?
> > >
> > > There are two possibilities that make reinit_completion() necessary:
> > > 1. Suspend abort triggers completion, but is canceled before
> > > pm_wakeup_pending(), so need reinit to restart the
> > > wait_for_completion() process.
> > > 2. Handling back-to-back suspend attempts: after a subsequent suspend
> > > attempt finishes waiting for a previous suspend's fs_sync to finish,
> > > we need the reinit to start the wait_for_completion() process of the
> > > subsequent suspend's fs_sync.
> > >
> > If 1. and 2. matches the comment for wait_for_completion() below,
> >
> >         static DECLARE_COMPLETION(foo);
> >
> >         waiter          waker1          waker2
> >         ---             ---             ---
> >         for (;;) {
> >           reinit_completion(&foo)
> >           do anything
> >           wait_for_completion(&foo)
> >                         do bar1         do bar2
> >                         complete(&foo)  complete(&foo)
> >           if (end)
> >                 break;
> >         }
> >
> > the chance for reinit to drop one wakeup is not zero.
> > If drop makes sense, for what do you wait after receiving two wakeups?
> >
> 
> If I understand correctly, you are referring to the case where
> multiple wakers trigger wait_for_complete() simultaneously, hence
> having at least one waker's complete() being ignored?
> 
> If so, I see two possibilities with multiple wakers:
> 1. fs_sync finishing + suspend abort1 + ... + suspend abortN
> 2. suspend abort1 + ... + suspend abortN
> 
> Simplifying, if two wakers come in simultaneously, while one of the
> wakers may have its complete() ignored, the state of that waker is
> still checked after wait_for_completion(), with
> if(pm_wakeup_pending()) and while(need_pm_sleep_fs_sync_requeue) for
> suspend aborts and fs_sync finishing respectively.
> 
Note one of the two wakeups may come after the two checks.

       reinit_completion(&foo)
       do anything
       wait_for_completion(&foo)
       		complete(&foo) from waker1
       check1
       check2
       		complete(&foo) from waker2 // dropped by reinit

> > > > > +             /*
> > > > > +              * 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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > > > > +
> > > > > +             /*
> > > > > +              * 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;
> > > > > +     } while (need_pm_sleep_fs_sync_requeue);
> > > > > +
> > > > > +     return 0;
> > > > > +}

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Rafael J. Wysocki]

On Sat, Oct 18, 2025 at 1:39 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 by Saravana Kannan at LPC 2024 [1], where the general
> consensus was to allow filesystem sync on a parallel thread. In case of
> abort, the suspend process will stop waiting on an in-progress
> filesystem sync, and continue by aborting suspend before the filesystem
> sync is complete.
>
> Additionally, there is extra care needed to account for back-to-back
> sleeps while maintaining functionality to immediately abort during the
> filesystem sync stage. This patch handles this by serializing the
> 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.

It would be good to spell out the rationale for starting another
filesystem sync when suspend starts while the previous sync is still
in progress.

> [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 v5:
> - Update spin_lock() to spin_lock_irqsave() since abort can be in IRQ context
> - Updated changelog description to be more precise regarding continuing abort
>   sleep before fs_sync() is complete
> - Rename abort_sleep_during_fs_sync() to pm_stop_waiting_for_fs_sync()
> - Simplify from a goto to do-while in pm_sleep_fs_sync()
> - v4 link: https://lore.kernel.org/all/20250911185314.2377124-1-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         | 75 +++++++++++++++++++++++++++++++++++++
>  kernel/power/suspend.c      |  7 +++-
>  5 files changed, 96 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index d1283ff1080b..689c16b08b38 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)
> +               pm_stop_waiting_for_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);
> +       pm_stop_waiting_for_fs_sync();
>         s2idle_wake();
>  }
>  EXPORT_SYMBOL_GPL(pm_system_wakeup);
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index b02876f1ae38..dc6829b3836f 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -450,6 +450,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 pm_stop_waiting_for_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);
> @@ -505,6 +507,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 pm_stop_waiting_for_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 14e85ff23551..9c8db4b3c114 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -824,7 +824,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..81a53d833358 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -570,6 +570,81 @@ 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);
> +
> +/**
> + * pm_stop_waiting_for_fs_sync - Abort fs_sync to abort sleep early
> + *
> + * This function causes the suspend process to stop waiting on an in-progress
> + * filesystem sync, such that the suspend process can be aborted before the
> + * filesystem sync is complete.
> + */
> +void pm_stop_waiting_for_fs_sync(void)
> +{
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> +       complete(&pm_sleep_fs_sync_complete);
> +       spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
> +}
> +
> +static void sync_filesystems_fn(struct work_struct *work)
> +{
> +       unsigned long flags;
> +
> +       ksys_sync_helper();
> +       spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> +       pm_sleep_fs_sync_queued = false;
> +       complete(&pm_sleep_fs_sync_complete);
> +       spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
> +}
> +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;
> +       unsigned long flags;
> +
> +       do {
> +               spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> +               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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> +
> +               /*
> +                * 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;
> +       } while (need_pm_sleep_fs_sync_requeue);
> +
> +       return 0;
> +}

If I'm not mistaken, the mechanism by which one more sync is started
right after completing the previous one (that was in progress when
suspend started) can be designed differently.

1. Use a dedicated ordered workqueue for the sync work items.
2. Use a counter instead of the two boolean vars for synchronization.
3. In pm_sleep_fs_sync(), if the counter is less than 2, increment the
counter and queue up a sync work item.
4. In sync_filesystems_fn(), decrement the counter.

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Saravana Kannan]

On Thu, Oct 23, 2025 at 12:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Sat, Oct 18, 2025 at 1:39 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 by Saravana Kannan at LPC 2024 [1], where the general
> > consensus was to allow filesystem sync on a parallel thread. In case of
> > abort, the suspend process will stop waiting on an in-progress
> > filesystem sync, and continue by aborting suspend before the filesystem
> > sync is complete.
> >
> > Additionally, there is extra care needed to account for back-to-back
> > sleeps while maintaining functionality to immediately abort during the
> > filesystem sync stage. This patch handles this by serializing the
> > 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.
>
> It would be good to spell out the rationale for starting another
> filesystem sync when suspend starts while the previous sync is still
> in progress.
>
> > [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 v5:
> > - Update spin_lock() to spin_lock_irqsave() since abort can be in IRQ context
> > - Updated changelog description to be more precise regarding continuing abort
> >   sleep before fs_sync() is complete
> > - Rename abort_sleep_during_fs_sync() to pm_stop_waiting_for_fs_sync()
> > - Simplify from a goto to do-while in pm_sleep_fs_sync()
> > - v4 link: https://lore.kernel.org/all/20250911185314.2377124-1-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         | 75 +++++++++++++++++++++++++++++++++++++
> >  kernel/power/suspend.c      |  7 +++-
> >  5 files changed, 96 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> > index d1283ff1080b..689c16b08b38 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)
> > +               pm_stop_waiting_for_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);
> > +       pm_stop_waiting_for_fs_sync();
> >         s2idle_wake();
> >  }
> >  EXPORT_SYMBOL_GPL(pm_system_wakeup);
> > diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> > index b02876f1ae38..dc6829b3836f 100644
> > --- a/include/linux/suspend.h
> > +++ b/include/linux/suspend.h
> > @@ -450,6 +450,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 pm_stop_waiting_for_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);
> > @@ -505,6 +507,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 pm_stop_waiting_for_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 14e85ff23551..9c8db4b3c114 100644
> > --- a/kernel/power/hibernate.c
> > +++ b/kernel/power/hibernate.c
> > @@ -824,7 +824,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..81a53d833358 100644
> > --- a/kernel/power/main.c
> > +++ b/kernel/power/main.c
> > @@ -570,6 +570,81 @@ 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);
> > +
> > +/**
> > + * pm_stop_waiting_for_fs_sync - Abort fs_sync to abort sleep early
> > + *
> > + * This function causes the suspend process to stop waiting on an in-progress
> > + * filesystem sync, such that the suspend process can be aborted before the
> > + * filesystem sync is complete.
> > + */
> > +void pm_stop_waiting_for_fs_sync(void)
> > +{
> > +       unsigned long flags;
> > +
> > +       spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > +       complete(&pm_sleep_fs_sync_complete);
> > +       spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > +}
> > +
> > +static void sync_filesystems_fn(struct work_struct *work)
> > +{
> > +       unsigned long flags;
> > +
> > +       ksys_sync_helper();
> > +       spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > +       pm_sleep_fs_sync_queued = false;
> > +       complete(&pm_sleep_fs_sync_complete);
> > +       spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > +}
> > +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;
> > +       unsigned long flags;
> > +
> > +       do {
> > +               spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > +               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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > +
> > +               /*
> > +                * 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;
> > +       } while (need_pm_sleep_fs_sync_requeue);
> > +
> > +       return 0;
> > +}
>
> If I'm not mistaken, the mechanism by which one more sync is started
> right after completing the previous one (that was in progress when
> suspend started) can be designed differently.
>
> 1. Use a dedicated ordered workqueue for the sync work items.
> 2. Use a counter instead of the two boolean vars for synchronization.
> 3. In pm_sleep_fs_sync(), if the counter is less than 2, increment the
> counter and queue up a sync work item.
> 4. In sync_filesystems_fn(), decrement the counter.

The problem with this is that we can't reuse the same work item. We'll
have to allocate one each time. Otherwise, we'll be queuing one that's
already queued. Right?

-Saravana

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Rafael J. Wysocki]

On Fri, Oct 24, 2025 at 12:47 AM Saravana Kannan <saravanak@google.com> wrote:
>
> On Thu, Oct 23, 2025 at 12:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Sat, Oct 18, 2025 at 1:39 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 by Saravana Kannan at LPC 2024 [1], where the general
> > > consensus was to allow filesystem sync on a parallel thread. In case of
> > > abort, the suspend process will stop waiting on an in-progress
> > > filesystem sync, and continue by aborting suspend before the filesystem
> > > sync is complete.
> > >
> > > Additionally, there is extra care needed to account for back-to-back
> > > sleeps while maintaining functionality to immediately abort during the
> > > filesystem sync stage. This patch handles this by serializing the
> > > 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.
> >
> > It would be good to spell out the rationale for starting another
> > filesystem sync when suspend starts while the previous sync is still
> > in progress.
> >
> > > [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 v5:
> > > - Update spin_lock() to spin_lock_irqsave() since abort can be in IRQ context
> > > - Updated changelog description to be more precise regarding continuing abort
> > >   sleep before fs_sync() is complete
> > > - Rename abort_sleep_during_fs_sync() to pm_stop_waiting_for_fs_sync()
> > > - Simplify from a goto to do-while in pm_sleep_fs_sync()
> > > - v4 link: https://lore.kernel.org/all/20250911185314.2377124-1-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         | 75 +++++++++++++++++++++++++++++++++++++
> > >  kernel/power/suspend.c      |  7 +++-
> > >  5 files changed, 96 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> > > index d1283ff1080b..689c16b08b38 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)
> > > +               pm_stop_waiting_for_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);
> > > +       pm_stop_waiting_for_fs_sync();
> > >         s2idle_wake();
> > >  }
> > >  EXPORT_SYMBOL_GPL(pm_system_wakeup);
> > > diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> > > index b02876f1ae38..dc6829b3836f 100644
> > > --- a/include/linux/suspend.h
> > > +++ b/include/linux/suspend.h
> > > @@ -450,6 +450,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 pm_stop_waiting_for_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);
> > > @@ -505,6 +507,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 pm_stop_waiting_for_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 14e85ff23551..9c8db4b3c114 100644
> > > --- a/kernel/power/hibernate.c
> > > +++ b/kernel/power/hibernate.c
> > > @@ -824,7 +824,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..81a53d833358 100644
> > > --- a/kernel/power/main.c
> > > +++ b/kernel/power/main.c
> > > @@ -570,6 +570,81 @@ 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);
> > > +
> > > +/**
> > > + * pm_stop_waiting_for_fs_sync - Abort fs_sync to abort sleep early
> > > + *
> > > + * This function causes the suspend process to stop waiting on an in-progress
> > > + * filesystem sync, such that the suspend process can be aborted before the
> > > + * filesystem sync is complete.
> > > + */
> > > +void pm_stop_waiting_for_fs_sync(void)
> > > +{
> > > +       unsigned long flags;
> > > +
> > > +       spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > > +       complete(&pm_sleep_fs_sync_complete);
> > > +       spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > > +}
> > > +
> > > +static void sync_filesystems_fn(struct work_struct *work)
> > > +{
> > > +       unsigned long flags;
> > > +
> > > +       ksys_sync_helper();
> > > +       spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > > +       pm_sleep_fs_sync_queued = false;
> > > +       complete(&pm_sleep_fs_sync_complete);
> > > +       spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > > +}
> > > +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;
> > > +       unsigned long flags;
> > > +
> > > +       do {
> > > +               spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > > +               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_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > > +
> > > +               /*
> > > +                * 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;
> > > +       } while (need_pm_sleep_fs_sync_requeue);
> > > +
> > > +       return 0;
> > > +}
> >
> > If I'm not mistaken, the mechanism by which one more sync is started
> > right after completing the previous one (that was in progress when
> > suspend started) can be designed differently.
> >
> > 1. Use a dedicated ordered workqueue for the sync work items.
> > 2. Use a counter instead of the two boolean vars for synchronization.
> > 3. In pm_sleep_fs_sync(), if the counter is less than 2, increment the
> > counter and queue up a sync work item.
> > 4. In sync_filesystems_fn(), decrement the counter.
>
> The problem with this is that we can't reuse the same work item. We'll
> have to allocate one each time. Otherwise, we'll be queuing one that's
> already queued. Right?

Of course you can't queue up an already queued work, but there may be
two of them and then in 3 above use work0 when the counter is 0 and
use work1 when the counter is 1.  No big deal.

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Rafael J. Wysocki]

On Fri, Oct 24, 2025 at 10:37 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Oct 24, 2025 at 12:47 AM Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Thu, Oct 23, 2025 at 12:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Sat, Oct 18, 2025 at 1:39 AM Samuel Wu <wusamuel@google.com> wrote:
> > > >

[cut]

> > >
> > > If I'm not mistaken, the mechanism by which one more sync is started
> > > right after completing the previous one (that was in progress when
> > > suspend started) can be designed differently.
> > >
> > > 1. Use a dedicated ordered workqueue for the sync work items.
> > > 2. Use a counter instead of the two boolean vars for synchronization.
> > > 3. In pm_sleep_fs_sync(), if the counter is less than 2, increment the
> > > counter and queue up a sync work item.
> > > 4. In sync_filesystems_fn(), decrement the counter.
> >
> > The problem with this is that we can't reuse the same work item. We'll
> > have to allocate one each time. Otherwise, we'll be queuing one that's
> > already queued. Right?
>
> Of course you can't queue up an already queued work, but there may be
> two of them and then in 3 above use work0 when the counter is 0 and
> use work1 when the counter is 1.  No big deal.

Moreover, sync_filesystems_fn() doesn't use its work argument, so the
work can be requeued as soon as it is not pending.

Now, when it is not pending, it has not been queued yet or the work
function is running, which is exactly when you want it to be queued:

1. Use a dedicated ordered workqueue for the sync work items.
2. Use a counter instead of the two boolean vars for synchronization.
3. In pm_sleep_fs_sync(), if the work is not pending, queue it up and
increment the counter.
4. In sync_filesystems_fn(), decrement the counter (after carrying out
the fs sync) and complete the completion if the counter is 0.

Of course, the above requires locking, but I don't think that the lock
needs to be spinlock.  A mutex would work just as well AFAICS.

Thanks!

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Samuel Wu]

On Wed, Oct 22, 2025 at 3:32 PM Hillf Danton <hdanton@sina.com> wrote:
>
> On Wed, 22 Oct 2025 11:41:37 -0700 Samuel Wu wrote:
> > On Tue, Oct 21, 2025 at 6:16 PM Hillf Danton <hdanton@sina.com> wrote:
> > > On Tue, 21 Oct 2025 13:13:39 -0700 Samuel Wu wrote:
> > > > On Fri, Oct 17, 2025 at 5:17 PM Hillf Danton <hdanton@sina.com> wrote:
> > > > > On Fri, 17 Oct 2025 23:39:06 +0000 Samuel Wu wrote:
> > > > > > +/**
> > > > > > + * 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;
> > > > > > +     unsigned long flags;
> > > > > > +
> > > > > > +     do {
> > > > > > +             spin_lock_irqsave(&pm_sleep_fs_sync_lock, flags);
> > > > > > +             reinit_completion(&pm_sleep_fs_sync_complete);
> > > > >
> > > > > Given difficulty following up here, can you specify why reinit is needed?
> > > >
> > > > There are two possibilities that make reinit_completion() necessary:
> > > > 1. Suspend abort triggers completion, but is canceled before
> > > > pm_wakeup_pending(), so need reinit to restart the
> > > > wait_for_completion() process.
> > > > 2. Handling back-to-back suspend attempts: after a subsequent suspend
> > > > attempt finishes waiting for a previous suspend's fs_sync to finish,
> > > > we need the reinit to start the wait_for_completion() process of the
> > > > subsequent suspend's fs_sync.
> > > >
> > > If 1. and 2. matches the comment for wait_for_completion() below,
> > >
> > >         static DECLARE_COMPLETION(foo);
> > >
> > >         waiter          waker1          waker2
> > >         ---             ---             ---
> > >         for (;;) {
> > >           reinit_completion(&foo)
> > >           do anything
> > >           wait_for_completion(&foo)
> > >                         do bar1         do bar2
> > >                         complete(&foo)  complete(&foo)
> > >           if (end)
> > >                 break;
> > >         }
> > >
> > > the chance for reinit to drop one wakeup is not zero.
> > > If drop makes sense, for what do you wait after receiving two wakeups?
> > >
> >
> > If I understand correctly, you are referring to the case where
> > multiple wakers trigger wait_for_complete() simultaneously, hence
> > having at least one waker's complete() being ignored?
> >
> > If so, I see two possibilities with multiple wakers:
> > 1. fs_sync finishing + suspend abort1 + ... + suspend abortN
> > 2. suspend abort1 + ... + suspend abortN
> >
> > Simplifying, if two wakers come in simultaneously, while one of the
> > wakers may have its complete() ignored, the state of that waker is
> > still checked after wait_for_completion(), with
> > if(pm_wakeup_pending()) and while(need_pm_sleep_fs_sync_requeue) for
> > suspend aborts and fs_sync finishing respectively.
> >
> Note one of the two wakeups may come after the two checks.
>
>        reinit_completion(&foo)
>        do anything
>        wait_for_completion(&foo)
>                 complete(&foo) from waker1
>        check1
>        check2
>                 complete(&foo) from waker2 // dropped by reinit
>

Thank you Hillf for the foresight and feedback! I'm thinking v6 needs
this structure to address that scenario:

lock
do
        reinit_completion
        // bookkeeping, queue fs_sync
        unlock
        wait_for_completion
        lock
        checks
while
unlock

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Samuel Wu]

On Wed, Oct 29, 2025 at 6:23 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Oct 24, 2025 at 10:37 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Fri, Oct 24, 2025 at 12:47 AM Saravana Kannan <saravanak@google.com> wrote:
> > >
> > > On Thu, Oct 23, 2025 at 12:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > > >
> > > > On Sat, Oct 18, 2025 at 1:39 AM Samuel Wu <wusamuel@google.com> wrote:
> > > > >
>
> [cut]
>
> > > >
> > > > If I'm not mistaken, the mechanism by which one more sync is started
> > > > right after completing the previous one (that was in progress when
> > > > suspend started) can be designed differently.
> > > >
> > > > 1. Use a dedicated ordered workqueue for the sync work items.
> > > > 2. Use a counter instead of the two boolean vars for synchronization.
> > > > 3. In pm_sleep_fs_sync(), if the counter is less than 2, increment the
> > > > counter and queue up a sync work item.
> > > > 4. In sync_filesystems_fn(), decrement the counter.
> > >
> > > The problem with this is that we can't reuse the same work item. We'll
> > > have to allocate one each time. Otherwise, we'll be queuing one that's
> > > already queued. Right?
> >
> > Of course you can't queue up an already queued work, but there may be
> > two of them and then in 3 above use work0 when the counter is 0 and
> > use work1 when the counter is 1.  No big deal.
>
> Moreover, sync_filesystems_fn() doesn't use its work argument, so the
> work can be requeued as soon as it is not pending.
>
> Now, when it is not pending, it has not been queued yet or the work
> function is running, which is exactly when you want it to be queued:
>
> 1. Use a dedicated ordered workqueue for the sync work items.
> 2. Use a counter instead of the two boolean vars for synchronization.
> 3. In pm_sleep_fs_sync(), if the work is not pending, queue it up and
> increment the counter.
> 4. In sync_filesystems_fn(), decrement the counter (after carrying out
> the fs sync) and complete the completion if the counter is 0.

Thank you for the thoughtful feedback Rafael.

I agree with these points and will incorporate it in v6; this approach
seems more elegant.

> Of course, the above requires locking, but I don't think that the lock
> needs to be spinlock.  A mutex would work just as well AFAICS.
>
> Thanks!

I'm still thinking this needs to be spin_lock_irqsave(), since
pm_stop_waiting_for_fs_sync() is called in an interrupt context and
needs the lock such that the abort signals don't get lost.

Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

[Rafael J. Wysocki]

On Wed, Oct 29, 2025 at 8:13 PM Samuel Wu <wusamuel@google.com> wrote:
>
> On Wed, Oct 29, 2025 at 6:23 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Fri, Oct 24, 2025 at 10:37 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > >
> > > On Fri, Oct 24, 2025 at 12:47 AM Saravana Kannan <saravanak@google.com> wrote:
> > > >
> > > > On Thu, Oct 23, 2025 at 12:43 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> > > > >
> > > > > On Sat, Oct 18, 2025 at 1:39 AM Samuel Wu <wusamuel@google.com> wrote:
> > > > > >
> >
> > [cut]
> >
> > > > >
> > > > > If I'm not mistaken, the mechanism by which one more sync is started
> > > > > right after completing the previous one (that was in progress when
> > > > > suspend started) can be designed differently.
> > > > >
> > > > > 1. Use a dedicated ordered workqueue for the sync work items.
> > > > > 2. Use a counter instead of the two boolean vars for synchronization.
> > > > > 3. In pm_sleep_fs_sync(), if the counter is less than 2, increment the
> > > > > counter and queue up a sync work item.
> > > > > 4. In sync_filesystems_fn(), decrement the counter.
> > > >
> > > > The problem with this is that we can't reuse the same work item. We'll
> > > > have to allocate one each time. Otherwise, we'll be queuing one that's
> > > > already queued. Right?
> > >
> > > Of course you can't queue up an already queued work, but there may be
> > > two of them and then in 3 above use work0 when the counter is 0 and
> > > use work1 when the counter is 1.  No big deal.
> >
> > Moreover, sync_filesystems_fn() doesn't use its work argument, so the
> > work can be requeued as soon as it is not pending.
> >
> > Now, when it is not pending, it has not been queued yet or the work
> > function is running, which is exactly when you want it to be queued:
> >
> > 1. Use a dedicated ordered workqueue for the sync work items.
> > 2. Use a counter instead of the two boolean vars for synchronization.
> > 3. In pm_sleep_fs_sync(), if the work is not pending, queue it up and
> > increment the counter.
> > 4. In sync_filesystems_fn(), decrement the counter (after carrying out
> > the fs sync) and complete the completion if the counter is 0.
>
> Thank you for the thoughtful feedback Rafael.
>
> I agree with these points and will incorporate it in v6; this approach
> seems more elegant.
>
> > Of course, the above requires locking, but I don't think that the lock
> > needs to be spinlock.  A mutex would work just as well AFAICS.
> >
> > Thanks!
>
> I'm still thinking this needs to be spin_lock_irqsave(), since
> pm_stop_waiting_for_fs_sync() is called in an interrupt context and
> needs the lock such that the abort signals don't get lost.

OK, I see.  __pm_stay_awake() will call it under a spinlock, for one example.

Yes, you need a spinlock, but in thread context it is sufficient to
use spin_lock_irq() (no need to save the flags).