linux-kernel - Re: [PATCH v3 3/3] PM: Support abort during fs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAGETcx-7qkHPQdUXf_SQMzZbTdcxF3oMk4nqfRnb6=wf0QQzcQ@mail.gmail.com>
Date: Wed, 20 Aug 2025 23:45:03 -0700
From: Saravana Kannan <saravanak@...gle.com>
To: Samuel Wu <wusamuel@...gle.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Pavel Machek <pavel@...nel.org>, Len Brown <lenb@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Danilo Krummrich <dakr@...nel.org>, kernel-team@...roid.com, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] PM: Support abort during fs_sync of back-to-back suspends

On Wed, Aug 20, 2025 at 5:43 PM Samuel Wu <wusamuel@...gle.com> wrote:
>
> There is extra care needed to account for back-to-back suspends 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 suspend's filesystem sync operation will only
> start when the previous suspend's filesystem sync has finished. While
> waiting for the previous suspend's filesystem sync to finish, the
> subsequent suspend will still abort early if a wakeup event is
> triggered, solving the original issue of filesystem sync blocking abort.
>
> Suggested-by: Saravana Kannan <saravanak@...gle.com>
> Signed-off-by: Samuel Wu <wusamuel@...gle.com>
> Reviewed-by: Saravana Kannan <saravanak@...gle.com>

I think you meant to add my Reviewed-by: to all 3 patches.

Rafael,

Feel free to add it when you pick up these patches.

-Saravana
> ---
>  kernel/power/suspend.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index edacd2a4143b..514c590ec383 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -75,6 +75,8 @@ bool pm_suspend_default_s2idle(void)
>  }
>  EXPORT_SYMBOL_GPL(pm_suspend_default_s2idle);
>
> +static bool suspend_fs_sync_queued;
> +static DEFINE_SPINLOCK(suspend_fs_sync_lock);
>  static DECLARE_COMPLETION(suspend_fs_sync_complete);
>
>  /**
> @@ -85,7 +87,9 @@ static DECLARE_COMPLETION(suspend_fs_sync_complete);
>   */
>  void suspend_abort_fs_sync(void)
>  {
> +       spin_lock(&suspend_fs_sync_lock);
>         complete(&suspend_fs_sync_complete);
> +       spin_unlock(&suspend_fs_sync_lock);
>  }
>
>  void s2idle_set_ops(const struct platform_s2idle_ops *ops)
> @@ -420,7 +424,11 @@ void __weak arch_suspend_enable_irqs(void)
>  static void sync_filesystems_fn(struct work_struct *work)
>  {
>         ksys_sync_helper();
> +
> +       spin_lock(&suspend_fs_sync_lock);
> +       suspend_fs_sync_queued = false;
>         complete(&suspend_fs_sync_complete);
> +       spin_unlock(&suspend_fs_sync_lock);
>  }
>  static DECLARE_WORK(sync_filesystems, sync_filesystems_fn);
>
> @@ -432,8 +440,26 @@ static DECLARE_WORK(sync_filesystems, sync_filesystems_fn);
>   */
>  static int suspend_fs_sync_with_abort(void)
>  {
> +       bool need_suspend_fs_sync_requeue;
> +
> +Start_fs_sync:
> +       spin_lock(&suspend_fs_sync_lock);
>         reinit_completion(&suspend_fs_sync_complete);
> -       schedule_work(&sync_filesystems);
> +       /*
> +        * Handle the case where a suspend immediately follows a previous
> +        * suspend 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 suspending.
> +        */
> +       if (suspend_fs_sync_queued) {
> +               need_suspend_fs_sync_requeue = true;
> +       } else {
> +               need_suspend_fs_sync_requeue = false;
> +               suspend_fs_sync_queued = true;
> +               schedule_work(&sync_filesystems);
> +       }
> +       spin_unlock(&suspend_fs_sync_lock);
> +
>         /*
>          * Completion is triggered by fs_sync finishing or a suspend abort
>          * signal, whichever comes first
> @@ -441,6 +467,8 @@ static int suspend_fs_sync_with_abort(void)
>         wait_for_completion(&suspend_fs_sync_complete);
>         if (pm_wakeup_pending())
>                 return -EBUSY;
> +       if (need_suspend_fs_sync_requeue)
> +               goto Start_fs_sync;
>
>         return 0;
>  }
> --
> 2.51.0.261.g7ce5a0a67e-goog
>