On Friday, April 27, 2012, Rafael J. Wysocki wrote:
> On Friday, April 27, 2012, Arve Hj?nnev?g wrote:
> > 2012/4/27 Rafael J. Wysocki <[email protected]>:
> > > On Friday, April 27, 2012, Arve Hj?nnev?g wrote:
> > >> 2012/4/26 Rafael J. Wysocki <[email protected]>:
> > >> ...
> > >> > ---
> > >> > From: Rafael J. Wysocki <[email protected]>
> > >> > Subject: PM / Sleep: Add user space interface for manipulating wakeup sources, v3
> > >> >
> > >> > Android allows user space to manipulate wakelocks using two
> > >> > sysfs file located in /sys/power/, wake_lock and wake_unlock.
> > >> > Writing a wakelock name and optionally a timeout to the wake_lock
> > >> > file causes the wakelock whose name was written to be acquired (it
> > >> > is created before is necessary), optionally with the given timeout.
> > >> > Writing the name of a wakelock to wake_unlock causes that wakelock
> > >> > to be released.
> > >> >
> > >> > Implement an analogous interface for user space using wakeup sources.
> > >> > Add the /sys/power/wake_lock and /sys/power/wake_unlock files
> > >> > allowing user space to create, activate and deactivate wakeup
> > >> > sources, such that writing a name and optionally a timeout to
> > >> > wake_lock causes the wakeup source of that name to be activated,
> > >> > optionally with the given timeout. If that wakeup source doesn't
> > >> > exist, it will be created and then activated. Writing a name to
> > >> > wake_unlock causes the wakeup source of that name, if there is one,
> > >> > to be deactivated. Wakeup sources created with the help of
> > >> > wake_lock that haven't been used for more than 5 minutes are garbage
> > >> > collected and destroyed. Moreover, there can be only WL_NUMBER_LIMIT
> > >>
> > >> I think it would be better if the garbage collection and limit was
> > >> configurable and optional. I would probably turn both features off
> > >> since I do not want to chase down bugs because a wakelock was ignored,
> > >> and I think the garbage collection will erase stats that we care
> > >> about.
> > >
> > > OK, but would you mind if I added the configurability as a separate incremental
> > > patch?
> > >
> >
> > That is fine with me.
>
> Cool, thanks!
The following two patches add the configuration options for the limit and
garbage collector. Please let me know if they are OK with you.
Thanks,
Rafael
From: Rafael J. Wysocki <[email protected]>
Make it possible to configure out the user space wakeup sources
garbage collector for debugging and default Android builds.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/Kconfig | 5 ++
kernel/power/wakelock.c | 101 +++++++++++++++++++++++++++++-------------------
2 files changed, 67 insertions(+), 39 deletions(-)
Index: linux/kernel/power/Kconfig
===================================================================
--- linux.orig/kernel/power/Kconfig
+++ linux/kernel/power/Kconfig
@@ -125,6 +125,11 @@ config PM_WAKELOCKS_LIMIT
default 100
depends on PM_WAKELOCKS
+config PM_WAKELOCKS_GC
+ bool "Garbage collector for user space wakeup sources"
+ depends on PM_WAKELOCKS
+ default y
+
config PM_RUNTIME
bool "Run-time PM core functionality"
depends on !IA64_HP_SIM
Index: linux/kernel/power/wakelock.c
===================================================================
--- linux.orig/kernel/power/wakelock.c
+++ linux/kernel/power/wakelock.c
@@ -17,21 +17,18 @@
#include <linux/rbtree.h>
#include <linux/slab.h>
-#define WL_GC_COUNT_MAX 100
-#define WL_GC_TIME_SEC 300
-
static DEFINE_MUTEX(wakelocks_lock);
struct wakelock {
char *name;
struct rb_node node;
struct wakeup_source ws;
+#ifdef CONFIG_PM_WAKELOCKS_GC
struct list_head lru;
+#endif
};
static struct rb_root wakelocks_tree = RB_ROOT;
-static LIST_HEAD(wakelocks_lru_list);
-static unsigned int wakelocks_gc_count;
ssize_t pm_show_wakelocks(char *buf, bool show_active)
{
@@ -79,6 +76,61 @@ static inline void increment_wakelocks_n
static inline void decrement_wakelocks_number(void) {}
#endif /* CONFIG_PM_WAKELOCKS_LIMIT */
+#ifdef CONFIG_PM_WAKELOCKS_GC
+#define WL_GC_COUNT_MAX 100
+#define WL_GC_TIME_SEC 300
+
+static LIST_HEAD(wakelocks_lru_list);
+static unsigned int wakelocks_gc_count;
+
+static inline void wakelocks_lru_add(struct wakelock *wl)
+{
+ list_add(&wl->lru, &wakelocks_lru_list);
+}
+
+static inline void wakelocks_lru_most_recent(struct wakelock *wl)
+{
+ list_move(&wl->lru, &wakelocks_lru_list);
+}
+
+static void wakelocks_gc(void)
+{
+ struct wakelock *wl, *aux;
+ ktime_t now;
+
+ if (++wakelocks_gc_count <= WL_GC_COUNT_MAX)
+ return;
+
+ now = ktime_get();
+ list_for_each_entry_safe_reverse(wl, aux, &wakelocks_lru_list, lru) {
+ u64 idle_time_ns;
+ bool active;
+
+ spin_lock_irq(&wl->ws.lock);
+ idle_time_ns = ktime_to_ns(ktime_sub(now, wl->ws.last_time));
+ active = wl->ws.active;
+ spin_unlock_irq(&wl->ws.lock);
+
+ if (idle_time_ns < ((u64)WL_GC_TIME_SEC * NSEC_PER_SEC))
+ break;
+
+ if (!active) {
+ wakeup_source_remove(&wl->ws);
+ rb_erase(&wl->node, &wakelocks_tree);
+ list_del(&wl->lru);
+ kfree(wl->name);
+ kfree(wl);
+ decrement_wakelocks_number();
+ }
+ }
+ wakelocks_gc_count = 0;
+}
+#else /* !CONFIG_PM_WAKELOCKS_GC */
+static inline void wakelocks_lru_add(struct wakelock *wl) {}
+static inline void wakelocks_lru_most_recent(struct wakelock *wl) {}
+static inline void wakelocks_gc(void) {}
+#endif /* !CONFIG_PM_WAKELOCKS_GC */
+
static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
bool add_if_not_found)
{
@@ -123,7 +175,7 @@ static struct wakelock *wakelock_lookup_
wakeup_source_add(&wl->ws);
rb_link_node(&wl->node, parent, node);
rb_insert_color(&wl->node, &wakelocks_tree);
- list_add(&wl->lru, &wakelocks_lru_list);
+ wakelocks_lru_add(wl);
increment_wakelocks_number();
return wl;
}
@@ -166,42 +218,13 @@ int pm_wake_lock(const char *buf)
__pm_stay_awake(&wl->ws);
}
- list_move(&wl->lru, &wakelocks_lru_list);
+ wakelocks_lru_most_recent(wl);
out:
mutex_unlock(&wakelocks_lock);
return ret;
}
-static void wakelocks_gc(void)
-{
- struct wakelock *wl, *aux;
- ktime_t now = ktime_get();
-
- list_for_each_entry_safe_reverse(wl, aux, &wakelocks_lru_list, lru) {
- u64 idle_time_ns;
- bool active;
-
- spin_lock_irq(&wl->ws.lock);
- idle_time_ns = ktime_to_ns(ktime_sub(now, wl->ws.last_time));
- active = wl->ws.active;
- spin_unlock_irq(&wl->ws.lock);
-
- if (idle_time_ns < ((u64)WL_GC_TIME_SEC * NSEC_PER_SEC))
- break;
-
- if (!active) {
- wakeup_source_remove(&wl->ws);
- rb_erase(&wl->node, &wakelocks_tree);
- list_del(&wl->lru);
- kfree(wl->name);
- kfree(wl);
- decrement_wakelocks_number();
- }
- }
- wakelocks_gc_count = 0;
-}
-
int pm_wake_unlock(const char *buf)
{
struct wakelock *wl;
@@ -226,9 +249,9 @@ int pm_wake_unlock(const char *buf)
goto out;
}
__pm_relax(&wl->ws);
- list_move(&wl->lru, &wakelocks_lru_list);
- if (++wakelocks_gc_count > WL_GC_COUNT_MAX)
- wakelocks_gc();
+
+ wakelocks_lru_most_recent(wl);
+ wakelocks_gc();
out:
mutex_unlock(&wakelocks_lock);
From: Rafael J. Wysocki <[email protected]>
Make it possible to configure out the check against the limit of
user space wakeup sources for debugging and default Android builds.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/Kconfig | 6 ++++++
kernel/power/wakelock.c | 31 ++++++++++++++++++++++++++-----
2 files changed, 32 insertions(+), 5 deletions(-)
Index: linux/kernel/power/Kconfig
===================================================================
--- linux.orig/kernel/power/Kconfig
+++ linux/kernel/power/Kconfig
@@ -119,6 +119,12 @@ config PM_WAKELOCKS
Allow user space to create, activate and deactivate wakeup source
objects with the help of a sysfs-based interface.
+config PM_WAKELOCKS_LIMIT
+ int "Maximum number of user space wakeup sources (0 = no limit)"
+ range 0 100000
+ default 100
+ depends on PM_WAKELOCKS
+
config PM_RUNTIME
bool "Run-time PM core functionality"
depends on !IA64_HP_SIM
Index: linux/kernel/power/wakelock.c
===================================================================
--- linux.orig/kernel/power/wakelock.c
+++ linux/kernel/power/wakelock.c
@@ -17,7 +17,6 @@
#include <linux/rbtree.h>
#include <linux/slab.h>
-#define WL_NUMBER_LIMIT 100
#define WL_GC_COUNT_MAX 100
#define WL_GC_TIME_SEC 300
@@ -32,7 +31,6 @@ struct wakelock {
static struct rb_root wakelocks_tree = RB_ROOT;
static LIST_HEAD(wakelocks_lru_list);
-static unsigned int number_of_wakelocks;
static unsigned int wakelocks_gc_count;
ssize_t pm_show_wakelocks(char *buf, bool show_active)
@@ -58,6 +56,29 @@ ssize_t pm_show_wakelocks(char *buf, boo
return (str - buf);
}
+#if CONFIG_PM_WAKELOCKS_LIMIT > 0
+static unsigned int number_of_wakelocks;
+
+static inline bool wakelocks_limit_exceeded(void)
+{
+ return number_of_wakelocks > CONFIG_PM_WAKELOCKS_LIMIT;
+}
+
+static inline void increment_wakelocks_number(void)
+{
+ number_of_wakelocks++;
+}
+
+static inline void decrement_wakelocks_number(void)
+{
+ number_of_wakelocks--;
+}
+#else /* CONFIG_PM_WAKELOCKS_LIMIT = 0 */
+static inline bool wakelocks_limit_exceeded(void) { return false; }
+static inline void increment_wakelocks_number(void) {}
+static inline void decrement_wakelocks_number(void) {}
+#endif /* CONFIG_PM_WAKELOCKS_LIMIT */
+
static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
bool add_if_not_found)
{
@@ -85,7 +106,7 @@ static struct wakelock *wakelock_lookup_
if (!add_if_not_found)
return ERR_PTR(-EINVAL);
- if (number_of_wakelocks > WL_NUMBER_LIMIT)
+ if (wakelocks_limit_exceeded())
return ERR_PTR(-ENOSPC);
/* Not found, we have to add a new one. */
@@ -103,7 +124,7 @@ static struct wakelock *wakelock_lookup_
rb_link_node(&wl->node, parent, node);
rb_insert_color(&wl->node, &wakelocks_tree);
list_add(&wl->lru, &wakelocks_lru_list);
- number_of_wakelocks++;
+ increment_wakelocks_number();
return wl;
}
@@ -175,7 +196,7 @@ static void wakelocks_gc(void)
list_del(&wl->lru);
kfree(wl->name);
kfree(wl);
- number_of_wakelocks--;
+ decrement_wakelocks_number();
}
}
wakelocks_gc_count = 0;
On Thu, May 3, 2012 at 12:29 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Friday, April 27, 2012, Rafael J. Wysocki wrote:
>> On Friday, April 27, 2012, Arve Hj?nnev?g wrote:
>> > 2012/4/27 Rafael J. Wysocki <[email protected]>:
>> > > On Friday, April 27, 2012, Arve Hj?nnev?g wrote:
>> > >> 2012/4/26 Rafael J. Wysocki <[email protected]>:
>> > >> ...
>> > >> > ---
>> > >> > From: Rafael J. Wysocki <[email protected]>
>> > >> > Subject: PM / Sleep: Add user space interface for manipulating wakeup sources, v3
>> > >> >
>> > >> > Android allows user space to manipulate wakelocks using two
>> > >> > sysfs file located in /sys/power/, wake_lock and wake_unlock.
>> > >> > Writing a wakelock name and optionally a timeout to the wake_lock
>> > >> > file causes the wakelock whose name was written to be acquired (it
>> > >> > is created before is necessary), optionally with the given timeout.
>> > >> > Writing the name of a wakelock to wake_unlock causes that wakelock
>> > >> > to be released.
>> > >> >
>> > >> > Implement an analogous interface for user space using wakeup sources.
>> > >> > Add the /sys/power/wake_lock and /sys/power/wake_unlock files
>> > >> > allowing user space to create, activate and deactivate wakeup
>> > >> > sources, such that writing a name and optionally a timeout to
>> > >> > wake_lock causes the wakeup source of that name to be activated,
>> > >> > optionally with the given timeout. ?If that wakeup source doesn't
>> > >> > exist, it will be created and then activated. ?Writing a name to
>> > >> > wake_unlock causes the wakeup source of that name, if there is one,
>> > >> > to be deactivated. ?Wakeup sources created with the help of
>> > >> > wake_lock that haven't been used for more than 5 minutes are garbage
>> > >> > collected and destroyed. ?Moreover, there can be only WL_NUMBER_LIMIT
>> > >>
>> > >> I think it would be better if the garbage collection and limit was
>> > >> configurable and optional. I would probably turn both features off
>> > >> since I do not want to chase down bugs because a wakelock was ignored,
>> > >> and I think the garbage collection will erase stats that we care
>> > >> about.
>> > >
>> > > OK, but would you mind if I added the configurability as a separate incremental
>> > > patch?
>> > >
>> >
>> > That is fine with me.
>>
>> Cool, thanks!
>
> The following two patches add the configuration options for the limit and
> garbage collector. ?Please let me know if they are OK with you.
>
Yes.
--
Arve Hj?nnev?g
On Friday, May 04, 2012, Arve Hj?nnev?g wrote:
> On Thu, May 3, 2012 at 12:29 PM, Rafael J. Wysocki <[email protected]> wrote:
> > On Friday, April 27, 2012, Rafael J. Wysocki wrote:
> >> On Friday, April 27, 2012, Arve Hj?nnev?g wrote:
> >> > 2012/4/27 Rafael J. Wysocki <[email protected]>:
> >> > > On Friday, April 27, 2012, Arve Hj?nnev?g wrote:
> >> > >> 2012/4/26 Rafael J. Wysocki <[email protected]>:
> >> > >> ...
> >> > >> > ---
> >> > >> > From: Rafael J. Wysocki <[email protected]>
> >> > >> > Subject: PM / Sleep: Add user space interface for manipulating wakeup sources, v3
> >> > >> >
> >> > >> > Android allows user space to manipulate wakelocks using two
> >> > >> > sysfs file located in /sys/power/, wake_lock and wake_unlock.
> >> > >> > Writing a wakelock name and optionally a timeout to the wake_lock
> >> > >> > file causes the wakelock whose name was written to be acquired (it
> >> > >> > is created before is necessary), optionally with the given timeout.
> >> > >> > Writing the name of a wakelock to wake_unlock causes that wakelock
> >> > >> > to be released.
> >> > >> >
> >> > >> > Implement an analogous interface for user space using wakeup sources.
> >> > >> > Add the /sys/power/wake_lock and /sys/power/wake_unlock files
> >> > >> > allowing user space to create, activate and deactivate wakeup
> >> > >> > sources, such that writing a name and optionally a timeout to
> >> > >> > wake_lock causes the wakeup source of that name to be activated,
> >> > >> > optionally with the given timeout. If that wakeup source doesn't
> >> > >> > exist, it will be created and then activated. Writing a name to
> >> > >> > wake_unlock causes the wakeup source of that name, if there is one,
> >> > >> > to be deactivated. Wakeup sources created with the help of
> >> > >> > wake_lock that haven't been used for more than 5 minutes are garbage
> >> > >> > collected and destroyed. Moreover, there can be only WL_NUMBER_LIMIT
> >> > >>
> >> > >> I think it would be better if the garbage collection and limit was
> >> > >> configurable and optional. I would probably turn both features off
> >> > >> since I do not want to chase down bugs because a wakelock was ignored,
> >> > >> and I think the garbage collection will erase stats that we care
> >> > >> about.
> >> > >
> >> > > OK, but would you mind if I added the configurability as a separate incremental
> >> > > patch?
> >> > >
> >> >
> >> > That is fine with me.
> >>
> >> Cool, thanks!
> >
> > The following two patches add the configuration options for the limit and
> > garbage collector. Please let me know if they are OK with you.
> >
>
> Yes.
Good, thanks!