[RFC PATCH v2] suspend/resume performance improvement

[RFC PATCH v2] suspend/resume performance improvement

[EunTaik Lee]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 3989 bytes --]

When a task that calls state_store() to suspend
the device has used up most of its time slice,
suspend sometimes take too long. (User noticeable)

Suspend/resume is a system wide operation.
So, instead of depending on a userspace task's time
slice, let kworker do the work to avoid a long wait
on the runqueue.

Signed-off-by: Eun Taik Lee <eun.taik.lee@samsung.com>
---
 kernel/power/main.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 90 insertions(+), 5 deletions(-)

diff --git a/kernel/power/main.c b/kernel/power/main.c
index 86e8157..848bf2d 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -15,7 +15,7 @@
 #include <linux/workqueue.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
-
+#include <linux/completion.h>
 #include "power.h"
 
 DEFINE_MUTEX(pm_mutex);
@@ -335,15 +335,46 @@ static suspend_state_t decode_state(const char *buf, size_t n)
 	return PM_SUSPEND_ON;
 }
 
+static struct workqueue_struct *suspend_helper_wq;
+struct state_store_params {
+	const char *buf;
+	size_t n;
+};
+
+struct suspend_helper_data {
+	struct work_struct work;
+	struct completion done;
+	struct state_store_params params;
+	int result;
+};
+struct suspend_helper_data suspend_helper_data;
+
+static ssize_t state_store_helper(struct kobject *kobj,
+		struct kobj_attribute *attr,
+		const char *buf, size_t n);
+
 static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr,
 			   const char *buf, size_t n)
 {
+	return state_store_helper(kobj, attr, buf, n);
+}
+
+power_attr(state);
+
+static void suspend_helper(struct work_struct *work)
+{
+	struct suspend_helper_data *data = (struct suspend_helper_data *)
+		container_of(work, struct suspend_helper_data, work);
+	const char *buf = data->params.buf;
+	size_t n = data->params.n;
 	suspend_state_t state;
-	int error;
+	int error = 0;
+
+	pr_debug("%s: start!\n", __func__);
 
 	error = pm_autosleep_lock();
 	if (error)
-		return error;
+		goto out_nolock;
 
 	if (pm_autosleep_state() > PM_SUSPEND_ON) {
 		error = -EBUSY;
@@ -358,12 +389,64 @@ static ssize_t state_store(struct kobject *kobj, struct kobj_attribute *attr,
 	else
 		error = -EINVAL;
 
- out:
+out:
 	pm_autosleep_unlock();
+
+out_nolock:
+	/* set result and notify completion */
+	data->result = error;
+	complete(&data->done);
+
+	pr_debug("%s: result = %d\n", __func__, error);
+}
+
+static ssize_t state_store_helper(struct kobject *kobj,
+		struct kobj_attribute *attr,
+		const char *buf, size_t n)
+{
+	int error;
+	int freezable = 0;
+
+	/* we don't need to freeze. so tell the freezer */
+	if (!freezer_should_skip(current)) {
+		freezable = 1;
+		freezer_do_not_count();
+		pr_debug("%s: freezer should skip me (%s:%d)\n",
+			__func__, current->comm, current->pid);
+	}
+
+	suspend_helper_data.params.buf = buf;
+	suspend_helper_data.params.n = n;
+	init_completion(&suspend_helper_data.done);
+
+	/* use kworker for suspend resume */
+	queue_work(suspend_helper_wq, &suspend_helper_data.work);
+
+	/* wait for suspend/resume work to be complete */
+	wait_for_completion(&suspend_helper_data.done);
+
+	if (freezable) {
+		/* set ourself as freezable */
+		freezer_count();
+	}
+
+	error = suspend_helper_data.result;
+	pr_debug("%s: suspend_helper returned %d\n", __func__, error);
+
 	return error ? error : n;
 }
 
-power_attr(state);
+static int suspend_helper_init(void)
+{
+	suspend_helper_wq = alloc_ordered_workqueue("suspend_helper", 0);
+	if (!suspend_helper_wq)
+		return -ENOMEM;
+
+	INIT_WORK(&suspend_helper_data.work, suspend_helper);
+	init_completion(&suspend_helper_data.done);
+
+	return 0;
+}
 
 #ifdef CONFIG_PM_SLEEP
 /*
@@ -640,6 +723,8 @@ static int __init pm_init(void)
 	if (error)
 		return error;
 	pm_print_times_init();
+	suspend_helper_init();
+
 	return pm_autosleep_init();
 }
 
-- 
1.9.1

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [RFC PATCH v2] suspend/resume performance improvement

[Rafael J. Wysocki]

On Sunday, May 31, 2015 05:07:42 AM EunTaik Lee wrote:
> When a task that calls state_store() to suspend
> the device has used up most of its time slice,
> suspend sometimes take too long. (User noticeable)

How long?  Perhaps this is not the only explanation of the delay?

> Suspend/resume is a system wide operation.
> So, instead of depending on a userspace task's time
> slice, let kworker do the work to avoid a long wait
> on the runqueue.
> 
> Signed-off-by: Eun Taik Lee <eun.taik.lee@samsung.com>

First, if you send a v2, please say what's changed since v1.

Second, this most likely is hiding a problem of some sort and the theory
about the time slice being used up entirely by the suspend process is not
convincing.

Also the patch duplicates exsiting functionality that should be extended
instead if anything.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.