[PATCH, RESEND 1/2] cgroups: export cgroup_iter_{start,next,end}

[PATCH, RESEND 1/2] cgroups: export cgroup_iter_{start,next,end}

[Kirill A. Shutsemov]

From: Kirill A. Shutemov <kirill@shutemov.name>

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 kernel/cgroup.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index b24d702..e3be81a 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2443,6 +2443,7 @@ void cgroup_iter_start(struct cgroup *cgrp, struct cgroup_iter *it)
 	it->cg_link = &cgrp->css_sets;
 	cgroup_advance_iter(cgrp, it);
 }
+EXPORT_SYMBOL(cgroup_iter_start);
 
 struct task_struct *cgroup_iter_next(struct cgroup *cgrp,
 					struct cgroup_iter *it)
@@ -2467,11 +2468,13 @@ struct task_struct *cgroup_iter_next(struct cgroup *cgrp,
 	}
 	return res;
 }
+EXPORT_SYMBOL(cgroup_iter_next);
 
 void cgroup_iter_end(struct cgroup *cgrp, struct cgroup_iter *it)
 {
 	read_unlock(&css_set_lock);
 }
+EXPORT_SYMBOL(cgroup_iter_end);
 
 static inline int started_after_time(struct task_struct *t1,
 				     struct timespec *time,
-- 
1.7.3.5

[PATCH, v2 2/2] cgroups: introduce timer slack subsystem

[Kirill A. Shutsemov]

From: Kirill A. Shutemov <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>

Provides a way of tasks grouping by timer slack value. Introduces per
cgroup timer slack value which will override the default timer slack
value once a task is attached to a cgroup.

It's useful in mobile devices where certain background apps are
attached to a cgroup and minimum wakeups are desired.

Based on patch by Jacob Pan.

Signed-off-by: Kirill A. Shutemov <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>
---

Changelog:
v2:
 - fixed with CONFIG_CGROUP_TIMER_SLACK=y

---
 include/linux/cgroup_subsys.h |    6 ++
 include/linux/init_task.h     |    4 +-
 init/Kconfig                  |   10 ++++
 kernel/Makefile               |    1 +
 kernel/cgroup_timer_slack.c   |  116 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 136 insertions(+), 1 deletions(-)
 create mode 100644 kernel/cgroup_timer_slack.c

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/init_task.h b/include/linux/init_task.h
index caa151f..48eca8f 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -124,6 +124,8 @@ extern struct cred init_cred;
 # define INIT_PERF_EVENTS(tsk)
 #endif
 
+#define TIMER_SLACK_NS_DEFAULT 50000
+
 /*
  *  INIT_TASK is used to set up the first task table, touch at
  * your own risk!. Base=0, limit=0x1fffff (=2MB)
@@ -177,7 +179,7 @@ extern struct cred init_cred;
 	.cpu_timers	= INIT_CPU_TIMERS(tsk.cpu_timers),		\
 	.fs_excl	= ATOMIC_INIT(0),				\
 	.pi_lock	= __RAW_SPIN_LOCK_UNLOCKED(tsk.pi_lock),	\
-	.timer_slack_ns = 50000, /* 50 usec default slack */		\
+	.timer_slack_ns = TIMER_SLACK_NS_DEFAULT,			\
 	.pids = {							\
 		[PIDTYPE_PID]  = INIT_PID_LINK(PIDTYPE_PID),		\
 		[PIDTYPE_PGID] = INIT_PID_LINK(PIDTYPE_PGID),		\
diff --git a/init/Kconfig b/init/Kconfig
index be788c0..f21b4ce 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -596,6 +596,16 @@ config CGROUP_FREEZER
 	  Provides a way to freeze and unfreeze all tasks in a
 	  cgroup.
 
+config CGROUP_TIMER_SLACK
+	tristate "Timer slack cgroup subsystem"
+	help
+	  Provides a way of tasks grouping by timer slack value.
+	  Introduces per cgroup timer slack value which will override
+	  the default timer slack value once a task is attached to a
+	  cgroup.
+	  It's useful in mobile devices where certain background apps
+	  are attached to a cgroup and minimum 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..daa452d
--- /dev/null
+++ b/kernel/cgroup_timer_slack.c
@@ -0,0 +1,116 @@
+#include <linux/cgroup.h>
+#include <linux/init_task.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+struct cgroup_subsys timer_slack_subsys;
+struct timer_slack_cgroup {
+	struct cgroup_subsys_state css;
+	unsigned long timer_slack_ns;
+};
+
+static struct timer_slack_cgroup *cgroup_to_tslack_cgroup(struct cgroup *cgroup)
+{
+	struct cgroup_subsys_state *css;
+
+	css = cgroup_subsys_state(cgroup, timer_slack_subsys.subsys_id);
+	return container_of(css, struct timer_slack_cgroup, css);
+}
+
+static struct cgroup_subsys_state *
+tslack_cgroup_create(struct cgroup_subsys *subsys, struct cgroup *cgroup)
+{
+	struct timer_slack_cgroup *tslack_cgroup;
+	struct cgroup *parent = cgroup->parent;
+
+	tslack_cgroup = kmalloc(sizeof(*tslack_cgroup), GFP_KERNEL);
+	if (!tslack_cgroup)
+		return ERR_PTR(-ENOMEM);
+
+	if (parent)
+		tslack_cgroup->timer_slack_ns =
+			cgroup_to_tslack_cgroup(parent)->timer_slack_ns;
+	else
+		tslack_cgroup->timer_slack_ns = TIMER_SLACK_NS_DEFAULT;
+
+	return &tslack_cgroup->css;
+}
+
+static void tslack_cgroup_destroy(struct cgroup_subsys *subsys,
+		struct cgroup *cgroup)
+{
+	kfree(cgroup_to_tslack_cgroup(cgroup));
+}
+
+static void tslack_cgroup_attach(struct cgroup_subsys *subsys,
+		struct cgroup *cgroup, struct cgroup *prev,
+		struct task_struct *tsk, bool threadgroup)
+{
+	struct timer_slack_cgroup *tslack_cgroup;
+
+	tslack_cgroup = cgroup_to_tslack_cgroup(cgroup);
+	task_lock(tsk);
+	tsk->timer_slack_ns = tslack_cgroup->timer_slack_ns;
+	task_unlock(tsk);
+}
+
+static u64 tslack_read_slack_ns(struct cgroup *cgroup, struct cftype *cft)
+{
+       return cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns;
+}
+
+static int tslack_write_slack_ns(struct cgroup *cgroup, struct cftype *cft,
+		u64 val)
+{
+	struct cgroup_iter it;
+	struct task_struct *task;
+
+	if (!val)
+		return -EINVAL;
+
+	cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns = val;
+
+	/* change timer slack value for all tasks in the cgroup */
+	cgroup_iter_start(cgroup, &it);
+	while ((task = cgroup_iter_next(cgroup, &it)))
+		task->timer_slack_ns = val;
+	cgroup_iter_end(cgroup, &it);
+
+	return 0;
+}
+
+static struct cftype cft_timer_slack = {
+	.name = "slack_ns",
+	.read_u64 = tslack_read_slack_ns,
+	.write_u64 = tslack_write_slack_ns,
+};
+
+static int tslack_cgroup_populate(struct cgroup_subsys *subsys,
+		struct cgroup *cgroup)
+{
+	return cgroup_add_file(cgroup, subsys, &cft_timer_slack);
+}
+
+struct cgroup_subsys timer_slack_subsys = {
+	.name = "timer_slack",
+	.module = THIS_MODULE,
+	.subsys_id = timer_slack_subsys_id,
+	.create = tslack_cgroup_create,
+	.destroy = tslack_cgroup_destroy,
+	.attach = tslack_cgroup_attach,
+	.populate = tslack_cgroup_populate,
+};
+
+static int __init init_cgroup_timer_slack(void)
+{
+	return cgroup_load_subsys(&timer_slack_subsys);
+}
+
+static void __exit exit_cgroup_timer_slack(void)
+{
+	cgroup_unload_subsys(&timer_slack_subsys);
+}
+
+module_init(init_cgroup_timer_slack);
+module_exit(exit_cgroup_timer_slack);
+MODULE_LICENSE("GPL");
-- 
1.7.3.5

[PATCH, v2 2/2] cgroups: introduce timer slack subsystem

[Kirill A. Shutsemov]

From: Kirill A. Shutemov <kirill@shutemov.name>

Provides a way of tasks grouping by timer slack value. Introduces per
cgroup timer slack value which will override the default timer slack
value once a task is attached to a cgroup.

It's useful in mobile devices where certain background apps are
attached to a cgroup and minimum wakeups are desired.

Based on patch by Jacob Pan.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---

Changelog:
v2:
 - fixed with CONFIG_CGROUP_TIMER_SLACK=y

---
 include/linux/cgroup_subsys.h |    6 ++
 include/linux/init_task.h     |    4 +-
 init/Kconfig                  |   10 ++++
 kernel/Makefile               |    1 +
 kernel/cgroup_timer_slack.c   |  116 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 136 insertions(+), 1 deletions(-)
 create mode 100644 kernel/cgroup_timer_slack.c

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/init_task.h b/include/linux/init_task.h
index caa151f..48eca8f 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -124,6 +124,8 @@ extern struct cred init_cred;
 # define INIT_PERF_EVENTS(tsk)
 #endif
 
+#define TIMER_SLACK_NS_DEFAULT 50000
+
 /*
  *  INIT_TASK is used to set up the first task table, touch at
  * your own risk!. Base=0, limit=0x1fffff (=2MB)
@@ -177,7 +179,7 @@ extern struct cred init_cred;
 	.cpu_timers	= INIT_CPU_TIMERS(tsk.cpu_timers),		\
 	.fs_excl	= ATOMIC_INIT(0),				\
 	.pi_lock	= __RAW_SPIN_LOCK_UNLOCKED(tsk.pi_lock),	\
-	.timer_slack_ns = 50000, /* 50 usec default slack */		\
+	.timer_slack_ns = TIMER_SLACK_NS_DEFAULT,			\
 	.pids = {							\
 		[PIDTYPE_PID]  = INIT_PID_LINK(PIDTYPE_PID),		\
 		[PIDTYPE_PGID] = INIT_PID_LINK(PIDTYPE_PGID),		\
diff --git a/init/Kconfig b/init/Kconfig
index be788c0..f21b4ce 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -596,6 +596,16 @@ config CGROUP_FREEZER
 	  Provides a way to freeze and unfreeze all tasks in a
 	  cgroup.
 
+config CGROUP_TIMER_SLACK
+	tristate "Timer slack cgroup subsystem"
+	help
+	  Provides a way of tasks grouping by timer slack value.
+	  Introduces per cgroup timer slack value which will override
+	  the default timer slack value once a task is attached to a
+	  cgroup.
+	  It's useful in mobile devices where certain background apps
+	  are attached to a cgroup and minimum 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..daa452d
--- /dev/null
+++ b/kernel/cgroup_timer_slack.c
@@ -0,0 +1,116 @@
+#include <linux/cgroup.h>
+#include <linux/init_task.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+struct cgroup_subsys timer_slack_subsys;
+struct timer_slack_cgroup {
+	struct cgroup_subsys_state css;
+	unsigned long timer_slack_ns;
+};
+
+static struct timer_slack_cgroup *cgroup_to_tslack_cgroup(struct cgroup *cgroup)
+{
+	struct cgroup_subsys_state *css;
+
+	css = cgroup_subsys_state(cgroup, timer_slack_subsys.subsys_id);
+	return container_of(css, struct timer_slack_cgroup, css);
+}
+
+static struct cgroup_subsys_state *
+tslack_cgroup_create(struct cgroup_subsys *subsys, struct cgroup *cgroup)
+{
+	struct timer_slack_cgroup *tslack_cgroup;
+	struct cgroup *parent = cgroup->parent;
+
+	tslack_cgroup = kmalloc(sizeof(*tslack_cgroup), GFP_KERNEL);
+	if (!tslack_cgroup)
+		return ERR_PTR(-ENOMEM);
+
+	if (parent)
+		tslack_cgroup->timer_slack_ns =
+			cgroup_to_tslack_cgroup(parent)->timer_slack_ns;
+	else
+		tslack_cgroup->timer_slack_ns = TIMER_SLACK_NS_DEFAULT;
+
+	return &tslack_cgroup->css;
+}
+
+static void tslack_cgroup_destroy(struct cgroup_subsys *subsys,
+		struct cgroup *cgroup)
+{
+	kfree(cgroup_to_tslack_cgroup(cgroup));
+}
+
+static void tslack_cgroup_attach(struct cgroup_subsys *subsys,
+		struct cgroup *cgroup, struct cgroup *prev,
+		struct task_struct *tsk, bool threadgroup)
+{
+	struct timer_slack_cgroup *tslack_cgroup;
+
+	tslack_cgroup = cgroup_to_tslack_cgroup(cgroup);
+	task_lock(tsk);
+	tsk->timer_slack_ns = tslack_cgroup->timer_slack_ns;
+	task_unlock(tsk);
+}
+
+static u64 tslack_read_slack_ns(struct cgroup *cgroup, struct cftype *cft)
+{
+       return cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns;
+}
+
+static int tslack_write_slack_ns(struct cgroup *cgroup, struct cftype *cft,
+		u64 val)
+{
+	struct cgroup_iter it;
+	struct task_struct *task;
+
+	if (!val)
+		return -EINVAL;
+
+	cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns = val;
+
+	/* change timer slack value for all tasks in the cgroup */
+	cgroup_iter_start(cgroup, &it);
+	while ((task = cgroup_iter_next(cgroup, &it)))
+		task->timer_slack_ns = val;
+	cgroup_iter_end(cgroup, &it);
+
+	return 0;
+}
+
+static struct cftype cft_timer_slack = {
+	.name = "slack_ns",
+	.read_u64 = tslack_read_slack_ns,
+	.write_u64 = tslack_write_slack_ns,
+};
+
+static int tslack_cgroup_populate(struct cgroup_subsys *subsys,
+		struct cgroup *cgroup)
+{
+	return cgroup_add_file(cgroup, subsys, &cft_timer_slack);
+}
+
+struct cgroup_subsys timer_slack_subsys = {
+	.name = "timer_slack",
+	.module = THIS_MODULE,
+	.subsys_id = timer_slack_subsys_id,
+	.create = tslack_cgroup_create,
+	.destroy = tslack_cgroup_destroy,
+	.attach = tslack_cgroup_attach,
+	.populate = tslack_cgroup_populate,
+};
+
+static int __init init_cgroup_timer_slack(void)
+{
+	return cgroup_load_subsys(&timer_slack_subsys);
+}
+
+static void __exit exit_cgroup_timer_slack(void)
+{
+	cgroup_unload_subsys(&timer_slack_subsys);
+}
+
+module_init(init_cgroup_timer_slack);
+module_exit(exit_cgroup_timer_slack);
+MODULE_LICENSE("GPL");
-- 
1.7.3.5

Re: [PATCH, v2 2/2] cgroups: introduce timer slack subsystem

[Matt Helsley]

On Tue, Feb 01, 2011 at 12:13:16PM +0200, Kirill A. Shutsemov wrote:
> From: Kirill A. Shutemov <kirill@shutemov.name>
> 
> Provides a way of tasks grouping by timer slack value. Introduces per
> cgroup timer slack value which will override the default timer slack
> value once a task is attached to a cgroup.
> 
> It's useful in mobile devices where certain background apps are
> attached to a cgroup and minimum wakeups are desired.
> 
> Based on patch by Jacob Pan.
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
> 
> Changelog:
> v2:
>  - fixed with CONFIG_CGROUP_TIMER_SLACK=y
> 
> ---
>  include/linux/cgroup_subsys.h |    6 ++
>  include/linux/init_task.h     |    4 +-
>  init/Kconfig                  |   10 ++++
>  kernel/Makefile               |    1 +
>  kernel/cgroup_timer_slack.c   |  116 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 136 insertions(+), 1 deletions(-)
>  create mode 100644 kernel/cgroup_timer_slack.c
> 
> 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/init_task.h b/include/linux/init_task.h
> index caa151f..48eca8f 100644
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -124,6 +124,8 @@ extern struct cred init_cred;
>  # define INIT_PERF_EVENTS(tsk)
>  #endif
> 
> +#define TIMER_SLACK_NS_DEFAULT 50000
> +
>  /*
>   *  INIT_TASK is used to set up the first task table, touch at
>   * your own risk!. Base=0, limit=0x1fffff (=2MB)
> @@ -177,7 +179,7 @@ extern struct cred init_cred;
>  	.cpu_timers	= INIT_CPU_TIMERS(tsk.cpu_timers),		\
>  	.fs_excl	= ATOMIC_INIT(0),				\
>  	.pi_lock	= __RAW_SPIN_LOCK_UNLOCKED(tsk.pi_lock),	\
> -	.timer_slack_ns = 50000, /* 50 usec default slack */		\
> +	.timer_slack_ns = TIMER_SLACK_NS_DEFAULT,			\
>  	.pids = {							\
>  		[PIDTYPE_PID]  = INIT_PID_LINK(PIDTYPE_PID),		\
>  		[PIDTYPE_PGID] = INIT_PID_LINK(PIDTYPE_PGID),		\
> diff --git a/init/Kconfig b/init/Kconfig
> index be788c0..f21b4ce 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -596,6 +596,16 @@ config CGROUP_FREEZER
>  	  Provides a way to freeze and unfreeze all tasks in a
>  	  cgroup.
> 
> +config CGROUP_TIMER_SLACK
> +	tristate "Timer slack cgroup subsystem"
> +	help
> +	  Provides a way of tasks grouping by timer slack value.
> +	  Introduces per cgroup timer slack value which will override
> +	  the default timer slack value once a task is attached to a
> +	  cgroup.
> +	  It's useful in mobile devices where certain background apps
> +	  are attached to a cgroup and minimum wakeups are desired.

nit: perhaps s/minimum/combined/ ?

> +
>  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..daa452d
> --- /dev/null
> +++ b/kernel/cgroup_timer_slack.c
> @@ -0,0 +1,116 @@
> +#include <linux/cgroup.h>
> +#include <linux/init_task.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +struct cgroup_subsys timer_slack_subsys;
> +struct timer_slack_cgroup {
> +	struct cgroup_subsys_state css;
> +	unsigned long timer_slack_ns;

rename to min_timer_slack_ns ?

> +};
> +
> +static struct timer_slack_cgroup *cgroup_to_tslack_cgroup(struct cgroup *cgroup)
> +{
> +	struct cgroup_subsys_state *css;
> +
> +	css = cgroup_subsys_state(cgroup, timer_slack_subsys.subsys_id);
> +	return container_of(css, struct timer_slack_cgroup, css);
> +}
> +
> +static struct cgroup_subsys_state *
> +tslack_cgroup_create(struct cgroup_subsys *subsys, struct cgroup *cgroup)
> +{
> +	struct timer_slack_cgroup *tslack_cgroup;
> +	struct cgroup *parent = cgroup->parent;
> +
> +	tslack_cgroup = kmalloc(sizeof(*tslack_cgroup), GFP_KERNEL);
> +	if (!tslack_cgroup)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (parent)
> +		tslack_cgroup->timer_slack_ns =
> +			cgroup_to_tslack_cgroup(parent)->timer_slack_ns;
> +	else
> +		tslack_cgroup->timer_slack_ns = TIMER_SLACK_NS_DEFAULT;
> +
> +	return &tslack_cgroup->css;
> +}
> +
> +static void tslack_cgroup_destroy(struct cgroup_subsys *subsys,
> +		struct cgroup *cgroup)
> +{
> +	kfree(cgroup_to_tslack_cgroup(cgroup));
> +}
> +
> +static void tslack_cgroup_attach(struct cgroup_subsys *subsys,
> +		struct cgroup *cgroup, struct cgroup *prev,
> +		struct task_struct *tsk, bool threadgroup)
> +{
> +	struct timer_slack_cgroup *tslack_cgroup;
> +
> +	tslack_cgroup = cgroup_to_tslack_cgroup(cgroup);
> +	task_lock(tsk);
> +	tsk->timer_slack_ns = tslack_cgroup->timer_slack_ns;
> +	task_unlock(tsk);
> +}
> +
> +static u64 tslack_read_slack_ns(struct cgroup *cgroup, struct cftype *cft)
> +{
> +       return cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns;
> +}
> +
> +static int tslack_write_slack_ns(struct cgroup *cgroup, struct cftype *cft,
> +		u64 val)
> +{
> +	struct cgroup_iter it;
> +	struct task_struct *task;
> +
> +	if (!val)
> +		return -EINVAL;
> +
> +	cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns = val;
> +
> +	/* change timer slack value for all tasks in the cgroup */
> +	cgroup_iter_start(cgroup, &it);
> +	while ((task = cgroup_iter_next(cgroup, &it)))
> +		task->timer_slack_ns = val;

If a task in the cgroup has a larger timer slack than timer_slack_ns
is it really necessary to change it? If not, you could do:

		task->timer_slack_ns = max(task->timer_slack_ns, val);

and likely get fewer wakeups than otherwise.

> +	cgroup_iter_end(cgroup, &it);

So this is largely just an interface to write timer slack from outside
a task, "in bulk", and only restrict what can be done via the prctl syscall.
It does not enforce restrictions on timer slack set via the cgroups
interface. If I have:

	root@host:/# mount -t cgroup -o slack_ns none /parent
	root@host:/# echo 100000 > /parent/timer_slack
	root@host:/# mkdir /parent/child
	root@host:/# chown me.me -R /parent/child

Then user "me" can set any timer slack desired rather than only timer
slack >= 100000ns:

	me@host:~$ echo $$ > /parent/child/tasks
	me@host:~$ echo 42 > /parent/child/timer_slack

Thus the only tasks in this cgroup with restricted timer slack are the
ones unaware of the "modern" interface.

Would it be more useful (though way more complex cgroup code I suppose) to
disallow child cgroups from using less timer slack than their parents?

> +
> +	return 0;
> +}
> +
> +static struct cftype cft_timer_slack = {
> +	.name = "slack_ns",

A minor nit:

Interesting that you appended _ns to the subsystem name rather than...

> +	.read_u64 = tslack_read_slack_ns,
> +	.write_u64 = tslack_write_slack_ns,
> +};
> +
> +static int tslack_cgroup_populate(struct cgroup_subsys *subsys,
> +		struct cgroup *cgroup)
> +{
> +	return cgroup_add_file(cgroup, subsys, &cft_timer_slack);
> +}
> +
> +struct cgroup_subsys timer_slack_subsys = {
> +	.name = "timer_slack",

...to the file name. That's clever -- I almost didn't notice :). Yet I
can't help but think that appending "_ns" to the timer_slack file
itself would make proper use of the interface more obvious and
thus improper use less likely.

Cheers,
	-Matt Helsley

Re: [PATCH, v2 2/2] cgroups: introduce timer slack subsystem

[Matt Helsley]

On Tue, Feb 01, 2011 at 12:13:16PM +0200, Kirill A. Shutsemov wrote:
> From: Kirill A. Shutemov <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>
> 
> Provides a way of tasks grouping by timer slack value. Introduces per
> cgroup timer slack value which will override the default timer slack
> value once a task is attached to a cgroup.
> 
> It's useful in mobile devices where certain background apps are
> attached to a cgroup and minimum wakeups are desired.
> 
> Based on patch by Jacob Pan.
> 
> Signed-off-by: Kirill A. Shutemov <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>
> ---
> 
> Changelog:
> v2:
>  - fixed with CONFIG_CGROUP_TIMER_SLACK=y
> 
> ---
>  include/linux/cgroup_subsys.h |    6 ++
>  include/linux/init_task.h     |    4 +-
>  init/Kconfig                  |   10 ++++
>  kernel/Makefile               |    1 +
>  kernel/cgroup_timer_slack.c   |  116 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 136 insertions(+), 1 deletions(-)
>  create mode 100644 kernel/cgroup_timer_slack.c
> 
> 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/init_task.h b/include/linux/init_task.h
> index caa151f..48eca8f 100644
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -124,6 +124,8 @@ extern struct cred init_cred;
>  # define INIT_PERF_EVENTS(tsk)
>  #endif
> 
> +#define TIMER_SLACK_NS_DEFAULT 50000
> +
>  /*
>   *  INIT_TASK is used to set up the first task table, touch at
>   * your own risk!. Base=0, limit=0x1fffff (=2MB)
> @@ -177,7 +179,7 @@ extern struct cred init_cred;
>  	.cpu_timers	= INIT_CPU_TIMERS(tsk.cpu_timers),		\
>  	.fs_excl	= ATOMIC_INIT(0),				\
>  	.pi_lock	= __RAW_SPIN_LOCK_UNLOCKED(tsk.pi_lock),	\
> -	.timer_slack_ns = 50000, /* 50 usec default slack */		\
> +	.timer_slack_ns = TIMER_SLACK_NS_DEFAULT,			\
>  	.pids = {							\
>  		[PIDTYPE_PID]  = INIT_PID_LINK(PIDTYPE_PID),		\
>  		[PIDTYPE_PGID] = INIT_PID_LINK(PIDTYPE_PGID),		\
> diff --git a/init/Kconfig b/init/Kconfig
> index be788c0..f21b4ce 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -596,6 +596,16 @@ config CGROUP_FREEZER
>  	  Provides a way to freeze and unfreeze all tasks in a
>  	  cgroup.
> 
> +config CGROUP_TIMER_SLACK
> +	tristate "Timer slack cgroup subsystem"
> +	help
> +	  Provides a way of tasks grouping by timer slack value.
> +	  Introduces per cgroup timer slack value which will override
> +	  the default timer slack value once a task is attached to a
> +	  cgroup.
> +	  It's useful in mobile devices where certain background apps
> +	  are attached to a cgroup and minimum wakeups are desired.

nit: perhaps s/minimum/combined/ ?

> +
>  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..daa452d
> --- /dev/null
> +++ b/kernel/cgroup_timer_slack.c
> @@ -0,0 +1,116 @@
> +#include <linux/cgroup.h>
> +#include <linux/init_task.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +struct cgroup_subsys timer_slack_subsys;
> +struct timer_slack_cgroup {
> +	struct cgroup_subsys_state css;
> +	unsigned long timer_slack_ns;

rename to min_timer_slack_ns ?

> +};
> +
> +static struct timer_slack_cgroup *cgroup_to_tslack_cgroup(struct cgroup *cgroup)
> +{
> +	struct cgroup_subsys_state *css;
> +
> +	css = cgroup_subsys_state(cgroup, timer_slack_subsys.subsys_id);
> +	return container_of(css, struct timer_slack_cgroup, css);
> +}
> +
> +static struct cgroup_subsys_state *
> +tslack_cgroup_create(struct cgroup_subsys *subsys, struct cgroup *cgroup)
> +{
> +	struct timer_slack_cgroup *tslack_cgroup;
> +	struct cgroup *parent = cgroup->parent;
> +
> +	tslack_cgroup = kmalloc(sizeof(*tslack_cgroup), GFP_KERNEL);
> +	if (!tslack_cgroup)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (parent)
> +		tslack_cgroup->timer_slack_ns =
> +			cgroup_to_tslack_cgroup(parent)->timer_slack_ns;
> +	else
> +		tslack_cgroup->timer_slack_ns = TIMER_SLACK_NS_DEFAULT;
> +
> +	return &tslack_cgroup->css;
> +}
> +
> +static void tslack_cgroup_destroy(struct cgroup_subsys *subsys,
> +		struct cgroup *cgroup)
> +{
> +	kfree(cgroup_to_tslack_cgroup(cgroup));
> +}
> +
> +static void tslack_cgroup_attach(struct cgroup_subsys *subsys,
> +		struct cgroup *cgroup, struct cgroup *prev,
> +		struct task_struct *tsk, bool threadgroup)
> +{
> +	struct timer_slack_cgroup *tslack_cgroup;
> +
> +	tslack_cgroup = cgroup_to_tslack_cgroup(cgroup);
> +	task_lock(tsk);
> +	tsk->timer_slack_ns = tslack_cgroup->timer_slack_ns;
> +	task_unlock(tsk);
> +}
> +
> +static u64 tslack_read_slack_ns(struct cgroup *cgroup, struct cftype *cft)
> +{
> +       return cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns;
> +}
> +
> +static int tslack_write_slack_ns(struct cgroup *cgroup, struct cftype *cft,
> +		u64 val)
> +{
> +	struct cgroup_iter it;
> +	struct task_struct *task;
> +
> +	if (!val)
> +		return -EINVAL;
> +
> +	cgroup_to_tslack_cgroup(cgroup)->timer_slack_ns = val;
> +
> +	/* change timer slack value for all tasks in the cgroup */
> +	cgroup_iter_start(cgroup, &it);
> +	while ((task = cgroup_iter_next(cgroup, &it)))
> +		task->timer_slack_ns = val;

If a task in the cgroup has a larger timer slack than timer_slack_ns
is it really necessary to change it? If not, you could do:

		task->timer_slack_ns = max(task->timer_slack_ns, val);

and likely get fewer wakeups than otherwise.

> +	cgroup_iter_end(cgroup, &it);

So this is largely just an interface to write timer slack from outside
a task, "in bulk", and only restrict what can be done via the prctl syscall.
It does not enforce restrictions on timer slack set via the cgroups
interface. If I have:

	root@host:/# mount -t cgroup -o slack_ns none /parent
	root@host:/# echo 100000 > /parent/timer_slack
	root@host:/# mkdir /parent/child
	root@host:/# chown me.me -R /parent/child

Then user "me" can set any timer slack desired rather than only timer
slack >= 100000ns:

	me@host:~$ echo $$ > /parent/child/tasks
	me@host:~$ echo 42 > /parent/child/timer_slack

Thus the only tasks in this cgroup with restricted timer slack are the
ones unaware of the "modern" interface.

Would it be more useful (though way more complex cgroup code I suppose) to
disallow child cgroups from using less timer slack than their parents?

> +
> +	return 0;
> +}
> +
> +static struct cftype cft_timer_slack = {
> +	.name = "slack_ns",

A minor nit:

Interesting that you appended _ns to the subsystem name rather than...

> +	.read_u64 = tslack_read_slack_ns,
> +	.write_u64 = tslack_write_slack_ns,
> +};
> +
> +static int tslack_cgroup_populate(struct cgroup_subsys *subsys,
> +		struct cgroup *cgroup)
> +{
> +	return cgroup_add_file(cgroup, subsys, &cft_timer_slack);
> +}
> +
> +struct cgroup_subsys timer_slack_subsys = {
> +	.name = "timer_slack",

...to the file name. That's clever -- I almost didn't notice :). Yet I
can't help but think that appending "_ns" to the timer_slack file
itself would make proper use of the interface more obvious and
thus improper use less likely.

Cheers,
	-Matt Helsley