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: <CAG2Kcto4XPWMqf_ALZht9wivKki7bNKgCwijLruTsj8CDHaCgA@mail.gmail.com>
Date: Wed, 29 Oct 2025 12:03:22 -0700
From: Samuel Wu <wusamuel@...gle.com>
To: Hillf Danton <hdanton@...a.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>, 
	Saravana Kannan <saravanak@...gle.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 22, 2025 at 3:32 PM Hillf Danton <hdanton@...a.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@...a.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@...a.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