On Wed, Jun 29, 2022 at 4:02 PM Jason A. Donenfeld <Jason@xxxxxxxxx> wrote: > > Hi Kalesh, > > On Wed, Jun 29, 2022 at 03:26:33PM -0700, Kalesh Singh wrote: > > Thanks for taking a look. I'm concerned holding the sys/power/state > > open would have unintentional side effects. Adding the > > /sys/power/userspace_autosuspender seems more appropriate. We don't > > have a use case for the refcounting, so would prefer the simpler > > writing '0' / '1' to toggle semantics. > > Alright. So I've cooked you up some code that you can submit, since I > assume based on Christoph's bristliness that he won't do so. The below > adds /sys/power/pm_userspace_autosleeper, which you can write a 0 or a 1 > into, and fixes up wireguard and random.c to use it. The code is > untested, but should generally be the correct thing, I think. > > So in order of operations: > > 1. You write a patch for SystemSuspend.cpp 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 Rafael's PM tree, including as a > `Link: ...` this thread and the Gerrit link. > > 3. When/if Rafael accepts the patch, you submit the Gerrit CL. > > 4. When both have landed, Christoph moves forward with his > CONFIG_ANDROID removal. > > Does that seem like a reasonable way forward? Sounds like a plan. I'll clean up and repost your patch once the Gerrit change is ready. Thanks, Kalesh > > Jason > > diff --git a/drivers/char/random.c b/drivers/char/random.c > index e3dd1dd3dd22..c25e3be10d9c 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 && > - !IS_ENABLED(CONFIG_PM_AUTOSLEEP) && !IS_ENABLED(CONFIG_ANDROID)))) { > + !IS_ENABLED(CONFIG_PM_AUTOSLEEP) && !pm_userspace_autosleeper_enabled))) { > crng_reseed(); > pr_notice("crng reseeded on system resumption\n"); > } > diff --git a/drivers/net/wireguard/device.c b/drivers/net/wireguard/device.c > index aa9a7a5970fd..1983e0fadb6e 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. > */ > - if (IS_ENABLED(CONFIG_PM_AUTOSLEEP) || IS_ENABLED(CONFIG_ANDROID)) > + if (IS_ENABLED(CONFIG_PM_AUTOSLEEP) || pm_userspace_autosleeper_enabled) > return 0; > > if (action != PM_HIBERNATION_PREPARE && action != PM_SUSPEND_PREPARE) > diff --git a/include/linux/suspend.h b/include/linux/suspend.h > index 70f2921e2e70..0acff26f87b4 100644 > --- a/include/linux/suspend.h > +++ b/include/linux/suspend.h > @@ -498,6 +498,7 @@ extern void ksys_sync_helper(void); > /* drivers/base/power/wakeup.c */ > extern bool events_check_enabled; > extern suspend_state_t pm_suspend_target_state; > +extern bool pm_userspace_autosleeper_enabled; > > extern bool pm_wakeup_pending(void); > extern void pm_system_wakeup(void); > @@ -537,6 +538,8 @@ static inline void pm_system_irq_wakeup(unsigned int irq_number) {} > static inline void lock_system_sleep(void) {} > static inline void unlock_system_sleep(void) {} > > +#define pm_userspace_autosleeper_enabled (false) > + > #endif /* !CONFIG_PM_SLEEP */ > > #ifdef CONFIG_PM_SLEEP_DEBUG > diff --git a/kernel/power/main.c b/kernel/power/main.c > index e3694034b753..08f32a281010 100644 > --- a/kernel/power/main.c > +++ b/kernel/power/main.c > @@ -120,6 +120,23 @@ static ssize_t pm_async_store(struct kobject *kobj, struct kobj_attribute *attr, > > power_attr(pm_async); > > +bool pm_userspace_autosleeper_enabled; > + > +static ssize_t pm_userspace_autosleeper_show(struct kobject *kobj, > + struct kobj_attribute *attr, char *buf) > +{ > + return sprintf(buf, "%d\n", pm_userspace_autosleeper_enabled); > +} > + > +static ssize_t pm_userspace_autosleeper_store(struct kobject *kobj, > + struct kobj_attribute *attr, > + const char *buf, size_t n) > +{ > + return kstrtobool(buf, &pm_userspace_autosleeper_enabled); > +} > + > +power_attr(pm_userspace_autosleeper); > + > #ifdef CONFIG_SUSPEND > static ssize_t mem_sleep_show(struct kobject *kobj, struct kobj_attribute *attr, > char *buf) > @@ -869,6 +886,7 @@ static struct attribute * g[] = { > #ifdef CONFIG_PM_SLEEP > &pm_async_attr.attr, > &wakeup_count_attr.attr, > + &pm_userspace_autosleeper.attr, > #ifdef CONFIG_SUSPEND > &mem_sleep_attr.attr, > &sync_on_suspend_attr.attr, >