PM: Add suspend block api.
in [Kernel]
|
Prev: A share can be mounted multiple times on the same mountpoint when using the option 'noac'
Next: parport_pc.c and parport_serial.c need improvements
From: Oleg Nesterov on 30 Apr 2010 13:40 On 04/30, Tejun Heo wrote: > > On 04/29/2010 11:16 PM, Rafael J. Wysocki wrote: > >>> Also, we already have one PM workqueue. It is used for runtime PM, but I guess > >>> it may be used just as well for the opportunistic suspend. It is freezable, > >>> but would it hurt? > >> > > Freezable workqueues have to be singlethread or else there will be unfixable > > races, so you can safely assume things will stay as they are in this respect. > > Rafael, can you elaborate a bit more on this? Just in case I missed > something while doing cmwq as it currently doesn't have such limit. Currently _cpu_down() can't flush and/or stop the frozen cwq->thread. IIRC this is fixable, but needs the nasty complications. We should thaw + stop the frozen cwq->thread, then move the pending works to another CPU. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Pavel Machek on 30 Apr 2010 14:00 Hi! > Adds /sys/power/policy that selects the behaviour of /sys/power/state. > After setting the policy to opportunistic, writes to /sys/power/state > become non-blocking requests that specify which suspend state to enter > when no suspend blockers are active. A special state, "on", stops the > process by activating the "main" suspend blocker. I really don't like how this changes semantics of 'state'. I guess I'd prefer leaving state as is -- forced transition to hibernation while system is set to opportunistically suspend seems sane -- and adding something like /sys/power/autosleep with 'off' or 'suspend' values? > > Signed-off-by: Arve Hj??nnev??g <arve(a)android.com> > --- > Documentation/power/opportunistic-suspend.txt | 114 +++++++++++ > include/linux/suspend_blocker.h | 64 ++++++ > kernel/power/Kconfig | 16 ++ > kernel/power/Makefile | 1 + > kernel/power/main.c | 89 ++++++++- > kernel/power/power.h | 5 + > kernel/power/suspend.c | 4 +- > kernel/power/suspend_blocker.c | 269 +++++++++++++++++++++++++ > 8 files changed, 556 insertions(+), 6 deletions(-) > create mode 100644 Documentation/power/opportunistic-suspend.txt > create mode 100755 include/linux/suspend_blocker.h > create mode 100644 kernel/power/suspend_blocker.c > > diff --git a/Documentation/power/opportunistic-suspend.txt b/Documentation/power/opportunistic-suspend.txt > new file mode 100644 > index 0000000..1a29d10 > --- /dev/null > +++ b/Documentation/power/opportunistic-suspend.txt > @@ -0,0 +1,114 @@ > +Opportunistic Suspend > +===================== > + > +Opportunistic suspend is a feature allowing the system to be suspended (ie. put > +into one of the available sleep states) automatically whenever it is regarded > +as idle. The suspend blockers framework described below is used to determine > +when that happens. > + > +The /sys/power/policy sysfs attribute is used to switch the system between the > +opportunistic and "forced" suspend behavior, where in the latter case the > +system is only suspended if a specific value, corresponding to one of the > +available system sleep states, is written into /sys/power/state. However, in > +the former, opportunistic, case the system is put into the sleep state > +corresponding to the value written to /sys/power/state whenever there are no > +active suspend blockers. The default policy is "forced". Also, suspend blockers > +do not affect sleep states entered from idle. > + > +When the policy is "opportunisic", there is a special value, "on", that can be > +written to /sys/power/state. This will block the automatic sleep request, as if > +a suspend blocker was used by a device driver. This way the opportunistic > +suspend may be blocked by user space whithout switching back to the "forced" > +mode. > + > +A suspend blocker is an object used to inform the PM subsystem when the system > +can or cannot be suspended in the "opportunistic" mode (the "forced" mode > +ignores suspend blockers). To use it, a device driver creates a struct > +suspend_blocker that must be initialized with suspend_blocker_init(). Before > +freeing the suspend_blocker structure or its name, suspend_blocker_destroy() > +must be called on it. > + > +A suspend blocker is activated using suspend_block(), which prevents the PM > +subsystem from putting the system into the requested sleep state in the > +"opportunistic" mode until the suspend blocker is deactivated with > +suspend_unblock(). Multiple suspend blockers may be active simultaneously, and > +the system will not suspend as long as at least one of them is active. > + > +If opportunistic suspend is already in progress when suspend_block() is called, > +it will abort the suspend, unless suspend_ops->enter has already been > +executed. If suspend is aborted this way, the system is usually not fully > +operational at that point. The suspend callbacks of some drivers may still be > +running and it usually takes time to restore the system to the fully operational > +state. > + > +For example, in cell phones or other embedded systems, where powering the screen > +is a significant drain on the battery, suspend blockers can be used to allow > +user-space to decide whether a keystroke received while the system is suspended > +should cause the screen to be turned back on or allow the system to go back into > +suspend. Use set_irq_wake or a platform specific api to make sure the keypad > +interrupt wakes up the cpu. Once the keypad driver has resumed, the sequence of > +events can look like this: > + > +- The Keypad driver gets an interrupt. It then calls suspend_block on the > + keypad-scan suspend_blocker and starts scanning the keypad matrix. > +- The keypad-scan code detects a key change and reports it to the input-event > + driver. > +- The input-event driver sees the key change, enqueues an event, and calls > + suspend_block on the input-event-queue suspend_blocker. > +- The keypad-scan code detects that no keys are held and calls suspend_unblock > + on the keypad-scan suspend_blocker. > +- The user-space input-event thread returns from select/poll, calls > + suspend_block on the process-input-events suspend_blocker and then calls read > + on the input-event device. > +- The input-event driver dequeues the key-event and, since the queue is now > + empty, it calls suspend_unblock on the input-event-queue suspend_blocker. > +- The user-space input-event thread returns from read. If it determines that > + the key should leave the screen off, it calls suspend_unblock on the > + process_input_events suspend_blocker and then calls select or poll. The > + system will automatically suspend again, since now no suspend blockers are > + active. > + > + Key pressed Key released > + | | > +keypad-scan ++++++++++++++++++ > +input-event-queue +++ +++ > +process-input-events +++ +++ > + > + > +Driver API > +========== > + > +A driver can use the suspend block api by adding a suspend_blocker variable to > +its state and calling suspend_blocker_init. For instance: > +struct state { > + struct suspend_blocker suspend_blocker; > +} > + > +init() { > + suspend_blocker_init(&state->suspend_blocker, "suspend-blocker-name"); > +} > + > +Before freeing the memory, suspend_blocker_destroy must be called: > + > +uninit() { > + suspend_blocker_destroy(&state->suspend_blocker); > +} > + > +When the driver determines that it needs to run (usually in an interrupt > +handler) it calls suspend_block: > + suspend_block(&state->suspend_blocker); > + > +When it no longer needs to run it calls suspend_unblock: > + suspend_unblock(&state->suspend_blocker); > + > +Calling suspend_block when the suspend blocker is active or suspend_unblock when > +it is not active has no effect (i.e., these functions don't nest). This allows > +drivers to update their state and call suspend suspend_block or suspend_unblock > +based on the result. > +For instance: > + > +if (list_empty(&state->pending_work)) > + suspend_unblock(&state->suspend_blocker); > +else > + suspend_block(&state->suspend_blocker); > + > diff --git a/include/linux/suspend_blocker.h b/include/linux/suspend_blocker.h > new file mode 100755 > index 0000000..f9928cc > --- /dev/null > +++ b/include/linux/suspend_blocker.h > @@ -0,0 +1,64 @@ > +/* include/linux/suspend_blocker.h > + * > + * Copyright (C) 2007-2009 Google, Inc. > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#ifndef _LINUX_SUSPEND_BLOCKER_H > +#define _LINUX_SUSPEND_BLOCKER_H > + > +#include <linux/list.h> > + > +/** > + * struct suspend_blocker - the basic suspend_blocker structure > + * @link: List entry for active or inactive list. > + * @flags: Tracks initialized and active state. > + * @name: Name used for debugging. > + * > + * When a suspend_blocker is active it prevents the system from entering > + * opportunistic suspend. > + * > + * The suspend_blocker structure must be initialized by suspend_blocker_init() > + */ > + > +struct suspend_blocker { > +#ifdef CONFIG_OPPORTUNISTIC_SUSPEND > + struct list_head link; > + int flags; > + const char *name; > +#endif > +}; > + > +#ifdef CONFIG_OPPORTUNISTIC_SUSPEND > + > +void suspend_blocker_init(struct suspend_blocker *blocker, const char *name); > +void suspend_blocker_destroy(struct suspend_blocker *blocker); > +void suspend_block(struct suspend_blocker *blocker); > +void suspend_unblock(struct suspend_blocker *blocker); > +bool suspend_blocker_is_active(struct suspend_blocker *blocker); > +bool suspend_is_blocked(void); > + > +#else > + > +static inline void suspend_blocker_init(struct suspend_blocker *blocker, > + const char *name) {} > +static inline void suspend_blocker_destroy(struct suspend_blocker *blocker) {} > +static inline void suspend_block(struct suspend_blocker *blocker) {} > +static inline void suspend_unblock(struct suspend_blocker *blocker) {} > +static inline bool suspend_blocker_is_active(struct suspend_blocker *bl) > + { return 0; } > +static inline bool suspend_is_blocked(void) { return 0; } > + > +#endif > + > +#endif > + > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig > index 5c36ea9..55a06a1 100644 > --- a/kernel/power/Kconfig > +++ b/kernel/power/Kconfig > @@ -130,6 +130,22 @@ config SUSPEND_FREEZER > > Turning OFF this setting is NOT recommended! If in doubt, say Y. > > +config OPPORTUNISTIC_SUSPEND > + bool "Suspend blockers" > + depends on PM_SLEEP > + select RTC_LIB > + default n > + ---help--- > + Opportunistic sleep support. Allows the system to be put into a sleep > + state opportunistically, if it doesn't do any useful work at the > + moment. The PM subsystem is switched into this mode of operation by > + writing "opportunistic" into /sys/power/policy, while writing > + "forced" to this file turns the opportunistic suspend feature off. > + In the "opportunistic" mode suspend blockers are used to determine > + when to suspend the system and the value written to /sys/power/state > + determines the sleep state the system will be put into when there are > + no active suspend blockers. > + > config HIBERNATION_NVS > bool > > diff --git a/kernel/power/Makefile b/kernel/power/Makefile > index 4319181..ee5276d 100644 > --- a/kernel/power/Makefile > +++ b/kernel/power/Makefile > @@ -7,6 +7,7 @@ obj-$(CONFIG_PM) += main.o > obj-$(CONFIG_PM_SLEEP) += console.o > obj-$(CONFIG_FREEZER) += process.o > obj-$(CONFIG_SUSPEND) += suspend.o > +obj-$(CONFIG_OPPORTUNISTIC_SUSPEND) += suspend_blocker.o > obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o > obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o user.o > obj-$(CONFIG_HIBERNATION_NVS) += hibernate_nvs.o > diff --git a/kernel/power/main.c b/kernel/power/main.c > index b58800b..5f0af6c 100644 > --- a/kernel/power/main.c > +++ b/kernel/power/main.c > @@ -12,6 +12,7 @@ > #include <linux/string.h> > #include <linux/resume-trace.h> > #include <linux/workqueue.h> > +#include <linux/suspend_blocker.h> > > #include "power.h" > > @@ -20,6 +21,27 @@ DEFINE_MUTEX(pm_mutex); > unsigned int pm_flags; > EXPORT_SYMBOL(pm_flags); > > +struct policy { > + const char *name; > + bool (*valid_state)(suspend_state_t state); > + int (*set_state)(suspend_state_t state); > +}; > +static struct policy policies[] = { > + { > + .name = "forced", > + .valid_state = valid_state, > + .set_state = enter_state, > + }, > +#ifdef CONFIG_OPPORTUNISTIC_SUSPEND > + { > + .name = "opportunistic", > + .valid_state = request_suspend_valid_state, > + .set_state = request_suspend_state, > + }, > +#endif > +}; > +static int policy; > + > #ifdef CONFIG_PM_SLEEP > > /* Routines for PM-transition notifications */ > @@ -146,6 +168,12 @@ struct kobject *power_kobj; > * > * store() accepts one of those strings, translates it into the > * proper enumerated value, and initiates a suspend transition. > + * > + * If policy is set to opportunistic, store() does not block until the > + * system resumes, and it will try to re-enter the state until another > + * state is requested. Suspend blockers are respected and the requested > + * state will only be entered when no suspend blockers are active. > + * Write "on" to cancel. > */ > static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr, > char *buf) > @@ -155,12 +183,13 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr, > int i; > > for (i = 0; i < PM_SUSPEND_MAX; i++) { > - if (pm_states[i] && valid_state(i)) > + if (pm_states[i] && policies[policy].valid_state(i)) > s += sprintf(s,"%s ", pm_states[i]); > } > #endif > #ifdef CONFIG_HIBERNATION > - s += sprintf(s, "%s\n", "disk"); > + if (!policy) > + s += sprintf(s, "%s\n", "disk"); > #else > if (s != buf) > /* convert the last space to a newline */ > @@ -173,7 +202,7 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, > const char *buf, size_t n) > { > #ifdef CONFIG_SUSPEND > - suspend_state_t state = PM_SUSPEND_STANDBY; > + suspend_state_t state = PM_SUSPEND_ON; > const char * const *s; > #endif > char *p; > @@ -184,7 +213,7 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, > len = p ? p - buf : n; > > /* First, check if we are requested to hibernate */ > - if (len == 4 && !strncmp(buf, "disk", len)) { > + if (len == 4 && !strncmp(buf, "disk", len) && !policy) { > error = hibernate(); > goto Exit; > } > @@ -195,7 +224,7 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, > break; > } > if (state < PM_SUSPEND_MAX && *s) > - error = enter_state(state); > + error = policies[policy].set_state(state); > #endif > > Exit: > @@ -204,6 +233,55 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr, > > power_attr(state); > > +/** > + * policy - set policy for state > + */ > + > +static ssize_t policy_show(struct kobject *kobj, > + struct kobj_attribute *attr, char *buf) > +{ > + char *s = buf; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(policies); i++) { > + if (i == policy) > + s += sprintf(s, "[%s] ", policies[i].name); > + else > + s += sprintf(s, "%s ", policies[i].name); > + } > + if (s != buf) > + /* convert the last space to a newline */ > + *(s-1) = '\n'; > + return (s - buf); > +} > + > +static ssize_t policy_store(struct kobject *kobj, > + struct kobj_attribute *attr, > + const char *buf, size_t n) > +{ > + const char *s; > + char *p; > + int len; > + int i; > + > + p = memchr(buf, '\n', n); > + len = p ? p - buf : n; > + > + for (i = 0; i < ARRAY_SIZE(policies); i++) { > + s = policies[i].name; > + if (s && len == strlen(s) && !strncmp(buf, s, len)) { > + mutex_lock(&pm_mutex); > + policies[policy].set_state(PM_SUSPEND_ON); > + policy = i; > + mutex_unlock(&pm_mutex); > + return n; > + } > + } > + return -EINVAL; > +} > + > +power_attr(policy); > + > #ifdef CONFIG_PM_TRACE > int pm_trace_enabled; > > @@ -231,6 +309,7 @@ power_attr(pm_trace); > > static struct attribute * g[] = { > &state_attr.attr, > + &policy_attr.attr, > #ifdef CONFIG_PM_TRACE > &pm_trace_attr.attr, > #endif > diff --git a/kernel/power/power.h b/kernel/power/power.h > index 46c5a26..9b468d7 100644 > --- a/kernel/power/power.h > +++ b/kernel/power/power.h > @@ -236,3 +236,8 @@ static inline void suspend_thaw_processes(void) > { > } > #endif > + > +/* kernel/power/suspend_block.c */ > +extern int request_suspend_state(suspend_state_t state); > +extern bool request_suspend_valid_state(suspend_state_t state); > + > diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c > index 56e7dbb..dc42006 100644 > --- a/kernel/power/suspend.c > +++ b/kernel/power/suspend.c > @@ -16,10 +16,12 @@ > #include <linux/cpu.h> > #include <linux/syscalls.h> > #include <linux/gfp.h> > +#include <linux/suspend_blocker.h> > > #include "power.h" > > const char *const pm_states[PM_SUSPEND_MAX] = { > + [PM_SUSPEND_ON] = "on", > [PM_SUSPEND_STANDBY] = "standby", > [PM_SUSPEND_MEM] = "mem", > }; > @@ -157,7 +159,7 @@ static int suspend_enter(suspend_state_t state) > > error = sysdev_suspend(PMSG_SUSPEND); > if (!error) { > - if (!suspend_test(TEST_CORE)) > + if (!suspend_is_blocked() && !suspend_test(TEST_CORE)) > error = suspend_ops->enter(state); > sysdev_resume(); > } > diff --git a/kernel/power/suspend_blocker.c b/kernel/power/suspend_blocker.c > new file mode 100644 > index 0000000..9459361 > --- /dev/null > +++ b/kernel/power/suspend_blocker.c > @@ -0,0 +1,269 @@ > +/* kernel/power/suspend_blocker.c > + * > + * Copyright (C) 2005-2010 Google, Inc. > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +#include <linux/module.h> > +#include <linux/rtc.h> > +#include <linux/suspend.h> > +#include <linux/suspend_blocker.h> > +#include "power.h" > + > +enum { > + DEBUG_EXIT_SUSPEND = 1U << 0, > + DEBUG_WAKEUP = 1U << 1, > + DEBUG_USER_STATE = 1U << 2, > + DEBUG_SUSPEND = 1U << 3, > + DEBUG_SUSPEND_BLOCKER = 1U << 4, > +}; > +static int debug_mask = DEBUG_EXIT_SUSPEND | DEBUG_WAKEUP | DEBUG_USER_STATE; > +module_param_named(debug_mask, debug_mask, int, S_IRUGO | S_IWUSR | S_IWGRP); > + > +#define SB_INITIALIZED (1U << 8) > +#define SB_ACTIVE (1U << 9) > + > +static DEFINE_SPINLOCK(list_lock); > +static DEFINE_SPINLOCK(state_lock); > +static LIST_HEAD(inactive_blockers); > +static LIST_HEAD(active_blockers); > +static int current_event_num; > +struct workqueue_struct *suspend_work_queue; > +struct suspend_blocker main_suspend_blocker; > +static suspend_state_t requested_suspend_state = PM_SUSPEND_MEM; > +static bool enable_suspend_blockers; > + > +#define pr_info_time(fmt, args...) \ > + do { \ > + struct timespec ts; \ > + struct rtc_time tm; \ > + getnstimeofday(&ts); \ > + rtc_time_to_tm(ts.tv_sec, &tm); \ > + pr_info(fmt "(%d-%02d-%02d %02d:%02d:%02d.%09lu UTC)\n" , \ > + args, \ > + tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday, \ > + tm.tm_hour, tm.tm_min, tm.tm_sec, ts.tv_nsec); \ > + } while (0); > + > +static void print_active_blockers_locked(void) > +{ > + struct suspend_blocker *blocker; > + > + list_for_each_entry(blocker, &active_blockers, link) > + pr_info("active suspend blocker %s\n", blocker->name); > +} > + > +/** > + * suspend_is_blocked() - Check if suspend should be blocked > + * > + * suspend_is_blocked can be used by generic power management code to abort > + * suspend. > + * > + * To preserve backward compatibility suspend_is_blocked returns 0 unless it > + * is called during suspend initiated from the suspend_block code. > + */ > +bool suspend_is_blocked(void) > +{ > + if (!enable_suspend_blockers) > + return 0; > + return !list_empty(&active_blockers); > +} > + > +static void suspend_worker(struct work_struct *work) > +{ > + int ret; > + int entry_event_num; > + > + enable_suspend_blockers = true; > + while (!suspend_is_blocked()) { > + entry_event_num = current_event_num; > + > + if (debug_mask & DEBUG_SUSPEND) > + pr_info("suspend: enter suspend\n"); > + > + ret = pm_suspend(requested_suspend_state); > + > + if (debug_mask & DEBUG_EXIT_SUSPEND) > + pr_info_time("suspend: exit suspend, ret = %d ", ret); > + > + if (current_event_num == entry_event_num) > + pr_info("suspend: pm_suspend returned with no event\n"); > + } > + enable_suspend_blockers = false; > +} > +static DECLARE_WORK(suspend_work, suspend_worker); > + > +/** > + * suspend_blocker_init() - Initialize a suspend blocker > + * @blocker: The suspend blocker to initialize. > + * @name: The name of the suspend blocker to show in debug messages. > + * > + * The suspend blocker struct and name must not be freed before calling > + * suspend_blocker_destroy. > + */ > +void suspend_blocker_init(struct suspend_blocker *blocker, const char *name) > +{ > + unsigned long irqflags = 0; > + > + WARN_ON(!name); > + > + if (debug_mask & DEBUG_SUSPEND_BLOCKER) > + pr_info("suspend_blocker_init name=%s\n", name); > + > + blocker->name = name; > + blocker->flags = SB_INITIALIZED; > + INIT_LIST_HEAD(&blocker->link); > + > + spin_lock_irqsave(&list_lock, irqflags); > + list_add(&blocker->link, &inactive_blockers); > + spin_unlock_irqrestore(&list_lock, irqflags); > +} > +EXPORT_SYMBOL(suspend_blocker_init); > + > +/** > + * suspend_blocker_destroy() - Destroy a suspend blocker > + * @blocker: The suspend blocker to destroy. > + */ > +void suspend_blocker_destroy(struct suspend_blocker *blocker) > +{ > + unsigned long irqflags; > + if (WARN_ON(!(blocker->flags & SB_INITIALIZED))) > + return; > + > + if (debug_mask & DEBUG_SUSPEND_BLOCKER) > + pr_info("suspend_blocker_destroy name=%s\n", blocker->name); > + > + spin_lock_irqsave(&list_lock, irqflags); > + blocker->flags &= ~SB_INITIALIZED; > + list_del(&blocker->link); > + if ((blocker->flags & SB_ACTIVE) && list_empty(&active_blockers)) > + queue_work(suspend_work_queue, &suspend_work); > + spin_unlock_irqrestore(&list_lock, irqflags); > +} > +EXPORT_SYMBOL(suspend_blocker_destroy); > + > +/** > + * suspend_block() - Block suspend > + * @blocker: The suspend blocker to use > + * > + * It is safe to call this function from interrupt context. > + */ > +void suspend_block(struct suspend_blocker *blocker) > +{ > + unsigned long irqflags; > + > + if (WARN_ON(!(blocker->flags & SB_INITIALIZED))) > + return; > + > + spin_lock_irqsave(&list_lock, irqflags); > + blocker->flags |= SB_ACTIVE; > + list_del(&blocker->link); > + > + if (debug_mask & DEBUG_SUSPEND_BLOCKER) > + pr_info("suspend_block: %s\n", blocker->name); > + > + list_add(&blocker->link, &active_blockers); > + > + current_event_num++; > + spin_unlock_irqrestore(&list_lock, irqflags); > +} > +EXPORT_SYMBOL(suspend_block); > + > +/** > + * suspend_unblock() - Unblock suspend > + * @blocker: The suspend blocker to unblock. > + * > + * If no other suspend blockers block suspend, the system will suspend. > + * > + * It is safe to call this function from interrupt context. > + */ > +void suspend_unblock(struct suspend_blocker *blocker) > +{ > + unsigned long irqflags; > + > + if (WARN_ON(!(blocker->flags & SB_INITIALIZED))) > + return; > + > + spin_lock_irqsave(&list_lock, irqflags); > + > + if (debug_mask & DEBUG_SUSPEND_BLOCKER) > + pr_info("suspend_unblock: %s\n", blocker->name); > + > + list_del(&blocker->link); > + list_add(&blocker->link, &inactive_blockers); > + > + if ((blocker->flags & SB_ACTIVE) && list_empty(&active_blockers)) > + queue_work(suspend_work_queue, &suspend_work); > + blocker->flags &= ~(SB_ACTIVE); > + if (blocker == &main_suspend_blocker) { > + if (debug_mask & DEBUG_SUSPEND) > + print_active_blockers_locked(); > + } > + spin_unlock_irqrestore(&list_lock, irqflags); > +} > +EXPORT_SYMBOL(suspend_unblock); > + > +/** > + * suspend_blocker_is_active() - Test if a suspend blocker is blocking suspend > + * @blocker: The suspend blocker to check. > + * > + * Returns true if the suspend_blocker is currently active. > + */ > +bool suspend_blocker_is_active(struct suspend_blocker *blocker) > +{ > + WARN_ON(!(blocker->flags & SB_INITIALIZED)); > + > + return !!(blocker->flags & SB_ACTIVE); > +} > +EXPORT_SYMBOL(suspend_blocker_is_active); > + > +bool request_suspend_valid_state(suspend_state_t state) > +{ > + return (state == PM_SUSPEND_ON) || valid_state(state); > +} > + > +int request_suspend_state(suspend_state_t state) > +{ > + unsigned long irqflags; > + > + if (!request_suspend_valid_state(state)) > + return -ENODEV; > + > + spin_lock_irqsave(&state_lock, irqflags); > + > + if (debug_mask & DEBUG_USER_STATE) > + pr_info_time("request_suspend_state: %s (%d->%d) at %lld ", > + state != PM_SUSPEND_ON ? "sleep" : "wakeup", > + requested_suspend_state, state, > + ktime_to_ns(ktime_get())); > + > + requested_suspend_state = state; > + if (state == PM_SUSPEND_ON) > + suspend_block(&main_suspend_blocker); > + else > + suspend_unblock(&main_suspend_blocker); > + spin_unlock_irqrestore(&state_lock, irqflags); > + return 0; > +} > + > +static int __init suspend_block_init(void) > +{ > + suspend_work_queue = create_singlethread_workqueue("suspend"); > + if (!suspend_work_queue) > + return -ENOMEM; > + > + suspend_blocker_init(&main_suspend_blocker, "main"); > + suspend_block(&main_suspend_blocker); > + return 0; > +} > + > +core_initcall(suspend_block_init); -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arve Hjønnevåg on 30 Apr 2010 14:10 2010/4/29 Alan Stern <stern(a)rowland.harvard.edu>: > On Wed, 28 Apr 2010, Arve Hj�nnev�g wrote: > >> >> > �suspend blockers can be used to allow >> >> > +user-space to decide whether a keystroke received while the system is suspended >> >> > +should cause the screen to be turned back on or allow the system to go back into >> >> > +suspend. >> >> >> >> That's not right. �Handling the screen doesn't need suspend blockers: >> >> The program decides what to do and then either turns on the screen or >> >> else writes "mem" to /sys/power/state. >> >> That does not work though. Unless every key turns the screen on you >> will have a race every time the user presses a key you want to ignore. > > Of course. �You are confirming what I wrote immediately below: Suspend Yet you offered it as an example of why "Handling the screen doesn't need suspend blockers". > blockers help resolve races. �Note that this race has nothing to do > with the _screen_ in particular -- exactly the same race occurs if you > decide to turn on the audio speaker or some other piece of hardware. > I agree with this, but that does not mean that describing how you can handle the screen with suspend blockers is a bad example. >> >> �What suspend blockers add is >> >> the ability to resolve races and satisfy multiple constraints when >> >> going into suspend -- which has nothing to do with operating the >> >> screen. >> >> I'm not sure I agree with this. You cannot reliably turn the screen on >> from user space when the user presses a wakeup-key without suspend >> blockers. > > Let's say that it has nothing to do _specifically_ with the screen. > _Any_ action you want to take in userspace is difficult to coordinate > with system suspends if you don't have suspend blockers. > >> >> >> >> I _think_ what you're trying to get at can be expressed this way: >> >> >> >> � � � Here's an example showing how a cell phone or other embedded >> >> � � � system can handle keystrokes (or other input events) in the >> >> � � � presence of suspend blockers. �Use set_irq_wake... >> >> OK, but the last version was what you (Alan) suggested last year. > > So at least my mental processes have remained consistent over the span > of a year. �Nice to know I haven't undergone a complete personality > change... �:-) > >> >> � � � ... >> >> >> >> � � � - The user-space input-event thread returns from read. �It >> >> � � � carries out whatever activities are appropriate (for example, >> >> � � � powering up the display screen, running other programs, and so >> >> � � � on). �When it is finished, it calls suspend_unblock on the >> >> � � � process_input_events suspend_blocker and then calls select or >> >> � � � poll. �The system will automatically suspend again when it is >> >> � � � idle and no suspend blockers remain active. >> > >> > Yeah, that sounds better. �Arve, what do you think? >> > >> >> Idle is irrelevant and needs to be removed. This new last step is also >> no longer a concrete example, but if you really think is it better I >> can change it. > > Perhaps you would prefer to change this completely. �Write up a > description of what can go wrong when suspend blockers _aren't_ used, > and show how suspend blockers can prevent the problem from occurring. > > But whatever you do, don't make it appear that suspend blockers allow > user programs to make decisions (which is what you wrote before). �They > don't -- programs can make whatever decisions they want. �Suspend > blockers merely help them carry out the actions they have decided upon > in a safe manner. I think suspend blockers do allow user programs to make decisions. Without suspend blockers some decisions can only be safely be made in the kernel/drivers. > > And don't make it appear that suspend blockers can only be used for > solving screen-related problems. How about: - The user-space input-event thread returns from read. If it determines that the key should be ignored, it calls suspend_unblock on the process_input_events suspend_blocker and then calls select or poll. The system will automatically suspend again, since now no suspend blockers are active. If the key that was pressed instead should preform a simple action (for example, adjusting the volume), this action can be performed right before calling suspend_unblock on the process_input_events suspend_blocker. However, if the key triggers a longer-running action, that action needs its own suspend_blocker and suspend_block must be called on that suspend blocker before calling suspend_unblock on the process_input_events suspend_blocker. -- Arve Hj�nnev�g -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on 30 Apr 2010 14:30 On Thursday 29 April 2010, Arve Hj�nnev�g wrote: > 2010/4/28 Rafael J. Wysocki <rjw(a)sisk.pl>: > ... > >> > >> +/** > >> + * policy - set policy for state > >> + */ > >> + > >> +static ssize_t policy_show(struct kobject *kobj, > >> + struct kobj_attribute *attr, char *buf) > >> +{ > >> + char *s = buf; > >> + int i; > >> + > >> + for (i = 0; i < ARRAY_SIZE(policies); i++) { > >> + if (i == policy) > >> + s += sprintf(s, "[%s] ", policies[i].name); > >> + else > >> + s += sprintf(s, "%s ", policies[i].name); > >> + } > >> + if (s != buf) > >> + /* convert the last space to a newline */ > >> + *(s-1) = '\n'; > >> + return (s - buf); > >> +} > >> + > >> +static ssize_t policy_store(struct kobject *kobj, > >> + struct kobj_attribute *attr, > >> + const char *buf, size_t n) > >> +{ > >> + const char *s; > >> + char *p; > >> + int len; > >> + int i; > >> + > >> + p = memchr(buf, '\n', n); > >> + len = p ? p - buf : n; > >> + > >> + for (i = 0; i < ARRAY_SIZE(policies); i++) { > >> + s = policies[i].name; > >> + if (s && len == strlen(s) && !strncmp(buf, s, len)) { > >> + mutex_lock(&pm_mutex); > >> + policies[policy].set_state(PM_SUSPEND_ON); > >> + policy = i; > >> + mutex_unlock(&pm_mutex); > >> + return n; > >> + } > >> + } > >> + return -EINVAL; > >> +} > >> + > >> +power_attr(policy); > >> + > >> #ifdef CONFIG_PM_TRACE > >> int pm_trace_enabled; > >> > > > > Would you mind if I changed the above so that "policy" doesn't even show up > > if CONFIG_OPPORTUNISTIC_SUSPEND is unset? > > > I don't mind, but It did not seem worth the trouble to hide it. It > will only list the supported policies, and it is easy to add or remove > policies this way. > > > ... > >> +static void suspend_worker(struct work_struct *work) > >> +{ > >> + int ret; > >> + int entry_event_num; > >> + > >> + enable_suspend_blockers = true; > >> + while (!suspend_is_blocked()) { > >> + entry_event_num = current_event_num; > >> + > >> + if (debug_mask & DEBUG_SUSPEND) > >> + pr_info("suspend: enter suspend\n"); > >> + > >> + ret = pm_suspend(requested_suspend_state); > >> + > >> + if (debug_mask & DEBUG_EXIT_SUSPEND) > >> + pr_info_time("suspend: exit suspend, ret = %d ", ret); > >> + > >> + if (current_event_num == entry_event_num) > >> + pr_info("suspend: pm_suspend returned with no event\n"); > > > > Hmm, what exactly is this for? It looks like a debug thing to me. I'd use > > pr_debug() here and in both debug printk()s above. Would you agree? > > > > If the driver that caused the wakeup does not use suspend blockers, we > the only choice is to try to suspend again. I want to know if this > happened. The stats patch disable this printk by default since it will > show up in the stats, and the timeout patch (not included here) delays > the retry. > > ... > >> +EXPORT_SYMBOL(suspend_blocker_init); > > > > Is there a strong objection to changing that (and the other instances below) to > > EXPORT_SYMBOL_GPL? > > > > I don't know if it is a strong objection, but I prefer that this api > is available to all drivers. I don't want to prevent a user from using > opportunistic suspend because a non-gpl driver could not use suspend > blockers. I changed the suspend blocking work functions to be gpl only > though, since they are not required, and the workqueue api is > available to gpl code anyway. > > ... > >> +bool request_suspend_valid_state(suspend_state_t state) > >> +{ > >> + return (state == PM_SUSPEND_ON) || valid_state(state); > >> +} > >> + > >> +int request_suspend_state(suspend_state_t state) > >> +{ > >> + unsigned long irqflags; > >> + > >> + if (!request_suspend_valid_state(state)) > >> + return -ENODEV; > >> + > >> + spin_lock_irqsave(&state_lock, irqflags); > >> + > >> + if (debug_mask & DEBUG_USER_STATE) > >> + pr_info_time("request_suspend_state: %s (%d->%d) at %lld ", > >> + state != PM_SUSPEND_ON ? "sleep" : "wakeup", > >> + requested_suspend_state, state, > >> + ktime_to_ns(ktime_get())); > >> + > >> + requested_suspend_state = state; > >> + if (state == PM_SUSPEND_ON) > >> + suspend_block(&main_suspend_blocker); > >> + else > >> + suspend_unblock(&main_suspend_blocker); > >> + spin_unlock_irqrestore(&state_lock, irqflags); > >> + return 0; > >> +} > > > > I think the two functions above should be static, shouldn't they? > > No, they are used from main.c. > > > > >> +static int __init suspend_block_init(void) > >> +{ > >> + suspend_work_queue = create_singlethread_workqueue("suspend"); > >> + if (!suspend_work_queue) > >> + return -ENOMEM; > >> + > >> + suspend_blocker_init(&main_suspend_blocker, "main"); > >> + suspend_block(&main_suspend_blocker); > >> + return 0; > >> +} > >> + > >> +core_initcall(suspend_block_init); > > > > Hmm. Why don't you want to put that initialization into pm_init() (in > > kernel/power/main.c)? > > It was not needed before, but I changed pm_init to call > suspend_block_init after creating pm_wq. > > > > > Also, we already have one PM workqueue. It is used for runtime PM, but I guess > > it may be used just as well for the opportunistic suspend. It is freezable, > > but would it hurt? > > No, it works, the freezable flag is just ignored when I call > pm_suspend and I don't run anything else on the workqueue while > threads are frozen. It does need to be a single threaded workqueue > though, so make sure you don't just change that. Freezable workqueues have to be singlethread or else there will be unfixable races, so you can safely assume things will stay as they are in this respect. Rafael > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 30 Apr 2010 14:50
On Thu, 29 Apr 2010, Arve Hj�nnev�g wrote: > >> That does not work though. Unless every key turns the screen on you > >> will have a race every time the user presses a key you want to ignore. > > > > Of course. �You are confirming what I wrote immediately below: Suspend > Yet you offered it as an example of why "Handling the screen doesn't > need suspend blockers". What I meant was this: Without suspend blockers you can still turn the screen on and off, but you can't avoid races. Therefore: You don't need suspend blockers to handle the screen, but you do need them to handle races. > > blockers help resolve races. �Note that this race has nothing to do > > with the _screen_ in particular -- exactly the same race occurs if you > > decide to turn on the audio speaker or some other piece of hardware. > > > I agree with this, but that does not mean that describing how you can > handle the screen with suspend blockers is a bad example. Agreed; it's not a bad example. My objection is to the way the example is presented. > I think suspend blockers do allow user programs to make decisions. > Without suspend blockers some decisions can only be safely be made in > the kernel/drivers. This is our real disagreement -- it's mainly a question of the meaning of words. When one says "A allows B to make a decision", this means that without A, B would literally be unable to decide what to do. It wouldn't be able to "make up its mind" (like the donkey that starves while stuck halfway between two piles of hay because it can't decide which pile to eat). That's not what you mean to say. In your case there's no difficulty in making the choice -- the program knows exactly what it wants to do. The problem is that without suspend blockers, it can't _carry out_ the chosen action. To put it another way, the suspend blockers don't help with deciding which action to take; rather they help with taking the action after it has been decided upon. Or put this way: Suppose suspend blockers were not available. Then the desired action would not be safe, so the program wouldn't be able to take it. The decision would thus be even simpler, since one of the choices would be eliminated. In this way, lack of suspend blockers makes the decision easier, not harder. So you shouldn't say that suspend blockers allow the program to make the decision. What your example _really_ shows is how a program can carry out a prolonged action that requires the system to be awake for an extended period of time. Even with suspend blockers, the right way to do this safely isn't necessarily obvious. That's how the example helps. > How about: > > - The user-space input-event thread returns from read. If it > determines that the key should be ignored, it calls suspend_unblock on > the process_input_events suspend_blocker and then calls select or > poll. The system will automatically suspend again, since now no > suspend blockers are active. Do you want to mention here that everything will work correctly even if the system suspends before the thread can call select or poll? > If the key that was pressed instead should preform a simple action > (for example, adjusting the volume), this action can be performed > right before calling suspend_unblock on the process_input_events > suspend_blocker. However, if the key triggers a longer-running action, > that action needs its own suspend_blocker and suspend_block must be > called on that suspend blocker before calling suspend_unblock on the > process_input_events suspend_blocker. That's good! I like it. Now if you can change the beginning of the example, to say that it shows how programs should use suspend blockers instead of showing that suspend blockers allow programs to make decisions. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |