Hook up group scheduler with control groups

PATCH In review Srivatsa Vaddagiri 4 participants 7 messages Sep 27, 2007 – Sep 28, 2007 cgroup

lore

Share & tools

lore

Srivatsa Vaddagiri

author

2007-09-27 14:55 UTC #0 lore raw

PATCH 1/1 Hook up group scheduler with control groups

Andrew,
This is a resend of the patch I had sent earlier at:

http://marc.info/?l=linux-kernel&m=119065506607858

This patch enables group cpu scheduler feature to work with control groups.

Could you include this in -mm for folks to test it?

Enable "cgroup" (formerly containers) based fair group scheduling. This will let administrator create arbitrary groups of tasks (using "cgroup" pseudo filesystem) and control their cpu bandwidth usage.

Signed-off-by Srivatsa Vaddagiri <[email protected]>

Signed-off-by Dhaval Giani <[email protected]>

---
 include/linux/cgroup_subsys.h |    6 ++
 init/Kconfig                  |   24 +++++---
 kernel/sched.c                |  122 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 7 deletions(-)

include/linux/cgroup_subsys.h +6 -0

Index: current/include/linux/cgroup_subsys.h
===================================================================
@@ -36,3 +36,9 @@ SUBSYS(mem_cgroup)
 #endif
 
 /* */
+
+#ifdef CONFIG_FAIR_CGROUP_SCHED
+SUBSYS(cpu_cgroup)
+#endif
+
+/* */

init/Kconfig +17 -7

Index: current/init/Kconfig
===================================================================
@@ -319,6 +319,13 @@ config CPUSETS
 
 	  Say N if unsure.
 
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
+          infrastructure that works with cgroups
+	depends on CGROUPS
+
 config FAIR_GROUP_SCHED
 	bool "Fair group CPU scheduler"
 	default y

@@ -338,14 +345,17 @@ config FAIR_USER_SCHED
 	  This option will choose userid as the basis for grouping
 	  tasks, thus providing equal CPU bandwidth to each user.
 
-endchoice
+config FAIR_CGROUP_SCHED
+	bool "Control groups"
+ 	depends on CGROUPS
+ 	help
+	  This option allows you to create arbitrary task groups
+	  using the "cgroup" pseudo filesystem and control
+	  the cpu bandwidth allocated to each such task group.
+	  Refer to Documentation/cgroups.txt for more information
+	  on "cgroup" pseudo filesystem.
 
-config RESOURCE_COUNTERS
-	bool "Resource counters"
-	help
-	  This option enables controller independent resource accounting
-          infrastructure that works with cgroups
-	depends on CGROUPS
+endchoice
 
 config SYSFS_DEPRECATED
 	bool "Create deprecated sysfs files"

kernel/sched.c +122 -0

141 lines not on this page · raw message

Signature · 2 lines

Regards,
vatsa

Randy Dunlap

2007-09-27 17:40 UTC #1 lore raw

Quoted 9 lines · On Thu, 27 Sep 2007 20:36:24 +0530 Srivatsa Vaddagiri wrote:

Andrew,
This is a resend of the patch I had sent earlier at:
http://marc.info/?l=linux-kernel&m=119065506607858
This patch enables group cpu scheduler feature to work with control groups.
Could you include this in -mm for folks to test it?

Hi :)

Uh, a few of my previous comments weren't fixed... (below)

Quoted 21 lines

--
Enable "cgroup" (formerly containers) based fair group scheduling. This will let administrator create arbitrary groups of tasks (using "cgroup" pseudo filesystem) and control their cpu bandwidth usage.
Signed-off-by Srivatsa Vaddagiri <[email protected]>
Signed-off-by Dhaval Giani <[email protected]>
---
 include/linux/cgroup_subsys.h |    6 ++
 init/Kconfig                  |   24 +++++---
 kernel/sched.c                |  122 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 7 deletions(-)
Index: current/init/Kconfig
===================================================================

init/Kconfig config CPUSETS · line 326

Quoted 8 lines

init/Kconfig +5 -0

@@ -319,6 +319,13 @@ config CPUSETS
 
 	  Say N if unsure.
 
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
+          infrastructure that works with cgroups

Use tab + 2 spaces consistently for help text indentation. End that sentence with a ".".

init/Kconfig config CPUSETS · line 331

Quoted 5 lines

init/Kconfig +2 -0

+	depends on CGROUPS
+
 config FAIR_GROUP_SCHED
 	bool "Fair group CPU scheduler"
 	default y

---
~Randy
Phaedrus says that Quality is about caring.

Dhaval Giani

2007-09-27 18:04 UTC #2 lore raw

Quoted 30 lines · Randy Dunlap

On Thu, 27 Sep 2007 20:36:24 +0530 Srivatsa Vaddagiri wrote:
Hi :)
Uh, a few of my previous comments weren't fixed... (below)
Quoted 24 lines
--
Enable "cgroup" (formerly containers) based fair group scheduling. This will let administrator create arbitrary groups of tasks (using "cgroup" pseudo filesystem) and control their cpu bandwidth usage.
Signed-off-by Srivatsa Vaddagiri <[email protected]>
Signed-off-by Dhaval Giani <[email protected]>
---
 include/linux/cgroup_subsys.h |    6 ++
 init/Kconfig                  |   24 +++++---
 kernel/sched.c                |  122 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 7 deletions(-)
Index: current/init/Kconfig
===================================================================
init/Kconfig +0 -0
@@ -319,6 +319,13 @@ config CPUSETS
 
 	  Say N if unsure.
 

Quoted 9 lines

init/Kconfig config CPUSETS · line 326
Quoted 5 lines
init/Kconfig +5 -0
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
+          infrastructure that works with cgroups
Use tab + 2 spaces consistently for help text indentation. End that sentence with a ".".

Hi,

hmmmm. So I looked into the sources with the patches applied, and they don't have an indentation problem. It looks fine. I'm not sure why the patch is getting generated like this though. Will fix up the '.' though.

Here it is,

Thanks,
--

Signed-off-by Srivatsa Vaddagiri <[email protected]>

Signed-off-by Dhaval Giani <[email protected]>

---
 include/linux/cgroup_subsys.h |    6 ++
 init/Kconfig                  |   24 +++++---
 kernel/sched.c                |  122 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 7 deletions(-)

include/linux/cgroup_subsys.h +6 -0

Index: current/include/linux/cgroup_subsys.h
===================================================================
@@ -36,3 +36,9 @@ SUBSYS(mem_cgroup)
 #endif
 
 /* */
+
+#ifdef CONFIG_FAIR_CGROUP_SCHED
+SUBSYS(cpu_cgroup)
+#endif
+
+/* */

init/Kconfig +17 -7

Index: current/init/Kconfig
===================================================================
@@ -319,6 +319,13 @@ config CPUSETS
 
 	  Say N if unsure.
 
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
+          infrastructure that works with cgroups.
+	depends on CGROUPS
+
 config FAIR_GROUP_SCHED
 	bool "Fair group CPU scheduler"
 	default y

@@ -338,14 +345,17 @@ config FAIR_USER_SCHED
 	  This option will choose userid as the basis for grouping
 	  tasks, thus providing equal CPU bandwidth to each user.
 
-endchoice
+config FAIR_CGROUP_SCHED
+	bool "Control groups"
+ 	depends on CGROUPS
+ 	help
+	  This option allows you to create arbitrary task groups
+	  using the "cgroup" pseudo filesystem and control
+	  the cpu bandwidth allocated to each such task group.
+	  Refer to Documentation/cgroups.txt for more information
+	  on "cgroup" pseudo filesystem.
 
-config RESOURCE_COUNTERS
-	bool "Resource counters"
-	help
-	  This option enables controller independent resource accounting
-          infrastructure that works with cgroups
-	depends on CGROUPS
+endchoice
 
 config SYSFS_DEPRECATED
 	bool "Create deprecated sysfs files"

kernel/sched.c +122 -0

141 lines not on this page · raw message

Signature · 2 lines

regards,
Dhaval

Randy Dunlap

2007-09-27 19:00 UTC #3 lore raw

Quoted 49 lines · On Thu, 27 Sep 2007 23:34:15 +0530 Dhaval Giani wrote:

Quoted 9 lines
init/Kconfig config CPUSETS
Quoted 5 lines
init/Kconfig +5 -0
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
+          infrastructure that works with cgroups
Use tab + 2 spaces consistently for help text indentation. End that sentence with a ".".
Hi,
hmmmm. So I looked into the sources with the patches applied, and they don't have an indentation problem. It looks fine. I'm not sure why the patch is getting generated like this though. Will fix up the '.' though.
Here it is,
Thanks,
--
Signed-off-by Srivatsa Vaddagiri <[email protected]>
Signed-off-by Dhaval Giani <[email protected]>
---
 include/linux/cgroup_subsys.h |    6 ++
 init/Kconfig                  |   24 +++++---
 kernel/sched.c                |  122 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 7 deletions(-)
Index: current/include/linux/cgroup_subsys.h
===================================================================
include/linux/cgroup_subsys.h +6 -0
@@ -36,3 +36,9 @@ SUBSYS(mem_cgroup)
 #endif
 
 /* */
+
+#ifdef CONFIG_FAIR_CGROUP_SCHED
+SUBSYS(cpu_cgroup)
+#endif
+
+/* */
Index: current/init/Kconfig
===================================================================

init/Kconfig config CPUSETS · line 325

Quoted 7 lines

init/Kconfig +4 -0

@@ -319,6 +319,13 @@ config CPUSETS
 
 	  Say N if unsure.
 
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting

Above line is tab + 2 spaces (i.e., correct).

init/Kconfig config CPUSETS · line 326

Quoted 1 line

init/Kconfig +1 -0

+          infrastructure that works with cgroups.

Above line indent is 10 spaces (i.e., not correct).

init/Kconfig config CPUSETS · line 331

Quoted 5 lines

init/Kconfig +2 -0

+	depends on CGROUPS
+
 config FAIR_GROUP_SCHED
 	bool "Fair group CPU scheduler"
 	default y

---
~Randy
Phaedrus says that Quality is about caring.

Dhaval Giani

2007-09-27 19:35 UTC #4 lore raw

Quoted 10 lines · Randy Dunlap

Review
Quoted 6 lines · On Thu, 27 Sep 2007 23:34:15 +0530 Dhaval Giani wrote:
(quoted diff) +4 -0
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
Above line is tab + 2 spaces (i.e., correct).

Quoted 4 lines

Quoted 1 line
+ infrastructure that works with cgroups.
Above line indent is 10 spaces (i.e., not correct).

Ah! Thanks for the explanation. Corrected patch follows.

Signed-off-by Srivatsa Vaddagiri <[email protected]>

Signed-off-by Dhaval Giani <[email protected]>

---
 include/linux/cgroup_subsys.h |    6 ++
 init/Kconfig                  |   24 +++++---
 kernel/sched.c                |  122 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+), 7 deletions(-)

include/linux/cgroup_subsys.h +6 -0

Index: current/include/linux/cgroup_subsys.h
===================================================================
@@ -36,3 +36,9 @@ SUBSYS(mem_cgroup)
 #endif
 
 /* */
+
+#ifdef CONFIG_FAIR_CGROUP_SCHED
+SUBSYS(cpu_cgroup)
+#endif
+
+/* */

init/Kconfig +17 -7

Index: current/init/Kconfig
===================================================================
@@ -319,6 +319,13 @@ config CPUSETS
 
 	  Say N if unsure.
 
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
+	  infrastructure that works with cgroups.
+	depends on CGROUPS
+
 config FAIR_GROUP_SCHED
 	bool "Fair group CPU scheduler"
 	default y

@@ -338,14 +345,17 @@ config FAIR_USER_SCHED
 	  This option will choose userid as the basis for grouping
 	  tasks, thus providing equal CPU bandwidth to each user.
 
-endchoice
+config FAIR_CGROUP_SCHED
+	bool "Control groups"
+ 	depends on CGROUPS
+ 	help
+	  This option allows you to create arbitrary task groups
+	  using the "cgroup" pseudo filesystem and control
+	  the cpu bandwidth allocated to each such task group.
+	  Refer to Documentation/cgroups.txt for more information
+	  on "cgroup" pseudo filesystem.
 
-config RESOURCE_COUNTERS
-	bool "Resource counters"
-	help
-	  This option enables controller independent resource accounting
-          infrastructure that works with cgroups
-	depends on CGROUPS
+endchoice
 
 config SYSFS_DEPRECATED
 	bool "Create deprecated sysfs files"

kernel/sched.c +122 -0

141 lines not on this page · raw message

Signature · 2 lines

regards,
Dhaval

Andrew Morton

2007-09-27 23:42 UTC #5 lore raw

Quoted 25 lines · On Fri, 28 Sep 2007 01:05:12 +0530 Dhaval Giani <[email protected]>…

Quoted 10 lines · Randy Dunlap
init/Kconfig config CPUSETS · line 325
Quoted 6 lines · On Thu, 27 Sep 2007 23:34:15 +0530 Dhaval Giani wrote:
init/Kconfig +4 -0
+config RESOURCE_COUNTERS
+	bool "Resource counters"
+	help
+	  This option enables controller independent resource accounting
Above line is tab + 2 spaces (i.e., correct).
Quoted 4 lines
Quoted 1 line
+ infrastructure that works with cgroups.
Above line indent is 10 spaces (i.e., not correct).
Ah! Thanks for the explanation. Corrected patch follows.
Signed-off-by Srivatsa Vaddagiri <[email protected]>
Signed-off-by Dhaval Giani <[email protected]>
...

kernel/sched.c static inline struct task_grp *task_grp( · line 233

Quoted 9 lines

kernel/sched.c +3 -0

@@ -219,6 +225,9 @@ static inline struct task_grp *task_grp(
 
 #ifdef CONFIG_FAIR_USER_SCHED
 	tg = p->user->tg;
+#elif CONFIG_FAIR_CGROUP_SCHED
+	tg = container_of(task_subsys_state(p, cpu_cgroup_subsys_id),
+				struct task_grp, css);
 #else
 	tg  = &init_task_grp;
 #endif

that's a bit funny-looking. Are CONFIG_FAIR_CGROUP_SCHED and CONFIG_FAIR_USER_SCHED mutually exclusive? Doesn't seem that way. if they're both defined then CONFIG_FAIR_USER_SCHED "wins".

Anyway, please confirm that this is correct?

I'll switch that to #elif defined(CONFIG_FAIR_CGROUP_SCHED)'. We can get gcc warnings with #if CONFIG_FOO', and people should be using `#ifdef CONFIG_FOO', so I assume the same applies to #elif.

Srivatsa Vaddagiri

author

2007-09-28 02:29 UTC #6 lore raw

Quoted 13 lines · Andrew Morton

kernel/sched.c static inline struct task_grp *task_grp( · line 233
Quoted 10 lines
kernel/sched.c +3 -0
@@ -219,6 +225,9 @@ static inline struct task_grp *task_grp(
 
 #ifdef CONFIG_FAIR_USER_SCHED
 	tg = p->user->tg;
+#elif CONFIG_FAIR_CGROUP_SCHED
+	tg = container_of(task_subsys_state(p, cpu_cgroup_subsys_id),
+				struct task_grp, css);
 #else
 	tg  = &init_task_grp;
 #endif
that's a bit funny-looking. Are CONFIG_FAIR_CGROUP_SCHED and CONFIG_FAIR_USER_SCHED mutually exclusive?

Yes. While configuring kernel, user can choose only one of those options and not both.

Quoted 1 line

Doesn't seem that way.

Hmm ..why do you say that?

Quoted 3 lines

if
they're both defined then CONFIG_FAIR_USER_SCHED "wins". Anyway, please confirm that this is correct?

They can't both be defined.

Quoted 3 lines

I'll switch that to #elif defined(CONFIG_FAIR_CGROUP_SCHED)'. We can get gcc warnings with #if CONFIG_FOO', and people should be using `#ifdef CONFIG_FOO', so I assume the same applies to #elif.

Thx for fixing it!

Signature · 2 lines

Regards,
vatsa