Re: [RFC][PATCH 5/7] PM / Sleep: Implement opportunistic sleep

Srivatsa S. Bhat
2012-02-23T05:36:17Z

On 02/23/2012 03:40 AM, Rafael J. Wysocki wrote:

> On Wednesday, February 22, 2012, Srivatsa S. Bhat wrote:
>> On 02/22/2012 05:05 AM, Rafael J. Wysocki wrote:
>>
>>> From: Rafael J. Wysocki <rjw@sisk.pl>
>>>
>>> Introduce a mechanism by which the kernel can trigger global
>>> transitions to a sleep state chosen by user space if there are no
>>> active wakeup sources.
>>>
>>> It consists of a new sysfs attribute, /sys/power/autosleep, that
>>> can be written one of the strings returned by reads from
>>> /sys/power/state, an ordered workqueue and a work item carrying out
>>> the "suspend" operations. If a string representing the system's
>>> sleep state is written to /sys/power/autosleep, the work item
>>> triggering transitions to that state is queued up and it requeues
>>> itself after every execution until user space writes "off" to
>>> /sys/power/autosleep.
>>>
>>> That work item enables the detection of wakeup events using the
>>> functions already defined in drivers/base/power/wakeup.c (with one
>>> small modification) and calls either pm_suspend(), or hibernate() to
>>> put the system into a sleep state. If a wakeup event is reported
>>> while the transition is in progress, it will abort the transition and
>>> the "system suspend" work item will be queued up again.
>>>
>>> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>>> ---
>>> Documentation/ABI/testing/sysfs-power | 17 +++++
>>> drivers/base/power/wakeup.c | 38 ++++++-----
>>> include/linux/suspend.h | 13 +++-
>>> kernel/power/Kconfig | 8 ++
>>> kernel/power/Makefile | 1
>>> kernel/power/autosleep.c | 98 ++++++++++++++++++++++++++++++
>>> kernel/power/main.c | 108 ++++++++++++++++++++++++++++------
>>> kernel/power/power.h | 18 +++++
>>> 8 files changed, 266 insertions(+), 35 deletions(-)
>>>
>>> Index: linux/kernel/power/Makefile
>>> ===================================================================
>>> --- linux.orig/kernel/power/Makefile
>>> +++ linux/kernel/power/Makefile
>>> @@ -9,5 +9,6 @@ obj-$(CONFIG_SUSPEND) += suspend.o
>>> obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o
>>> obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o user.o \
>>> block_io.o
>>> +obj-$(CONFIG_PM_AUTOSLEEP) += autosleep.o
>>>
>>> obj-$(CONFIG_MAGIC_SYSRQ) += poweroff.o
>>> Index: linux/kernel/power/Kconfig
>>> ===================================================================
>>> --- linux.orig/kernel/power/Kconfig
>>> +++ linux/kernel/power/Kconfig
>>> @@ -103,6 +103,14 @@ config PM_SLEEP_SMP
>>> select HOTPLUG
>>> select HOTPLUG_CPU
>>>
>>> +config PM_AUTOSLEEP
>>> + bool "Opportunistic sleep"
>>> + depends on PM_SLEEP
>>> + default n
>>> + ---help---
>>> + Allow the kernel to trigger a system transition into a global sleep
>>> + state automatically whenever there are no active wakeup sources.
>>> +
>>> config PM_RUNTIME
>>> bool "Run-time PM core functionality"
>>> depends on !IA64_HP_SIM
>>> Index: linux/kernel/power/power.h
>>> ===================================================================
>>> --- linux.orig/kernel/power/power.h
>>> +++ linux/kernel/power/power.h
>>> @@ -264,3 +264,21 @@ static inline void suspend_thaw_processe
>>> {
>>> }
>>> #endif
>>> +
>>> +#ifdef CONFIG_PM_AUTOSLEEP
>>> +
>>> +/* kernel/power/autosleep.c */
>>> +extern int pm_autosleep_init(void);
>>> +extern void pm_autosleep_lock(void);
>>> +extern void pm_autosleep_unlock(void);
>>> +extern suspend_state_t pm_autosleep_state(void);
>>> +extern int pm_autosleep_set_state(suspend_state_t state);
>>> +
>>> +#else /* !CONFIG_PM_AUTOSLEEP */
>>> +
>>> +static inline int pm_autosleep_init(void) { return 0; }
>>> +static inline void pm_autosleep_lock(void) {}
>>> +static inline void pm_autosleep_unlock(void) {}
>>> +static inline suspend_state_t pm_autosleep_state(void) { return PM_SUSPEND_ON; }
>>> +
>>> +#endif /* !CONFIG_PM_AUTOSLEEP */
>>> Index: linux/include/linux/suspend.h
>>> ===================================================================
>>> --- linux.orig/include/linux/suspend.h
>>> +++ linux/include/linux/suspend.h
>>> @@ -356,7 +356,7 @@ extern int unregister_pm_notifier(struct
>>> extern bool events_check_enabled;
>>>
>>> extern bool pm_wakeup_pending(void);
>>> -extern bool pm_get_wakeup_count(unsigned int *count);
>>> +extern bool pm_get_wakeup_count(unsigned int *count, bool block);
>>> extern bool pm_save_wakeup_count(unsigned int count);
>>>
>>> static inline void lock_system_sleep(void)
>>> @@ -407,6 +407,17 @@ static inline void unlock_system_sleep(v
>>>
>>> #endif /* !CONFIG_PM_SLEEP */
>>>
>>> +#ifdef CONFIG_PM_AUTOSLEEP
>>> +
>>> +/* kernel/power/autosleep.c */
>>> +void queue_up_suspend_work(void);
>>> +
>>> +#else /* !CONFIG_PM_AUTOSLEEP */
>>> +
>>> +static inline void queue_up_suspend_work(void) {}
>>> +
>>> +#endif /* !CONFIG_PM_AUTOSLEEP */
>>> +
>>> #ifdef CONFIG_ARCH_SAVE_PAGE_KEYS
>>> /*
>>> * The ARCH_SAVE_PAGE_KEYS functions can be used by an architecture
>>> Index: linux/kernel/power/autosleep.c
>>> ===================================================================
>>> --- /dev/null
>>> +++ linux/kernel/power/autosleep.c
>>> @@ -0,0 +1,98 @@
>>> +/*
>>> + * kernel/power/autosleep.c
>>> + *
>>> + * Opportunistic sleep support.
>>> + *
>>> + * Copyright (C) 2012 Rafael J. Wysocki <rjw@sisk.pl>
>>> + */
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/pm_wakeup.h>
>>> +
>>> +#include "power.h"
>>> +
>>> +static suspend_state_t autosleep_state;
>>> +static struct workqueue_struct *autosleep_wq;
>>> +static DEFINE_MUTEX(autosleep_lock);
>>> +
>>> +static void try_to_suspend(struct work_struct *work)
>>> +{
>>> + unsigned int initial_count, final_count;
>>> +
>>> + if (!pm_get_wakeup_count(&initial_count, true))
>>> + goto out;
>>> +
>>> + mutex_lock(&autosleep_lock);
>>> +
>>> + if (!pm_save_wakeup_count(initial_count)) {
>>> + mutex_unlock(&autosleep_lock);
>>> + goto out;
>>> + }
>>> +
>>> + if (autosleep_state == PM_SUSPEND_ON) {
>>> + mutex_unlock(&autosleep_lock);
>>> + return;
>>> + }
>>> + if (autosleep_state >= PM_SUSPEND_MAX)
>>> + hibernate();
>>> + else
>>> + pm_suspend(autosleep_state);
>>
>>
>> We are calling pm_suspend() or hibernate() directly here.
>> Won't this break build when CONFIG_SUSPEND or CONFIG_HIBERNATION is not set?
>> CONFIG_PM_AUTOSLEEP depends only on PM_SLEEP which means we could enable
>> either one of suspend or hibernation and yet come to this point, breaking
>> the option which was not enabled.
>
> Both pm_suspend() and hibernate() have appropriate static inline definitions
> for !CONFIG_SUSPEND and !CONFIG_HIBERNATION (in suspend.h), as far as I can say.
>

Oh, you are right.. I overlooked that, sorry!

Regards,
Srivatsa S. Bhat