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

Message-ID: <CAJZ5v0i_is4JZFGZcZhc3oiCzeUygKziPEtw5MwF5aZZxdPdvA@mail.gmail.com>
Date: Wed, 29 Oct 2025 21:35:28 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Samuel Wu <wusamuel@...gle.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, 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 8:13 PM Samuel Wu <wusamuel@...gle.com> wrote:
>
> 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.

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).