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

[Hillf Danton <hdanton@sina.com>]

On Wed, 22 Oct 2025 11:41:37 -0700 Samuel Wu wrote:
> On Tue, Oct 21, 2025 at 6:16 PM Hillf Danton <hdanton@sina.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@sina.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

> > > > > +             /*
> > > > > +              * Handle the case where a sleep immediately follows a previous
> > > > > +              * sleep 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 sleeping.
> > > > > +              */
> > > > > +             if (pm_sleep_fs_sync_queued) {
> > > > > +                     need_pm_sleep_fs_sync_requeue = true;
> > > > > +             } else {
> > > > > +                     need_pm_sleep_fs_sync_requeue = false;
> > > > > +                     pm_sleep_fs_sync_queued = true;
> > > > > +                     schedule_work(&sync_filesystems);
> > > > > +             }
> > > > > +             spin_unlock_irqrestore(&pm_sleep_fs_sync_lock, flags);
> > > > > +
> > > > > +             /*
> > > > > +              * Completion is triggered by fs_sync finishing or an abort sleep
> > > > > +              * signal, whichever comes first
> > > > > +              */
> > > > > +             wait_for_completion(&pm_sleep_fs_sync_complete);
> > > > > +             if (pm_wakeup_pending())
> > > > > +                     return -EBUSY;
> > > > > +     } while (need_pm_sleep_fs_sync_requeue);
> > > > > +
> > > > > +     return 0;
> > > > > +}