Re: [RFC PATCH v1] PM: wakeup: Provide interface for userspace to abort suspend

[Saravana Kannan]

On Mon, Jul 21, 2025 at 12:51 PM Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:

Thanks for responding to my RFC!

>
> On Fri, Jul 18, 2025 at 9:18 AM Saravana Kannan <saravanak@xxxxxxxxxx> wrote:
> >
> > Once suspend starts, it can take a while before file system sync
> > finishes and all the userspace threads are frozen. During this time,
> > there can be events that originate in userspace that would require the
> > suspend to be aborted.
> >
> > The only way to abort suspend from userspace as of today is to grab
> > and release a kernel wakelock using the /sys/power/wake_lock and
> > /sys/power/wake_unlock files. This has the disadvantage of:
> >
> > * Doing the useless work of creating and destroying wakelocks.
> > * If the userspace entity crashes after the wake lock is created, we
> >   get a wake lock/memory leak.
>
> But wakelocks are for this purpose.

Right, wakelocks might be working as intended where they don't go away
if the userspace process crashes.

>
> > To avoid all this and simplify the interface, this patch allows
> > canceling a suspend by writing UINT_MAX value to the
> > /sys/power/wakeup_count that is meant for tracking wakeup events.
> >
> > Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>
> > ---
> >
> > Rafael,
> >
> > If the idea looks good to you, I can also update Documentation and sent
> > it as a non-RFC patch. I'm not too tied on what file we use to trigger
> > an abort from userspace as long as it's possible.
>
> I would rather add an interface based on a special device file for
> wakelocks to address this.

While the device file based approach for creating kernel wakelocks
from userspace might be cleaner, this patch isn't trying to address
that though. We already have a UAPI for that and the userspace suspend
managers use it.

>
> For example, open it to create a wakelock with the name of a calling
> process, write 1 to it to block suspending, write 0 to it to unblock,
> close to remove it.
>
> Then it will go away automatically when the process exits.

The scenario this patch is trying to address is to abort a suspend
that was initiated by userspace (system with CONFIG_PM_AUTOSLEEP
disabled). It's not really a kernel wakelock request asking the kernel
to keep the system from suspending -- it doesn't need to because
CONFIG_PM_AUTOSLEEP is disabled. And we certainly don't want to create
a kernel wakelock for every single user space wakelock. That's a lot
of unnecessary overhead.

Another way to look at this is that we have a way to initiate suspend
by writing "mem" to /sys/power/state, but we don't have any way to
abort it. Another version of this patch that I was considering was to
write "abort" to /sys/power/state to abort an ongoing suspend. Would
that version of the patch make more sense to you?

Thanks,
Saravana


>
> >  drivers/base/power/wakeup.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> > index d1283ff1080b..9316de561bcc 100644
> > --- a/drivers/base/power/wakeup.c
> > +++ b/drivers/base/power/wakeup.c
> > @@ -1008,6 +1008,8 @@ bool pm_save_wakeup_count(unsigned int count)
> >         if (cnt == count && inpr == 0) {
> >                 saved_count = count;
> >                 events_check_enabled = true;
> > +       } else if (cnt == UINT_MAX) {
> > +               pm_system_wakeup();
> >         }
> >         raw_spin_unlock_irqrestore(&events_lock, flags);
> >         return events_check_enabled;
> > --
> > 2.50.0.727.gbf7dc18ff4-goog
> >
> >