Message-ID: <CAJZ5v0gzKN1cXfj508G4_9O2hKR0HncW4et3BNbaV+5Erh=LMA@mail.gmail.com>
Date: Tue, 4 Nov 2025 19:52:04 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Samuel Wu <wusamuel@...gle.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>, Pavel Machek <pavel@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Danilo Krummrich <dakr@...nel.org>, tuhaowen@...ontech.com, 
	Saravana Kannan <saravanak@...gle.com>, kernel-team@...roid.com, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6] PM: Support aborting sleep during filesystem sync

On Thu, Oct 30, 2025 at 10:01 PM Samuel Wu <wusamuel@...gle.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. Furthermore, in the case of the back-to-back
> sleeps, a subsequent filesystem sync is needed to ensure the latest
> files are synced right before sleep. If necessary, a subsequent sleep's
> filesystem sync will be queued, and 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@...gle.com>
> Signed-off-by: Samuel Wu <wusamuel@...gle.com>
> ---
> Changes in v6:
> - Use spin_lock_irq() in thread context
> - Use dedicated ordered workqueue for sync work items
> - Use a counter instead of two bools for synchronization
> - Queue fs_sync if it's not already pending on workqueue
> - pm_wakeup_clear(0) is prequisite to this feature, so move it within function
> - Updated commit text for motive of back-to-back fs syncs
> - Tighter lock/unlock around setup, checks, and loop
> - Fix function definitions for CONFIG_PM_SLEEP=n
> - v5 link: https://lore.kernel.org/all/20251017233907.2305303-1-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         | 81 +++++++++++++++++++++++++++++++++++++
>  kernel/power/suspend.c      |  4 +-
>  5 files changed, 100 insertions(+), 2 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..4795f55f9cbe 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 void pm_stop_waiting_for_fs_sync(void) {}
> +static inline int pm_sleep_fs_sync(void) { return 0; }
>
>  #define pm_notifier(fn, pri)   do { (void)(fn); } while (0)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 53166ef86ba4..1874fde4b4f3 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -820,7 +820,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 a6cbc3f4347a..23ca87a172a4 100644
> --- a/kernel/power/main.c
> +++ b/kernel/power/main.c
> @@ -582,6 +582,84 @@ bool pm_sleep_transition_in_progress(void)
>  {
>         return pm_suspend_in_progress() || hibernation_in_progress();
>  }
> +
> +static int pm_sleep_fs_syncs_queued;
> +static DEFINE_SPINLOCK(pm_sleep_fs_sync_lock);
> +static DECLARE_COMPLETION(pm_sleep_fs_sync_complete);
> +static struct workqueue_struct *pm_fs_sync_wq;
> +
> +static int __init pm_start_fs_sync_workqueue(void)
> +{
> +       pm_fs_sync_wq = alloc_ordered_workqueue("pm_fs_sync_wq", 0);
> +
> +       return pm_fs_sync_wq ? 0 : -ENOMEM;
> +}
> +
> +/**
> + * 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);
> +}

Apart from the kernel test robot reports,
pm_stop_waiting_for_fs_sync() has become slightly too heavy for
calling it from wakeup_source_activate().

Waking up the suspend process from there should be sufficient.  The
completion is not necessary for that in principle.

Powered by blists - more mailing lists