linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH, v9 0/3] Introduce timer slack controller
@ 2011-03-14 14:05 Kirill A. Shutemov
  2011-03-14 14:05 ` [PATCH, v9 1/3] hrtimer: introduce effective timer slack Kirill A. Shutemov
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Kirill A. Shutemov @ 2011-03-14 14:05 UTC (permalink / raw)
  To: Paul Menage, Li Zefan, Andrew Morton, Thomas Gleixner
  Cc: containers, jacob.jun.pan, Arjan van de Ven, linux-kernel,
	Matt Helsley, linux-api, Kirill A. Shutemov

Changelog:

v9:
 - update documentation
 - minor cleanup
v8:
 - change hiearchy rules
 - introduce timer_slack.effective_slack_ns
 - get_task_timer_slack() -> task_get_effective_timer_slack()
 - task_get_effective_timer_slack() splited in separate patch
 - implement PR_GET_EFFECTIVE_TIMERSLACK
v7:
 - totally reworked interface and rewritten from scratch
   (See Documentation/cgroups/timer_slack.txt for more information)
v6:
 - add documentation
 - use notifier_call_chain() instead of check hook
 - fix validate_change()
 - cleanup
v5:
 - -EBUSY on writing to timer_slack.min_slack_ns/max_slack_ns if a child has
   wider min-max range
v4:
 - hierarchy support
 - drop dummy_timer_slack_check()
 - workaround lockdep false (?) positive
 - allow 0 as timer slack value
v3:
 - rework interface
 - s/EXPORT_SYMBOL/EXPORT_SYMBOL_GPL/
v2:
 - fixed with CONFIG_CGROUP_TIMER_SLACK=y
v1:
 - initial revision

Kirill A. Shutemov (3):
  hrtimer: introduce effective timer slack
  hrtimer: implement PR_GET_EFFECTIVE_TIMERSLACK
  cgroups: introduce timer slack controller

 Documentation/cgroups/timer_slack.txt |   72 +++++++++++++++++++
 fs/select.c                           |    7 +--
 include/linux/cgroup_subsys.h         |    6 ++
 include/linux/prctl.h                 |    6 ++
 include/linux/sched.h                 |   10 +++
 init/Kconfig                          |    8 ++
 kernel/Makefile                       |    1 +
 kernel/cgroup_timer_slack.c           |  124 +++++++++++++++++++++++++++++++++
 kernel/fork.c                         |    4 +
 kernel/futex.c                        |    4 +-
 kernel/hrtimer.c                      |    2 +-
 kernel/sys.c                          |    3 +
 12 files changed, 239 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/cgroups/timer_slack.txt
 create mode 100644 kernel/cgroup_timer_slack.c

-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH, v9 1/3] hrtimer: introduce effective timer slack
  2011-03-14 14:05 [PATCH, v9 0/3] Introduce timer slack controller Kirill A. Shutemov
@ 2011-03-14 14:05 ` Kirill A. Shutemov
  2011-03-14 14:05 ` [PATCH, v9 2/3] hrtimer: implement PR_GET_EFFECTIVE_TIMERSLACK Kirill A. Shutemov
  2011-03-14 14:05 ` [PATCH, v9 3/3] cgroups: introduce timer slack controller Kirill A. Shutemov
  2 siblings, 0 replies; 7+ messages in thread
From: Kirill A. Shutemov @ 2011-03-14 14:05 UTC (permalink / raw)
  To: Paul Menage, Li Zefan, Andrew Morton, Thomas Gleixner
  Cc: containers, jacob.jun.pan, Arjan van de Ven, linux-kernel,
	Matt Helsley, linux-api, Kirill A. Shutemov

task_get_effective_timer_slack() returns timer slack value to be used
to configure per-task timers. It can be equal or higher than task's
timer slack value.

For now task_get_effective_timer_slack() returns timer_slack_ns of the
task. Timer slack cgroup controller will implement a bit more
sophisticated logic.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 fs/select.c           |    7 ++-----
 include/linux/sched.h |    6 ++++++
 kernel/fork.c         |    4 ++++
 kernel/futex.c        |    4 ++--
 kernel/hrtimer.c      |    2 +-
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/fs/select.c b/fs/select.c
index e56560d..fb7d6ec 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -69,7 +69,6 @@ static long __estimate_accuracy(struct timespec *tv)
 
 long select_estimate_accuracy(struct timespec *tv)
 {
-	unsigned long ret;
 	struct timespec now;
 
 	/*
@@ -81,10 +80,8 @@ long select_estimate_accuracy(struct timespec *tv)
 
 	ktime_get_ts(&now);
 	now = timespec_sub(*tv, now);
-	ret = __estimate_accuracy(&now);
-	if (ret < current->timer_slack_ns)
-		return current->timer_slack_ns;
-	return ret;
+	return min_t(long, __estimate_accuracy(&now),
+			task_get_effective_timer_slack(current));
 }
 
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 777d8a5..44f6f55 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2620,6 +2620,12 @@ static inline unsigned long rlimit_max(unsigned int limit)
 	return task_rlimit_max(current, limit);
 }
 
+static inline unsigned long task_get_effective_timer_slack(
+		struct task_struct *tsk)
+{
+	return tsk->timer_slack_ns;
+}
+
 #endif /* __KERNEL__ */
 
 #endif
diff --git a/kernel/fork.c b/kernel/fork.c
index 25e4291..4958de9 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1089,6 +1089,10 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 	memset(&p->rss_stat, 0, sizeof(p->rss_stat));
 #endif
 
+	/*
+	 * Save current task's (not effective) timer slack value as default
+	 * timer slack value for new task.
+	 */
 	p->default_timer_slack_ns = current->timer_slack_ns;
 
 	task_io_accounting_init(&p->ioac);
diff --git a/kernel/futex.c b/kernel/futex.c
index b766d28..e5811c4 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1845,7 +1845,7 @@ static int futex_wait(u32 __user *uaddr, unsigned int flags, u32 val,
 				      HRTIMER_MODE_ABS);
 		hrtimer_init_sleeper(to, current);
 		hrtimer_set_expires_range_ns(&to->timer, *abs_time,
-					     current->timer_slack_ns);
+				     task_get_effective_timer_slack(current));
 	}
 
 retry:
@@ -2242,7 +2242,7 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
 				      HRTIMER_MODE_ABS);
 		hrtimer_init_sleeper(to, current);
 		hrtimer_set_expires_range_ns(&to->timer, *abs_time,
-					     current->timer_slack_ns);
+				     task_get_effective_timer_slack(current));
 	}
 
 	/*
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 0c8d7c0..ffc9a8d 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -1542,7 +1542,7 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp,
 	int ret = 0;
 	unsigned long slack;
 
-	slack = current->timer_slack_ns;
+	slack = task_get_effective_timer_slack(current);
 	if (rt_task(current))
 		slack = 0;
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH, v9 2/3] hrtimer: implement PR_GET_EFFECTIVE_TIMERSLACK
  2011-03-14 14:05 [PATCH, v9 0/3] Introduce timer slack controller Kirill A. Shutemov
  2011-03-14 14:05 ` [PATCH, v9 1/3] hrtimer: introduce effective timer slack Kirill A. Shutemov
@ 2011-03-14 14:05 ` Kirill A. Shutemov
  2011-03-14 14:05 ` [PATCH, v9 3/3] cgroups: introduce timer slack controller Kirill A. Shutemov
  2 siblings, 0 replies; 7+ messages in thread
From: Kirill A. Shutemov @ 2011-03-14 14:05 UTC (permalink / raw)
  To: Paul Menage, Li Zefan, Andrew Morton, Thomas Gleixner
  Cc: containers, jacob.jun.pan, Arjan van de Ven, linux-kernel,
	Matt Helsley, linux-api, Kirill A. Shutemov

PR_GET_EFFECTIVE_TIMERSLACK allows process to know its effective timer
slack value.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 include/linux/prctl.h |    6 ++++++
 kernel/sys.c          |    3 +++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/linux/prctl.h b/include/linux/prctl.h
index a3baeb2..3199458 100644
--- a/include/linux/prctl.h
+++ b/include/linux/prctl.h
@@ -102,4 +102,10 @@
 
 #define PR_MCE_KILL_GET 34
 
+/*
+ * Get effective timerslack value for the process.
+ * It can be higher than PR_GET_TIMERSLACK.
+ */
+#define PR_GET_EFFECTIVE_TIMERSLACK 35
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 18da702..b2ec8bf 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1693,6 +1693,9 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 		case PR_GET_TIMERSLACK:
 			error = current->timer_slack_ns;
 			break;
+		case PR_GET_EFFECTIVE_TIMERSLACK:
+			error = task_get_effective_timer_slack(current);
+			break;
 		case PR_SET_TIMERSLACK:
 			if (arg2 <= 0)
 				current->timer_slack_ns =
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH, v9 3/3] cgroups: introduce timer slack controller
  2011-03-14 14:05 [PATCH, v9 0/3] Introduce timer slack controller Kirill A. Shutemov
  2011-03-14 14:05 ` [PATCH, v9 1/3] hrtimer: introduce effective timer slack Kirill A. Shutemov
  2011-03-14 14:05 ` [PATCH, v9 2/3] hrtimer: implement PR_GET_EFFECTIVE_TIMERSLACK Kirill A. Shutemov
@ 2011-03-14 14:05 ` Kirill A. Shutemov
  2011-03-14 23:46   ` Andrew Morton
  2 siblings, 1 reply; 7+ messages in thread
From: Kirill A. Shutemov @ 2011-03-14 14:05 UTC (permalink / raw)
  To: Paul Menage, Li Zefan, Andrew Morton, Thomas Gleixner
  Cc: containers, jacob.jun.pan, Arjan van de Ven, linux-kernel,
	Matt Helsley, linux-api, Kirill A. Shutemov

Every task_struct has timer_slack_ns value. This value uses to round up
poll() and select() timeout values. This feature can be useful in
mobile environment where combined wakeups are desired.

Originally, prctl() was the only way to change timer slack value of
a process. So you was not able change timer slack value of another
process.

cgroup subsys "timer_slack" implements timer slack controller. It
provides a way to set minimal timer slack value for a group of tasks.
If a task belongs to a cgroup with minimal timer slack value higher than
task's value, cgroup's value will be applied.

Timer slack controller allows to implement setting timer slack value of
a process based on a policy. For example, you can create foreground and
background cgroups and move tasks between them based on system state.

Idea-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 Documentation/cgroups/timer_slack.txt |   72 +++++++++++++++++++
 include/linux/cgroup_subsys.h         |    6 ++
 include/linux/sched.h                 |    4 +
 init/Kconfig                          |    8 ++
 kernel/Makefile                       |    1 +
 kernel/cgroup_timer_slack.c           |  124 +++++++++++++++++++++++++++++++++
 6 files changed, 215 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/cgroups/timer_slack.txt
 create mode 100644 kernel/cgroup_timer_slack.c

diff --git a/Documentation/cgroups/timer_slack.txt b/Documentation/cgroups/timer_slack.txt
new file mode 100644
index 0000000..4006eab
--- /dev/null
+++ b/Documentation/cgroups/timer_slack.txt
@@ -0,0 +1,72 @@
+Timer Slack Controller
+======================
+
+Overview
+--------
+
+Every task_struct has timer_slack_ns value. This value uses to round up
+poll() and select() timeout values. This feature can be useful in
+mobile environment where combined wakeups are desired.
+
+Originally, prctl() was the only way to change timer slack value of
+a process. So you was not able change timer slack value of another
+process.
+
+cgroup subsys "timer_slack" implements timer slack controller. It
+provides a way to set minimal timer slack value for a group of tasks.
+If a task belongs to a cgroup with minimal timer slack value higher than
+task's value, cgroup's value will be applied.
+
+Timer slack controller allows to implement setting timer slack value of
+a process based on a policy. For example, you can create foreground and
+background cgroups and move tasks between them based on system state.
+
+User interface
+--------------
+
+To get timer slack controller functionality you need to enable it in
+kernel configuration:
+
+CONFIG_CGROUP_TIMER_SLACK=y
+
+The controller provides two files:
+
+# mount -t cgroup -o timer_slack none /sys/fs/cgroup
+# ls /sys/fs/cgroup/timer_slack.*
+/sys/fs/cgroup/timer_slack.effective_slack_ns
+/sys/fs/cgroup/timer_slack.min_slack_ns
+
+By default timer_slack.min_slack_ns is 0:
+
+# cat /sys/fs/cgroup/timer_slack.min_slack_ns
+0
+
+You can set it to some value:
+
+# echo 50000 > /sys/fs/cgroup/timer_slack.min_slack_ns
+# cat /sys/fs/cgroup/timer_slack.min_slack_ns
+50000
+
+Tasks still can set task's value below 50000 using prctl(), but in this
+case cgroup's value will be applied.
+
+Timer slack controller supports hierarchical groups.
+
+# mkdir /sys/fs/cgroup/a
+# cat /sys/fs/cgroup/a/timer_slack.min_slack_ns
+50000
+# echo 70000 > /sys/fs/cgroup/a/timer_slack.min_slack_ns
+# cat /sys/fs/cgroup/a/timer_slack.min_slack_ns
+70000
+
+You can set any value you want, but effective value will the highest value
+up by hierarchy. You can see effective timer slack value for the cgroup from
+timer_slack.effective_slack_ns file:
+
+# cat /sys/fs/cgroup/a/timer_slack.effective_slack_ns
+70000
+# echo 100000 > /sys/fs/cgroup/timer_slack.min_slack_ns
+# cat /sys/fs/cgroup/a/timer_slack.min_slack_ns
+70000
+# cat /sys/fs/cgroup/a/timer_slack.effective_slack_ns
+100000
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index ccefff0..e399228 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -66,3 +66,9 @@ SUBSYS(blkio)
 #endif
 
 /* */
+
+#ifdef CONFIG_CGROUP_TIMER_SLACK
+SUBSYS(timer_slack)
+#endif
+
+/* */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 44f6f55..c10da5d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2620,11 +2620,15 @@ static inline unsigned long rlimit_max(unsigned int limit)
 	return task_rlimit_max(current, limit);
 }
 
+#ifdef CONFIG_CGROUP_TIMER_SLACK
+extern unsigned long task_get_effective_timer_slack(struct task_struct *tsk);
+#else
 static inline unsigned long task_get_effective_timer_slack(
 		struct task_struct *tsk)
 {
 	return tsk->timer_slack_ns;
 }
+#endif
 
 #endif /* __KERNEL__ */
 
diff --git a/init/Kconfig b/init/Kconfig
index be788c0..bbc4d9c 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -596,6 +596,14 @@ config CGROUP_FREEZER
 	  Provides a way to freeze and unfreeze all tasks in a
 	  cgroup.
 
+config CGROUP_TIMER_SLACK
+	bool "Timer slack cgroup controller"
+	help
+	  Provides a way to set minimal timer slack value for tasks in
+	  a cgroup.
+	  It's useful in mobile devices where certain background apps
+	  are attached to a cgroup and combined wakeups are desired.
+
 config CGROUP_DEVICE
 	bool "Device controller for cgroups"
 	help
diff --git a/kernel/Makefile b/kernel/Makefile
index 353d3fe..0b60239 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_BACKTRACE_SELF_TEST) += backtracetest.o
 obj-$(CONFIG_COMPAT) += compat.o
 obj-$(CONFIG_CGROUPS) += cgroup.o
 obj-$(CONFIG_CGROUP_FREEZER) += cgroup_freezer.o
+obj-$(CONFIG_CGROUP_TIMER_SLACK) += cgroup_timer_slack.o
 obj-$(CONFIG_CPUSETS) += cpuset.o
 obj-$(CONFIG_CGROUP_NS) += ns_cgroup.o
 obj-$(CONFIG_UTS_NS) += utsname.o
diff --git a/kernel/cgroup_timer_slack.c b/kernel/cgroup_timer_slack.c
new file mode 100644
index 0000000..9824807
--- /dev/null
+++ b/kernel/cgroup_timer_slack.c
@@ -0,0 +1,124 @@
+/*
+ * cgroup_timer_slack.c - control group timer slack subsystem
+ *
+ * Copyright Nokia Corparation, 2011
+ * Author: Kirill A. Shutemov
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/cgroup.h>
+#include <linux/slab.h>
+
+struct cgroup_subsys timer_slack_subsys;
+struct tslack_cgroup {
+	struct cgroup_subsys_state css;
+	unsigned long min_slack_ns;
+};
+
+static struct tslack_cgroup *cgroup_to_tslack(struct cgroup *cgroup)
+{
+	struct cgroup_subsys_state *css;
+
+	css = cgroup_subsys_state(cgroup, timer_slack_subsys.subsys_id);
+	return container_of(css, struct tslack_cgroup, css);
+}
+
+static struct cgroup_subsys_state *tslack_create(struct cgroup_subsys *subsys,
+		struct cgroup *cgroup)
+{
+	struct tslack_cgroup *tslack_cgroup;
+
+	tslack_cgroup = kmalloc(sizeof(*tslack_cgroup), GFP_KERNEL);
+	if (!tslack_cgroup)
+		return ERR_PTR(-ENOMEM);
+
+	if (cgroup->parent) {
+		struct tslack_cgroup *parent;
+
+		parent = cgroup_to_tslack(cgroup->parent);
+		tslack_cgroup->min_slack_ns = parent->min_slack_ns;
+	} else
+		tslack_cgroup->min_slack_ns = 0UL;
+
+	return &tslack_cgroup->css;
+}
+
+static void tslack_destroy(struct cgroup_subsys *tslack_cgroup,
+		struct cgroup *cgroup)
+{
+	kfree(cgroup_to_tslack(cgroup));
+}
+
+static u64 tslack_read_min(struct cgroup *cgroup, struct cftype *cft)
+{
+	return cgroup_to_tslack(cgroup)->min_slack_ns;
+}
+
+static int tslack_write_min(struct cgroup *cgroup, struct cftype *cft, u64 val)
+{
+	if (val > ULONG_MAX)
+		return -EINVAL;
+
+	cgroup_to_tslack(cgroup)->min_slack_ns = val;
+
+	return 0;
+}
+
+static u64 tslack_read_effective(struct cgroup *cgroup, struct cftype *cft)
+{
+	unsigned long min;
+
+	min = cgroup_to_tslack(cgroup)->min_slack_ns;
+	while (cgroup->parent) {
+		cgroup = cgroup->parent;
+		min = max(cgroup_to_tslack(cgroup)->min_slack_ns, min);
+	}
+
+	return min;
+}
+
+static struct cftype files[] = {
+	{
+		.name = "min_slack_ns",
+		.read_u64 = tslack_read_min,
+		.write_u64 = tslack_write_min,
+	},
+	{
+		.name = "effective_slack_ns",
+		.read_u64 = tslack_read_effective,
+	},
+};
+
+static int tslack_populate(struct cgroup_subsys *subsys, struct cgroup *cgroup)
+{
+	return cgroup_add_files(cgroup, subsys, files, ARRAY_SIZE(files));
+}
+
+struct cgroup_subsys timer_slack_subsys = {
+	.name		= "timer_slack",
+	.subsys_id	= timer_slack_subsys_id,
+	.create		= tslack_create,
+	.destroy	= tslack_destroy,
+	.populate	= tslack_populate,
+};
+
+unsigned long task_get_effective_timer_slack(struct task_struct *tsk)
+{
+	struct cgroup *cgroup;
+	unsigned long slack;
+
+	rcu_read_lock();
+	cgroup = task_cgroup(tsk, timer_slack_subsys.subsys_id);
+	slack = tslack_read_effective(cgroup, NULL);
+	rcu_read_unlock();
+
+	return max(tsk->timer_slack_ns, slack);
+}
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH, v9 3/3] cgroups: introduce timer slack controller
  2011-03-14 14:05 ` [PATCH, v9 3/3] cgroups: introduce timer slack controller Kirill A. Shutemov
@ 2011-03-14 23:46   ` Andrew Morton
  2011-03-15  9:11     ` Kirill A. Shutemov
  2011-03-16 11:35     ` jacob pan
  0 siblings, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2011-03-14 23:46 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Paul Menage, Li Zefan, Thomas Gleixner, containers,
	jacob.jun.pan, Arjan van de Ven, linux-kernel, Matt Helsley,
	linux-api

On Mon, 14 Mar 2011 16:05:24 +0200
Kirill A. Shutemov" <kirill@shutemov.name> wrote:

> +Overview
> +--------
> +
> +Every task_struct has timer_slack_ns value. This value uses to round up
> +poll() and select() timeout values. This feature can be useful in
> +mobile environment where combined wakeups are desired.
> +
> +Originally, prctl() was the only way to change timer slack value of
> +a process. So you was not able change timer slack value of another
> +process.
> +
> +cgroup subsys "timer_slack" implements timer slack controller. It
> +provides a way to set minimal timer slack value for a group of tasks.
> +If a task belongs to a cgroup with minimal timer slack value higher than
> +task's value, cgroup's value will be applied.
> +
> +Timer slack controller allows to implement setting timer slack value of
> +a process based on a policy. For example, you can create foreground and
> +background cgroups and move tasks between them based on system state.

(quoting myself from last time)

Why do we need a cgroup for this as opposed to (say) inheritance over
fork(), or a system-wide knob, or a per-process/threadgroup knob, or
just leaving the existing code as-is?  Presumably you felt that a
cgroup approach is better for manageability, but you didn't tell us
about this and you didn't explore alternative ways of solving the
problem-which-you-didn't-describe.




I'm still having trouble seeing why we should merge this.  Who will use
it, and for what reason and what benefits will they see?  Quantified
benefits, if possible!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, v9 3/3] cgroups: introduce timer slack controller
  2011-03-14 23:46   ` Andrew Morton
@ 2011-03-15  9:11     ` Kirill A. Shutemov
  2011-03-16 11:35     ` jacob pan
  1 sibling, 0 replies; 7+ messages in thread
From: Kirill A. Shutemov @ 2011-03-15  9:11 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Paul Menage, Li Zefan, Thomas Gleixner, containers,
	jacob.jun.pan, Arjan van de Ven, linux-kernel, Matt Helsley,
	linux-api

On Mon, Mar 14, 2011 at 04:46:52PM -0700, Andrew Morton wrote:
> On Mon, 14 Mar 2011 16:05:24 +0200
> Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> 
> > +Overview
> > +--------
> > +
> > +Every task_struct has timer_slack_ns value. This value uses to round up
> > +poll() and select() timeout values. This feature can be useful in
> > +mobile environment where combined wakeups are desired.
> > +
> > +Originally, prctl() was the only way to change timer slack value of
> > +a process. So you was not able change timer slack value of another
> > +process.
> > +
> > +cgroup subsys "timer_slack" implements timer slack controller. It
> > +provides a way to set minimal timer slack value for a group of tasks.
> > +If a task belongs to a cgroup with minimal timer slack value higher than
> > +task's value, cgroup's value will be applied.
> > +
> > +Timer slack controller allows to implement setting timer slack value of
> > +a process based on a policy. For example, you can create foreground and
> > +background cgroups and move tasks between them based on system state.
> 
> (quoting myself from last time)
> 
> Why do we need a cgroup for this as opposed to (say) inheritance over
> fork(), or a system-wide knob, or a per-process/threadgroup knob, or
> just leaving the existing code as-is?  Presumably you felt that a
> cgroup approach is better for manageability, but you didn't tell us
> about this and you didn't explore alternative ways of solving the
> problem-which-you-didn't-describe.

The main goal is reducing wakeups without affecting user experience.
Only userspace knows which tasks are important for UX. So we needed a
mechanism to define a policy in userspace.

Currently timer slack value inheritance over fork().

System-wide/threadgroup knob doesn't allow to change timer slack value of
a particular process. It's needed for foreground/background use-case.

With per-process know we have to reinvent grouping of tasks in userspace.

cgroups is a good solution for it, from my point of view.

> 
> I'm still having trouble seeing why we should merge this.  Who will use
> it, and for what reason and what benefits will they see?  Quantified
> benefits, if possible!
> 

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH, v9 3/3] cgroups: introduce timer slack controller
  2011-03-14 23:46   ` Andrew Morton
  2011-03-15  9:11     ` Kirill A. Shutemov
@ 2011-03-16 11:35     ` jacob pan
  1 sibling, 0 replies; 7+ messages in thread
From: jacob pan @ 2011-03-16 11:35 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kirill A. Shutemov, Paul Menage, Li Zefan, Thomas Gleixner,
	containers, Arjan van de Ven, linux-kernel, Matt Helsley,
	linux-api

On Mon, 14 Mar 2011 16:46:52 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Mon, 14 Mar 2011 16:05:24 +0200
> Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> 
> > +Overview
> > +--------
> > +
> > +Every task_struct has timer_slack_ns value. This value uses to
> > round up +poll() and select() timeout values. This feature can be
> > useful in +mobile environment where combined wakeups are desired.
> > +
> > +Originally, prctl() was the only way to change timer slack value of
> > +a process. So you was not able change timer slack value of another
> > +process.
> > +
> > +cgroup subsys "timer_slack" implements timer slack controller. It
> > +provides a way to set minimal timer slack value for a group of
> > tasks. +If a task belongs to a cgroup with minimal timer slack
> > value higher than +task's value, cgroup's value will be applied.
> > +
> > +Timer slack controller allows to implement setting timer slack
> > value of +a process based on a policy. For example, you can create
> > foreground and +background cgroups and move tasks between them
> > based on system state.
> 
> (quoting myself from last time)
> 
> Why do we need a cgroup for this as opposed to (say) inheritance over
> fork(), or a system-wide knob, or a per-process/threadgroup knob, or
> just leaving the existing code as-is?  Presumably you felt that a
> cgroup approach is better for manageability, but you didn't tell us
> about this and you didn't explore alternative ways of solving the
> problem-which-you-didn't-describe.
> 
> 
> 
> 
> I'm still having trouble seeing why we should merge this.  Who will
> use it, and for what reason and what benefits will they see?
> Quantified benefits, if possible!
> 
I am planning to use it for doing resource management for a mobile
device where we classify applications by groups based on priority and
trust level. Then at runtime, I plan to use timer slack as one of the
knobs to penalize/manage them at different degree to reduce power
consumption (via reduction of #wake-ups in this case).

Thanks,

Jacob

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-03-16 11:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-14 14:05 [PATCH, v9 0/3] Introduce timer slack controller Kirill A. Shutemov
2011-03-14 14:05 ` [PATCH, v9 1/3] hrtimer: introduce effective timer slack Kirill A. Shutemov
2011-03-14 14:05 ` [PATCH, v9 2/3] hrtimer: implement PR_GET_EFFECTIVE_TIMERSLACK Kirill A. Shutemov
2011-03-14 14:05 ` [PATCH, v9 3/3] cgroups: introduce timer slack controller Kirill A. Shutemov
2011-03-14 23:46   ` Andrew Morton
2011-03-15  9:11     ` Kirill A. Shutemov
2011-03-16 11:35     ` jacob pan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).