Hi Kalesh,

On Wed, Jun 29, 2022 at 09:25:32PM -0700, Kalesh Singh wrote:
> On Wed, Jun 29, 2022 at 5:30 PM Jason A. Donenfeld <ja...@zx2c4.com> wrote:
> >
> > Hey again,
> >
> > On Thu, Jun 30, 2022 at 2:24 AM Jason A. Donenfeld <ja...@zx2c4.com> wrote:
> > > 1) Introduce a simple CONFIG_PM_CONTINUOUS_AUTOSLEEPING Kconfig thing
> > >    with lots of discouraging help text.
> > >
> > > 2) Go with the /sys/power tunable and bikeshed the naming of that a bit
> > >    to get it to something that reflects this better, and document it as
> > >    being undesirable except for Android phones.
> >
> > One other quick thought, which I had mentioned earlier to Kalesh:
> >
> > 3) Make the semantics a process holding open a file descriptor, rather
> >    than writing 0/1 into a file. It'd be called /sys/power/
> >    userspace_autosleep_ctrl, or something, and it'd enable this behavior
> >    while it's opened. And maybe down the line somebody will want to add
> >    ioctls to it for a different purpose. This way it's less of a tunable
> >    and more of an indication that there's a userspace app doing/controlling
> >    something.
> >
> > This idea (3) may be a lot of added complexity for basically nothing,
> > but it might fit the usage semantics concerns a bit better than (2). But
> > anyway, just an idea. Any one of those three are fine with me.
> Two concerns John raised:
>   1) Adding new ABI we need to maintain
>   2) Having unclear config options
> Another idea, I think, is to add the Kconfig option as
> CONFIG_SUSPEND_SKIP_SYNC and I think it would address those concerns.

I mentioned in my reply to him that this doesn't really work for me:

| As a general rule, I don't expose knobs like that in wireguard /itself/,
| but wireguard has no problem with adapting to whatever machine properties
| it finds itself on. And besides, this *is* a very definite device
| property, something really particular and peculiar about the machine
| the kernel is running on. It's a concrete thing that the kernel should
| know about. So let's go with your "very clear description idea", above,
| instead.

IOW, we're not going to add a tunable on every possible place this is

Anyway if you don't want a runtime switch, make a compiletime switch
called CONFIG_PM_CONTINUOUS_RAPID_AUTOSLEEPING or whatever, write some
very discouraging help text, and call it a day. And this way you don't
have to worry about ABI and we can change this later on and do the whole
thing as a no-big-deal change that somebody can tweak later without

The below diff is some boiler plate to help you get started with that
direction. Similar order of operations for this one:

1. You write a patch for Android's base config to enable this option and
   post it on Gerrit.

2. You take the diff below, clean it up or bikeshed the naming a bit or
   do whatever there, and submit it to the kernel, including as a `Link:
   ...` this thread and the Gerrit link.

3. When the patch lands, you submit the Gerrit CL.

4. When both have landed, Christoph moves forward with his
   CONFIG_ANDROID removal.

So really, just pick an option here -- the runtime switch or the
compiletime switch or the crazy fd thing I mentioned -- and run with it.


diff --git a/drivers/char/random.c b/drivers/char/random.c
index e3dd1dd3dd22..5332236cb1ad 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -756,7 +756,7 @@ static int random_pm_notification(struct notifier_block 
*nb, unsigned long actio

        if (crng_ready() && (action == PM_RESTORE_PREPARE ||
            (action == PM_POST_SUSPEND &&
                pr_notice("crng reseeded on system resumption\n");
diff --git a/drivers/net/wireguard/device.c b/drivers/net/wireguard/device.c
index aa9a7a5970fd..b93171f2e6c9 100644
--- a/drivers/net/wireguard/device.c
+++ b/drivers/net/wireguard/device.c
@@ -69,7 +69,7 @@ static int wg_pm_notification(struct notifier_block *nb, 
unsigned long action, v
         * its normal operation rather than as a somewhat rare event, then we
         * don't actually want to clear keys.
                return 0;

        if (action != PM_HIBERNATION_PREPARE && action != PM_SUSPEND_PREPARE)
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index a12779650f15..bcbfbeb39d4f 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -150,6 +150,25 @@ config PM_WAKELOCKS
        Allow user space to create, activate and deactivate wakeup source
        objects with the help of a sysfs-based interface.

+       bool "Tune for rapid and consistent userspace calls to sleep"
+       depends on PM_SLEEP
+       help
+       Change the behavior of various sleep-sensitive code to deal with
+       userspace autosuspend daemons that put the machine to sleep and wake it
+       up extremely often and for short periods of time.
+       This option mostly disables code paths that most users really should
+       keep enabled. In particular, only enable this if:
+       - It is very common to be asleep for only 2 seconds before being woken; 
+       - It is very common to be awake for only 2 seconds before sleeping.
+       This likely only applies to Android devices, and not other machines.
+       Therefore, you should say N here, unless you're extremely certain that
+       this is what you want. The option otherwise has bad, undesirable
+       effects, and should not be enabled just for fun.
        int "Maximum number of user space wakeup sources (0 = no limit)"
        range 0 100000

Reply via email to