[markgross@thengar.org: Re: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)]

[markgross@thengar.org: Re: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)]

[mark gross]

Forwarding to bigger group.

----- Forwarded message from mark gross <markgross@thengar.org> -----

Subject: Re: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)
Date: Tue, 20 Sep 2011 13:35:50 -0700
From: mark gross <markgross@thengar.org>
To: mark gross <markgross@thegnar.org>
Reply-To: markgross@thegnar.org
Cc: linux-pm@lists.linux-foundation.org, arve@android.com, Alan Stern <stern@rowland.harvard.edu>, amit.kucheria@linaro.org, farrowg@sg.ibm.com, "Rafael J. Wysocki" <rjw@sisk.pl>


>From 640f7d4a5f062d693a05804c0013b303866771da Mon Sep 17 00:00:00 2001
From: mark gross <mark97229@gmail.com>
Date: Wed, 14 Sep 2011 17:02:36 -0700
Subject: [PATCH 1/2] Wake (from suspend) notification framework.  Add user mode hand shake
 for non-racy acknowledgment of wake up event by user mode processes that
 care per wake up source.

This attempts to address the issue of blocking subsequent suspends from
happening before every user mode process that cares has a chance to
process the last wakeup event.  i.e. the wake lock problem.

This version uses select / read / write methods for event delivery and
acknowledgment.  Also enables the delivery of multiple wake events.

Signed-off-by: mark gross <mark97229@gmail.com>
---
 Documentation/power/wakeup_events.txt |   39 ++++++++
 drivers/base/power/wakeup.c           |  171 ++++++++++++++++++++++++++++++++-
 include/linux/pm_wakeup.h             |   41 +++++++--
 3 files changed, 242 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/power/wakeup_events.txt

diff --git a/Documentation/power/wakeup_events.txt b/Documentation/power/wakeup_events.txt
new file mode 100644
index 0000000..2d79f04
--- /dev/null
+++ b/Documentation/power/wakeup_events.txt
@@ -0,0 +1,39 @@
+Wakeup Event Interface.
+
+This interface provides a notification of wakeup events to user mode processes
+in such a manner that blocks re-entry into suspend until the user mode
+processes that care have explicitly acknowledged the event.
+
+The kernel code to look at for this interface is implemented in:
+drivers/base/power/main.c
+drivers/base/power/wakeup.c
+include/linux/pm_wakeup.h
+
+There are a few assumptions that are made on the behavior of the user mode
+stack WRT requesting entry into the suspend state as well as the behavior of
+the process that request notification of wake up events.  Both are documented
+in this document.
+
+First it is assumed that the user mode power management daemon follow the
+protocol outlined in drivers/base/power/main.c.  See wakeup_count_show.  Which
+blocks while in progress wake events are non zero.
+
+Wake events will stay in progress as long as there are any registered processes
+that have not acknowledged the event or the wakeup source has unbalanced
+pm_stay_awake/pm_relax calls.  When all the processes that care about a given
+wakeup event finish acknowledging the event the read to /sys/power/wakeup_count
+will un block and a subsequent suspend can be attempted.
+
+This interface uses a similar implementation to pm_qos, where misc a device
+node is set up for each registered wakeup_source.  User mode processes register
+for notification by holding an open handle to the misc device associated with
+the wakeup_source.  When the process closes the misc device any pending
+notifications are automatically cleaned up.
+
+The interested process executes a blocking select until there are a non-zero
+number of pending wakeup events, upon unblocking the process can read from the
+file to see how many events are need acknowledgement and they are be
+acknowledged by doing dummy writes of that many bytes to the file.  (the user
+mode data isn't actually copied in the write function just the count is used)
+
+
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 84f7c7d..eb300d7 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -10,9 +10,13 @@
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/capability.h>
+#include <linux/freezer.h>
+#include <linux/miscdevice.h>
 #include <linux/suspend.h>
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
+#include <linux/fs.h>
+#include <linux/poll.h>
 
 #include "power.h"
 
@@ -51,6 +55,135 @@ static void pm_wakeup_timer_fn(unsigned long data);
 
 static LIST_HEAD(wakeup_sources);
 
+
+static int pm_blocker_open(struct inode *inode, struct file *filp)
+{
+	struct relax_blocker *blocker;
+	struct wakeup_source *ws;
+
+	spin_lock_irq(&events_lock);
+	list_for_each_entry_rcu(ws, &wakeup_sources, entry)
+		if (ws->miscdev.minor == iminor(inode))
+			break;
+	spin_unlock_irq(&events_lock);
+
+	if (!ws)
+		return -EPERM;
+
+	if (ws->dying)
+		return -EPERM;
+
+	blocker = kzalloc(sizeof(struct relax_blocker), GFP_KERNEL);
+	if (!blocker)
+		return -ENOMEM;
+
+	blocker->resumes_to_ack = 0;
+	blocker->ws = ws;
+
+	spin_lock_irq(&events_lock);
+	list_add_rcu(&(blocker->entry), &(ws->blockers.entry));
+	spin_unlock_irq(&events_lock);
+
+	filp->private_data = blocker;
+
+	return 0;
+}
+
+static int pm_blocker_release(struct inode *inode, struct file *filp)
+{
+	struct relax_blocker *blocker;
+
+	blocker = filp->private_data;
+	WARN_ON(!blocker);
+
+	spin_lock_irq(&events_lock);
+	list_del_rcu(&blocker->entry);
+
+	while (blocker->resumes_to_ack) {
+		blocker->ws->relax_count++;
+		blocker->resumes_to_ack--;
+	}
+	spin_unlock_irq(&events_lock);
+
+	kfree(blocker);
+
+	return 0;
+}
+
+
+unsigned int pm_blocker_poll(struct file *filp, struct poll_table_struct *wait)
+{
+	struct relax_blocker *blocker;
+
+	blocker = filp->private_data;
+	WARN_ON(!blocker);
+
+	if (blocker->ws->dying)
+		return -EINVAL;
+
+	if (blocker->resumes_to_ack)
+		return POLLIN|POLLOUT|POLLWRNORM;
+
+	poll_wait(filp, &blocker->ws->wq, wait);
+
+	if (blocker->resumes_to_ack)
+		return POLLIN|POLLOUT|POLLWRNORM;
+	else
+		return POLLIN|POLLRDNORM;
+}
+
+
+static ssize_t pm_blocker_read(struct file *filp, char __user *buf,
+		size_t count, loff_t *f_pos)
+{
+	s32 value = 0;
+	struct relax_blocker *blocker;
+
+	if ((*f_pos > 0) || (count != sizeof(value)))
+		return -EINVAL;
+
+	blocker = filp->private_data;
+	WARN_ON(!blocker);
+
+	if (blocker->ws->dying)
+		return -EINVAL;
+
+	value = blocker->resumes_to_ack;
+
+	return simple_read_from_buffer(
+			buf, count, f_pos, &value, sizeof(value));
+}
+
+static ssize_t pm_blocker_write(struct file *filp, const char __user *buf,
+		size_t count, loff_t *f_pos)
+{
+	struct relax_blocker *blocker;
+
+	blocker = filp->private_data;
+	if (count && blocker->resumes_to_ack) {
+		spin_lock_irq(&blocker->ws->lock);
+		WARN_ON(!blocker);
+		while (blocker->resumes_to_ack) {
+			blocker->ws->relax_count++;
+			blocker->resumes_to_ack--;
+		}
+		spin_unlock_irq(&blocker->ws->lock);
+		return count;
+	} else
+		return -EINVAL;
+}
+
+
+static const struct file_operations pm_wakeup_blocker_fops = {
+	.write	 = pm_blocker_write,
+	.read	 = pm_blocker_read,
+	.poll	 = pm_blocker_poll,
+	.open	 = pm_blocker_open,
+	.release = pm_blocker_release,
+	.llseek	 = noop_llseek,
+};
+
+
 /**
  * wakeup_source_create - Create a struct wakeup_source object.
  * @name: Name of the new wakeup source.
@@ -63,11 +196,24 @@ struct wakeup_source *wakeup_source_create(const char *name)
 	if (!ws)
 		return NULL;
 
+	INIT_LIST_HEAD(&ws->blockers.entry);
 	spin_lock_init(&ws->lock);
 	if (name)
 		ws->name = kstrdup(name, GFP_KERNEL);
 
-	return ws;
+	ws->miscdev.minor = MISC_DYNAMIC_MINOR;
+	ws->miscdev.name = ws->name;
+	ws->miscdev.fops = &pm_wakeup_blocker_fops;
+	init_waitqueue_head(&ws->wq);
+
+	if (misc_register(&ws->miscdev))
+		return ws;
+	else {
+		kfree(ws->name);
+		kfree(ws);
+	}
+	return NULL;
+
 }
 EXPORT_SYMBOL_GPL(wakeup_source_create);
 
@@ -81,6 +227,7 @@ void wakeup_source_destroy(struct wakeup_source *ws)
 		return;
 
 	spin_lock_irq(&ws->lock);
+	ws->dying = 1;
 	while (ws->active) {
 		spin_unlock_irq(&ws->lock);
 
@@ -89,6 +236,15 @@ void wakeup_source_destroy(struct wakeup_source *ws)
 		spin_lock_irq(&ws->lock);
 	}
 	spin_unlock_irq(&ws->lock);
+	/*
+	 * drain the wq of sleepers
+	 */
+	wake_up(&ws->wq);
+	while (waitqueue_active(&ws->wq))
+		schedule_timeout_interruptible(msecs_to_jiffies(TIMEOUT));
+
+	misc_deregister(&ws->miscdev);
+	/* Do I need to force close the misc file nodes? if so how? */
 
 	kfree(ws->name);
 	kfree(ws);
@@ -362,12 +518,24 @@ static void wakeup_source_activate(struct wakeup_source *ws)
 void __pm_stay_awake(struct wakeup_source *ws)
 {
 	unsigned long flags;
+	struct relax_blocker *blocker;
 
 	if (!ws)
 		return;
 
 	spin_lock_irqsave(&ws->lock, flags);
 	ws->event_count++;
+	/*
+	 * set all waked_acked to zero and poke stay awake for each
+	 * blocker that is waiting on a notification the relax calls
+	 * happen in the write functions.
+	 */
+	list_for_each_entry_rcu(blocker, &(ws->blockers.entry), entry) {
+		blocker->resumes_to_ack += 1;
+		ws->event_count++;
+	}
+	wake_up(&ws->wq);
+
 	if (!ws->active)
 		wakeup_source_activate(ws);
 	spin_unlock_irqrestore(&ws->lock, flags);
@@ -512,7 +680,6 @@ void __pm_wakeup_event(struct wakeup_source *ws, unsigned int msec)
 {
 	unsigned long flags;
 	unsigned long expires;
-
 	if (!ws)
 		return;
 
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index a32da96..178f7c3 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -26,7 +26,24 @@
 # error "please don't include this file directly"
 #endif
 
-#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/wait.h>
+
+/**
+ * struct relax_blocker - list of processes that must acknowledge wake
+ *                        event before pm_relax is called.
+ *
+ * @resumes_to_ack: number of events to acknowledge.
+ * @ws: wakeup_source this blocker is associated with set up at fops.open time.
+ */
+struct wakeup_source;
+struct relax_blocker {
+	struct list_head	entry;
+	int			resumes_to_ack;/* count of pending acks needed
+						  from this blocker before
+						  unblocking*/
+	struct wakeup_source	*ws;
+};
 
 /**
  * struct wakeup_source - Representation of wakeup sources
@@ -35,10 +52,15 @@
  * @max_time: Maximum time this wakeup source has been continuously active.
  * @last_time: Monotonic clock when the wakeup source's was activated last time.
  * @event_count: Number of signaled wakeup events.
- * @active_count: Number of times the wakeup sorce was activated.
- * @relax_count: Number of times the wakeup sorce was deactivated.
- * @hit_count: Number of times the wakeup sorce might abort system suspend.
+ * @active_count: Number of times the wakeup source was activated.
+ * @relax_count: Number of times the wakeup source was deactivated.
+ * @hit_count: Number of times the wakeup source might abort system suspend.
  * @active: Status of the wakeup source.
+ * @dying: flag to handle removing wakeup source with active users
+ * @wq: work queue for waking blocked processes on the wakeup source
+ * @miscdev: miscellaneous device needed for talking to user mode processes
+ * @blockers: list of registered processes that need to ack the wakeup event
+ *            before the next suspend is allowed to proceed.
  */
 struct wakeup_source {
 	char 			*name;
@@ -46,14 +68,19 @@ struct wakeup_source {
 	spinlock_t		lock;
 	struct timer_list	timer;
 	unsigned long		timer_expires;
-	ktime_t total_time;
-	ktime_t max_time;
-	ktime_t last_time;
+	ktime_t			total_time;
+	ktime_t			max_time;
+	ktime_t			last_time;
 	unsigned long		event_count;
 	unsigned long		active_count;
 	unsigned long		relax_count;
 	unsigned long		hit_count;
 	unsigned int		active:1;
+	unsigned int		dying:1;
+	wait_queue_head_t	wq;
+	struct miscdevice	miscdev;
+	struct relax_blocker	blockers;
+
 };
 
 #ifdef CONFIG_PM_SLEEP
-- 
1.7.4.1


----- End forwarded message -----

Re: [markgross@thengar.org: Re: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)]

[mark gross]

On Mon, Oct 03, 2011 at 09:32:26AM +0100, Alan Cox wrote:
> On Sun, 2 Oct 2011 09:48:49 -0700
> mark gross <markgross@thegnar.org> wrote:
> 
> > Forwarding to bigger group for discussion.
> 
> Looks clean enough - only question I have is do we need a separate
> 'suspend block' or can latency do it -suspend is a very very high latency
> event. I guess the suspend block is clearer in intent than abusing latency
> but I do wonder if the actual suspend path should also check latency
> constraints too. If I've asked for 5mS latency then suspend is a wrong
> choice!
> 
I don't think over loading the meaning of cpu_dma_latency would work
well for this.  If cpu_dma_latency constraint is > 5mS then EFAIL a
suspend attempt?  feels clunky.

If you use less than then anyone using a constraint (like some wifi
and audio) would block suspend.  also not so good.  


The poster child for needing this is the USB gadget device with
associated charging over USB.  Here we can't sleep because we'd have a
hard time meeting USB spec if we did.  Also, we don't want to sleep
while charging if charging is under OS control.  (checking temperature
is tricky while suspended.)

--mark

Re: [markgross@thengar.org: Re: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)]

[Alan Cox]

On Sun, 2 Oct 2011 09:48:49 -0700
mark gross <markgross@thegnar.org> wrote:

> Forwarding to bigger group for discussion.

Looks clean enough - only question I have is do we need a separate
'suspend block' or can latency do it -suspend is a very very high latency
event. I guess the suspend block is clearer in intent than abusing latency
but I do wonder if the actual suspend path should also check latency
constraints too. If I've asked for 5mS latency then suspend is a wrong
choice!

Alan

[markgross@thengar.org: Re: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)]

[mark gross]

Forwarding to bigger group for discussion.

----- Forwarded message from mark gross <markgross@thengar.org> -----

Subject: Re: [RFC] wake up notifications and suspend blocking (aka more wakelock stuff)
Date: Tue, 20 Sep 2011 13:36:15 -0700
From: mark gross <markgross@thengar.org>
To: mark gross <markgross@thegnar.org>
Reply-To: markgross@thegnar.org
Cc: linux-pm@lists.linux-foundation.org, arve@android.com, Alan Stern <stern@rowland.harvard.edu>, amit.kucheria@linaro.org, farrowg@sg.ibm.com, "Rafael J. Wysocki" <rjw@sisk.pl>


>From a2615c59cb3b7632cf22e2c25a10b401e2151bbf Mon Sep 17 00:00:00 2001
From: mark gross <mark97229@gmail.com>
Date: Mon, 19 Sep 2011 09:48:31 -0700
Subject: [PATCH 2/2] implement suspend bock pm_qos class such that whenever the
 pm_qos_request is > 0 suspend to ram is blocked.

This is one of the 2 features the wakelock design attempts to implement.
(the other is wake even notification consumption before re-entry into
suspend)

Signed-off-by: mark gross <mark97229@gmail.com>
---
 drivers/base/power/wakeup.c   |    5 +++--
 include/linux/pm_qos_params.h |    4 +++-
 kernel/pm_qos_params.c        |   14 +++++++++++++-
 kernel/power/suspend.c        |    7 +++++++
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index eb300d7..790010d 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -10,8 +10,8 @@
 #include <linux/slab.h>
 #include <linux/sched.h>
 #include <linux/capability.h>
-#include <linux/freezer.h>
 #include <linux/miscdevice.h>
+#include <linux/pm_qos_params.h>
 #include <linux/suspend.h>
 #include <linux/seq_file.h>
 #include <linux/debugfs.h>
@@ -791,7 +791,8 @@ bool pm_get_wakeup_count(unsigned int *count)
 
 	for (;;) {
 		split_counters(&cnt, &inpr);
-		if (inpr == 0 || signal_pending(current))
+		if (((pm_qos_request(PM_QOS_SUSPEND_BLOCK) < 1) && (inpr == 0))
+				|| signal_pending(current))
 			break;
 		pm_wakeup_update_hit_counts();
 		schedule_timeout_interruptible(msecs_to_jiffies(TIMEOUT));
diff --git a/include/linux/pm_qos_params.h b/include/linux/pm_qos_params.h
index a7d87f9..19b771d 100644
--- a/include/linux/pm_qos_params.h
+++ b/include/linux/pm_qos_params.h
@@ -12,13 +12,15 @@
 #define PM_QOS_CPU_DMA_LATENCY 1
 #define PM_QOS_NETWORK_LATENCY 2
 #define PM_QOS_NETWORK_THROUGHPUT 3
+#define PM_QOS_SUSPEND_BLOCK 4
 
-#define PM_QOS_NUM_CLASSES 4
+#define PM_QOS_NUM_CLASSES 5
 #define PM_QOS_DEFAULT_VALUE -1
 
 #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE	(2000 * USEC_PER_SEC)
 #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE	(2000 * USEC_PER_SEC)
 #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE	0
+#define PM_QOS_SUSPEND_BLOCK_DEFAULT_VALUE	0
 
 struct pm_qos_request_list {
 	struct plist_node list;
diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index 37f05d0..838ca8a 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -104,11 +104,23 @@ static struct pm_qos_object network_throughput_pm_qos = {
 };
 
 
+static BLOCKING_NOTIFIER_HEAD(suspend_block_notifier);
+static struct pm_qos_object suspend_block_pm_qos = {
+	.requests = PLIST_HEAD_INIT(suspend_block_pm_qos.requests),
+	.notifiers = &suspend_block_notifier,
+	.name = "suspend_block",
+	.target_value = PM_QOS_SUSPEND_BLOCK_DEFAULT_VALUE,
+	.default_value = PM_QOS_SUSPEND_BLOCK_DEFAULT_VALUE,
+	.type = PM_QOS_MAX,
+};
+
+
 static struct pm_qos_object *pm_qos_array[] = {
 	&null_pm_qos,
 	&cpu_dma_pm_qos,
 	&network_lat_pm_qos,
-	&network_throughput_pm_qos
+	&network_throughput_pm_qos,
+	&suspend_block_pm_qos
 };
 
 static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index b6b71ad..e71c8d7 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/mm.h>
+#include <linux/pm_qos_params.h>
 #include <linux/slab.h>
 #include <linux/suspend.h>
 #include <linux/syscore_ops.h>
@@ -278,6 +279,12 @@ int enter_state(suspend_state_t state)
 	if (!valid_state(state))
 		return -ENODEV;
 
+	if (state == PM_SUSPEND_MEM)
+		if (0 < pm_qos_request(PM_QOS_SUSPEND_BLOCK)) {
+			WARN(1, "suspend blocked by pm_qos");
+			return -EBUSY;
+		}
+
 	if (!mutex_trylock(&pm_mutex))
 		return -EBUSY;
 
-- 
1.7.4.1


----- End forwarded message -----