[PATCH] Add cgroup support for enabling controllers at boot time (v2)

[PATCH] Add cgroup support for enabling controllers at boot time (v2)

[Balbir Singh]

From: Paul Menage <menage@google.com>

Changelog v2

1. Add enabled support in /proc/cgroups
2. Add support for parsing of several controller names


The effects of cgroup_disable=foo are:

- foo isn't auto-mounted if you mount all cgroups in a single hierarchy
- foo isn't visible as an individually mountable subsystem

As a result there will only ever be one call to foo->create(), at init
time; all processes will stay in this group, and the group will never
be mounted on a visible hierarchy. Any additional effects (e.g. not
allocating metadata) are up to the foo subsystem.

This doesn't handle early_init subsystems (their "disabled" bit isn't
set be, but it could easily be extended to do so if any of the early_init
systems wanted it - I think it would just involve some nastier parameter
processing since it would occur before the command-line argument parser
had been run.

Signed-off-by: Paul Menage <menage@google.com>
Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---

 Documentation/kernel-parameters.txt |    4 +++
 include/linux/cgroup.h              |    1 
 kernel/cgroup.c                     |   45 ++++++++++++++++++++++++++++++++----
 3 files changed, 45 insertions(+), 5 deletions(-)

diff -puN include/linux/cgroup.h~cgroup_disable include/linux/cgroup.h
--- linux-2.6.25-rc4/include/linux/cgroup.h~cgroup_disable	2008-03-07 14:26:16.000000000 +0530
+++ linux-2.6.25-rc4-balbir/include/linux/cgroup.h	2008-03-07 14:26:16.000000000 +0530
@@ -256,6 +256,7 @@ struct cgroup_subsys {
 	void (*bind)(struct cgroup_subsys *ss, struct cgroup *root);
 	int subsys_id;
 	int active;
+	int disabled;
 	int early_init;
 #define MAX_CGROUP_TYPE_NAMELEN 32
 	const char *name;
diff -puN kernel/cgroup.c~cgroup_disable kernel/cgroup.c
--- linux-2.6.25-rc4/kernel/cgroup.c~cgroup_disable	2008-03-07 14:26:16.000000000 +0530
+++ linux-2.6.25-rc4-balbir/kernel/cgroup.c	2008-03-07 14:26:16.000000000 +0530
@@ -782,7 +782,14 @@ static int parse_cgroupfs_options(char *
 		if (!*token)
 			return -EINVAL;
 		if (!strcmp(token, "all")) {
-			opts->subsys_bits = (1 << CGROUP_SUBSYS_COUNT) - 1;
+			/* Add all non-disabled subsystems */
+			int i;
+			opts->subsys_bits = 0;
+			for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
+				struct cgroup_subsys *ss = subsys[i];
+				if (!ss->disabled)
+					opts->subsys_bits |= 1ul << i;
+			}
 		} else if (!strcmp(token, "noprefix")) {
 			set_bit(ROOT_NOPREFIX, &opts->flags);
 		} else if (!strncmp(token, "release_agent=", 14)) {
@@ -800,7 +807,8 @@ static int parse_cgroupfs_options(char *
 			for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
 				ss = subsys[i];
 				if (!strcmp(token, ss->name)) {
-					set_bit(i, &opts->subsys_bits);
+					if (!ss->disabled)
+						set_bit(i, &opts->subsys_bits);
 					break;
 				}
 			}
@@ -2600,13 +2608,13 @@ static int proc_cgroupstats_show(struct 
 {
 	int i;
 
-	seq_puts(m, "#subsys_name\thierarchy\tnum_cgroups\n");
+	seq_puts(m, "#subsys_name\thierarchy\tnum_cgroups\tenabled\n");
 	mutex_lock(&cgroup_mutex);
 	for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
 		struct cgroup_subsys *ss = subsys[i];
-		seq_printf(m, "%s\t%lu\t%d\n",
+		seq_printf(m, "%s\t%lu\t%d\t%d\n",
 			   ss->name, ss->root->subsys_bits,
-			   ss->root->number_of_cgroups);
+			   ss->root->number_of_cgroups, !ss->disabled);
 	}
 	mutex_unlock(&cgroup_mutex);
 	return 0;
@@ -3010,3 +3018,30 @@ static void cgroup_release_agent(struct 
 	spin_unlock(&release_list_lock);
 	mutex_unlock(&cgroup_mutex);
 }
+
+static int __init cgroup_disable(char *str)
+{
+	int i;
+
+	while (*str) {
+		for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
+			struct cgroup_subsys *ss = subsys[i];
+
+			if (!strncmp(str, ss->name, strlen(ss->name))) {
+				ss->disabled = 1;
+				printk(KERN_INFO "Disabling %s control group"
+					" subsystem\n", ss->name);
+				break;
+			}
+		}
+		/*
+		 * Find the next option if any
+		 */
+		while (*str && (*str != ','))
+			str++;
+		if (*str == ',')
+			str++;
+	}
+	return 1;
+}
+__setup("cgroup_disable=", cgroup_disable);
diff -puN Documentation/kernel-parameters.txt~cgroup_disable Documentation/kernel-parameters.txt
--- linux-2.6.25-rc4/Documentation/kernel-parameters.txt~cgroup_disable	2008-03-07 14:26:16.000000000 +0530
+++ linux-2.6.25-rc4-balbir/Documentation/kernel-parameters.txt	2008-03-07 14:26:16.000000000 +0530
@@ -383,6 +383,10 @@ and is between 256 and 4096 characters. 
 	ccw_timeout_log [S390]
 			See Documentation/s390/CommonIO for details.
 
+	cgroup_disable= [KNL] Disable a particular controller
+			Format: {name of the controller(s) to disable}
+				{Currently supported controllers - "memory"}
+
 	checkreqprot	[SELINUX] Set initial checkreqprot flag value.
 			Format: { "0" | "1" }
 			See security/selinux/Kconfig help text.
_

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

[PATCH] Make memory resource control aware of boot options (v2)

[Balbir Singh]

A boot option for the memory controller was discussed on lkml. It is a good
idea to add it, since it saves memory for people who want to turn off the
memory controller.

By default the option is on for the following two reasons

1. It provides compatibility with the current scheme where the memory
   controller turns on if the config option is enabled
2. It allows for wider testing of the memory controller, once the config
   option is enabled

We still allow the create, destroy callbacks to succeed, since they are
not aware of boot options. We do not populate the directory will
memory resource controller specific files.

Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---

 mm/memcontrol.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff -puN mm/memcontrol.c~memory-controller-add-boot-option mm/memcontrol.c
--- linux-2.6.25-rc4/mm/memcontrol.c~memory-controller-add-boot-option	2008-03-07 14:26:44.000000000 +0530
+++ linux-2.6.25-rc4-balbir/mm/memcontrol.c	2008-03-07 14:26:44.000000000 +0530
@@ -533,6 +533,9 @@ static int mem_cgroup_charge_common(stru
 	unsigned long nr_retries = MEM_CGROUP_RECLAIM_RETRIES;
 	struct mem_cgroup_per_zone *mz;
 
+	if (mem_cgroup_subsys.disabled)
+		return 0;
+
 	/*
 	 * Should page_cgroup's go to their own slab?
 	 * One could optimize the performance of the charging routine
@@ -665,6 +668,9 @@ void mem_cgroup_uncharge_page(struct pag
 	struct mem_cgroup_per_zone *mz;
 	unsigned long flags;
 
+	if (mem_cgroup_subsys.disabled)
+		return;
+
 	/*
 	 * Check if our page_cgroup is valid
 	 */
@@ -705,6 +711,9 @@ int mem_cgroup_prepare_migration(struct 
 {
 	struct page_cgroup *pc;
 
+	if (mem_cgroup_subsys.disabled)
+		return 0;
+
 	lock_page_cgroup(page);
 	pc = page_get_page_cgroup(page);
 	if (pc)
@@ -803,6 +812,9 @@ static int mem_cgroup_force_empty(struct
 	int ret = -EBUSY;
 	int node, zid;
 
+	if (mem_cgroup_subsys.disabled)
+		return 0;
+
 	css_get(&mem->css);
 	/*
 	 * page reclaim code (kswapd etc..) will move pages between
@@ -1053,6 +1065,8 @@ static void mem_cgroup_destroy(struct cg
 static int mem_cgroup_populate(struct cgroup_subsys *ss,
 				struct cgroup *cont)
 {
+	if (mem_cgroup_subsys.disabled)
+		return 0;
 	return cgroup_add_files(cont, ss, mem_cgroup_files,
 					ARRAY_SIZE(mem_cgroup_files));
 }
@@ -1065,6 +1079,9 @@ static void mem_cgroup_move_task(struct 
 	struct mm_struct *mm;
 	struct mem_cgroup *mem, *old_mem;
 
+	if (mem_cgroup_subsys.disabled)
+		return;
+
 	mm = get_task_mm(p);
 	if (mm == NULL)
 		return;
_

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

Re: [PATCH] Add cgroup support for enabling controllers at boot time (v2)

[David Rientjes]

On Fri, 7 Mar 2008, Balbir Singh wrote:

> diff -puN Documentation/kernel-parameters.txt~cgroup_disable Documentation/kernel-parameters.txt
> --- linux-2.6.25-rc4/Documentation/kernel-parameters.txt~cgroup_disable	2008-03-07 14:26:16.000000000 +0530
> +++ linux-2.6.25-rc4-balbir/Documentation/kernel-parameters.txt	2008-03-07 14:26:16.000000000 +0530
> @@ -383,6 +383,10 @@ and is between 256 and 4096 characters. 
>  	ccw_timeout_log [S390]
>  			See Documentation/s390/CommonIO for details.
>  
> +	cgroup_disable= [KNL] Disable a particular controller
> +			Format: {name of the controller(s) to disable}
> +				{Currently supported controllers - "memory"}
> +
>  	checkreqprot	[SELINUX] Set initial checkreqprot flag value.
>  			Format: { "0" | "1" }
>  			See security/selinux/Kconfig help text.

It would probably be helpful to mention in the documentation that the 
names of the subsystems must now be delimited by commas.

 [ I also find it very helpful to add randy.dunlap@oracle.com to the cc
   list for any patch touching Documentation ]

Re: [PATCH] Make memory resource control aware of boot options (v2)

[Andrew Morton]

On Fri, 07 Mar 2008 14:27:46 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> +	if (mem_cgroup_subsys.disabled)

My copy of `struct cgroup_subsys' doesn't have a .disabled?

If it did, I might want it __read_mostly.

Re: [PATCH] Make memory resource control aware of boot options (v2)

[Andrew Morton]

On Fri, 7 Mar 2008 01:06:49 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Fri, 07 Mar 2008 14:27:46 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> 
> > +	if (mem_cgroup_subsys.disabled)
> 
> My copy of `struct cgroup_subsys' doesn't have a .disabled?
> 

Ah.  You didn't sequence-number the patches, and they arrived out-of-order. 
tsk.

Re: [PATCH] Make memory resource control aware of boot options (v2)

[Balbir Singh]

Andrew Morton wrote:
> On Fri, 7 Mar 2008 01:06:49 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
>> On Fri, 07 Mar 2008 14:27:46 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>>
>>> +	if (mem_cgroup_subsys.disabled)
>> My copy of `struct cgroup_subsys' doesn't have a .disabled?
>>
> 
> Ah.  You didn't sequence-number the patches, and they arrived out-of-order. 
> tsk.
> 

Oops -- sorry about that.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

Re: [PATCH] Add cgroup support for enabling controllers at boot time (v2)

[Paul Menage]

On Fri, Mar 7, 2008 at 12:57 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>
>  This doesn't handle early_init subsystems (their "disabled" bit isn't
>  set be,

I think you meant something like

(their "disabled" bit isn't set before their initial "create" call is made)

>  +static int __init cgroup_disable(char *str)
>  +{
>  +       int i;
>  +
>  +       while (*str) {
>  +               for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
>  +                       struct cgroup_subsys *ss = subsys[i];
>  +
>  +                       if (!strncmp(str, ss->name, strlen(ss->name))) {
>  +                               ss->disabled = 1;
>  +                               printk(KERN_INFO "Disabling %s control group"
>  +                                       " subsystem\n", ss->name);
>  +                               break;

Doesn't this mean that cgroup_disable=cpu will disable whichever comes
first out of cpuset, cpuacct or cpu in the subsystem list?

I suggest just sticking with the original simpler version that
required separate cgroup_disabled=foo options for each system that you
want to disable.

Paul

Re: [PATCH] Add cgroup support for enabling controllers at boot time (v2)

[Paul Menage]

On Fri, Mar 7, 2008 at 1:25 AM, Paul Menage <menage@google.com> wrote:
>
>  Doesn't this mean that cgroup_disable=cpu will disable whichever comes
>  first out of cpuset, cpuacct or cpu in the subsystem list?

Or rather, it's the other way around - cgroup_disable=cpuset will
instead disable the "cpu" subsystem if "cpu" comes before "cpuset" in
the subsystem list.

Paul

Re: [PATCH] Add cgroup support for enabling controllers at boot time (v2)

[Balbir Singh]

Paul Menage wrote:
> On Fri, Mar 7, 2008 at 1:25 AM, Paul Menage <menage@google.com> wrote:
>>  Doesn't this mean that cgroup_disable=cpu will disable whichever comes
>>  first out of cpuset, cpuacct or cpu in the subsystem list?
> 
> Or rather, it's the other way around - cgroup_disable=cpuset will
> instead disable the "cpu" subsystem if "cpu" comes before "cpuset" in
> the subsystem list.
> 

Would it? I must be missing something, since we do a strncmp with ss->name.
I would expect that to match whole strings.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

Re: [PATCH] Add cgroup support for enabling controllers at boot time (v2)

[Paul Menage]

On Fri, Mar 7, 2008 at 4:58 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>  > Or rather, it's the other way around - cgroup_disable=cpuset will
>  > instead disable the "cpu" subsystem if "cpu" comes before "cpuset" in
>  > the subsystem list.
>  >
>
>  Would it? I must be missing something, since we do a strncmp with ss->name.
>  I would expect that to match whole strings.
>

No, strncmp only checks the first n characters - so in that case,
you'd be checking for !strncmp("cpuset", "cpu", 3), which will return
true

Paul

Re: [PATCH] Add cgroup support for enabling controllers at boot time (v2)

[Balbir Singh]

Paul Menage wrote:
> On Fri, Mar 7, 2008 at 4:58 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>>  > Or rather, it's the other way around - cgroup_disable=cpuset will
>>  > instead disable the "cpu" subsystem if "cpu" comes before "cpuset" in
>>  > the subsystem list.
>>  >
>>
>>  Would it? I must be missing something, since we do a strncmp with ss->name.
>>  I would expect that to match whole strings.
>>
> 
> No, strncmp only checks the first n characters - so in that case,
> you'd be checking for !strncmp("cpuset", "cpu", 3), which will return
> true

Aaah.. I see the problem now.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL