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

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAG2KctqoOPg4E6dN0UCjkTF8w0hC7FUwfYOWkw+i37G8OxcttQ@mail.gmail.com>
Date: Wed, 29 Oct 2025 12:13:05 -0700
From: Samuel Wu <wusamuel@...gle.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Saravana Kannan <saravanak@...gle.com>, Pavel Machek <pavel@...nel.org>, Len Brown <lenb@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Danilo Krummrich <dakr@...nel.org>, tuhaowen@...ontech.com, 
	kernel-team@...roid.com, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] PM: Support aborting sleep during filesystem sync

On Wed, Oct 29, 2025 at 6:23 AM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Fri, Oct 24, 2025 at 10:37 AM Rafael J. Wysocki <rafael@...nel.org> wrote:
> >
> > On Fri, Oct 24, 2025 at 12:47 AM Saravana Kannan <saravanak@...gle.com> wrote:
> > >
> > > On Thu, Oct 23, 2025 at 12:43 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
> > > >
> > > > On Sat, Oct 18, 2025 at 1:39 AM Samuel Wu <wusamuel@...gle.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.