containers: Generic Process Containers (V6)

--

This is an update to my multi-hierarchy multi-subsystem generic process containers patch. Changes include:

- updated to 2.6.20-rc1

- incorporating some fixes from Srivatsa Vaddagiri

- release agent path is per-hierarchy, and defaults to empty

- dropped the patch splitting cpusets and memsets for now, since it's a fairly mechanical patch, but a bit painful to maintain in the presence of any other cpusets changes in mainline.

A couple of important issues have arisen on which feedback would be appreciated:

1) The need (or otherwise) for multi-subsystem hierarchies
-----------------------------------------------------------

(Based on discussions with Paul Jackson)

The patch as it stands allows you to mount a container hierarchy as a filesystem, and at that point select a set of subsystems to be bound to that hierarchy. Subsystems can't be bound or unbound while the hierarchy is active. Dynamic binding/unbinding in this way is theoretically possible, but runs into various nasty conditions when you have to attach or detach a subsystem to a potentially large tree of containers and tasks. E.g. what do you do if halfway through unbinding the subsystem state from the containers in a hierarchy, you run into a container whose subsystem state is busy and hence can't be freed? Either we'd need to put fairly strict limits on the properties of subsystems that can be dynamically bound/unbound (i.e. can't refuse to transfer a task from one container to another, can't use css refcounting to keep subsystem state alive, etc) or we'd need to be prepared to do some very nasty error-handling and rollback.

PaulJ pointed out that there's a continuum between:
1) just one controller per container, period, or
2) full dynamic binding and unbinding of any controller from any one or more containers, with no limitations due to what else is or ever was bound to what when.

My current patch falls somewhere in the middle. PaulJ wondered whether it would be cleaner just to aim for case 1 - i.e. rather than having the concept of hierarchies to tie multiple subsystems together, have each subsystem be its own hierarchy.

My own feeling is that having each subsystem be its own hierarchy is certainly possible, but results in a lot more effort in userspace to manage - you have to manage N hierarchies of containers for N subsystems, rather than just one.

You also have the issue that if a task is forking just as you start moving it from one container to another (in each of the N hierarchies) you could end up with the child in the original container in some hierarchies, and in the new container in others, which isn't ideal.

little more overhead at fork()/exit() time (since there are more reference counts to atomically update) but a little less overhead for accessing the subsystem state for a particular task, since there's one less level of indirection involved.

2) Dynamic creation/destruction of containers from inside the kernel
--------------------------------------------------------------------

A recent patch from Serge Hallyn on [email protected] proposed a container filesystem somewhat similar to the one in this patch, designed to expose the hierarchy of namespaces (specifically, nsproxy objects). A major difference was that a child container could be created dynamically from within sys_unshare(), and automatically freed once it was no longer being referenced.

This could be fitted into my container model with a couple of small changes:

- a container_clone() function that essentially creates a new child container of the current container (in the specified subsystem's hierarchy) and moves the current process into the new container - the equivalent of doing

mkdir $current_container_dir/$some_unique_name echo $$ > $current_container_dir/$some_unique_name

- an auto_delete option for containers - similar to notify_on_exit, but rather than invoking a userspace program, simply deletes the container from within the kernel.

Then the nsproxy container could be implemented easily as a container subsystem - rather than having a direct nsproxy field in struct task, it would use the generic container pointer associated with the nsproxy subsystem; sys_unshare() would call container_clone() to create the new container.

=================================================================

Generic Process Containers
--------------------------

There have recently been various proposals floating around for resource management/accounting subsystems in the kernel, including ResGroups, User BeanCounters, NSProxy containers, and others. These all need the basic abstraction of being able to group together multiple processes in an aggregate, in order to track/limit the resources permitted to those processes, and all implement this grouping in different ways.

Already existing in the kernel is the cpuset subsystem; this has a process grouping mechanism that is mature, tested, and well documented (particularly with regards to synchronization rules).

This patchset extracts the process grouping code from cpusets into a generic container system, and makes the cpusets code a client of the container system.

It also provides several example clients of the container system, including ResGroups and BeanCounters

The change is implemented in three stages, plus three example subsystems that aren't necessarily intended to be merged as part of this patch set, but demonstrate the applicability of the framework.

1) extract the process grouping code from cpusets into a standalone system

2) remove the process grouping code from cpusets and hook into the container system

3) convert the container system to present a generic multi-hierarchy API, and make cpusets a client of that API

4) example of a simple CPU accounting container subsystem

5) example of implementing ResGroups and its numtasks controller over generic containers

6) example of implementing BeanCounters and its numfiles counter over generic containers

The intention is that the various resource management efforts can also become container clients, with the result that:

- the userspace APIs are (somewhat) normalised

- it's easier to test out e.g. the ResGroups CPU controller in conjunction with the BeanCounters memory controller

- the additional kernel footprint of any of the competing resource management systems is substantially reduced, since it doesn't need to provide process grouping/containment, hence improving their chances of getting into the kernel

--

This patch creates a generic process container system based on (and parallel top) the cpusets code. At a coarse level it was created by copying kernel/cpuset.c, doing s/cpuset/container/g, and stripping out any code that was cpuset-specific rather than applicable to any process container subsystem.

---
 Documentation/containers.txt |  229 +++++++
 fs/proc/base.c               |    7 
 include/linux/container.h    |   96 +++
 include/linux/sched.h        |    5 
 init/Kconfig                 |    9 
 init/main.c                  |    3 
 kernel/Makefile              |    1 
 kernel/container.c           | 1343 +++++++++++++++++++++++++++++++++++++++++++
 kernel/exit.c                |    2 
 kernel/fork.c                |    3 
 10 files changed, 1697 insertions(+), 1 deletion(-)

Index: container-2.6.20-rc1/fs/proc/base.c
===================================================================
@@ -68,6 +68,7 @@
 #include <linux/security.h>
 #include <linux/ptrace.h>
 #include <linux/seccomp.h>
+#include <linux/container.h>
 #include <linux/cpuset.h>
 #include <linux/audit.h>
 #include <linux/poll.h>

@@ -1868,6 +1869,9 @@ static struct pid_entry tgid_base_stuff[
 #ifdef CONFIG_CPUSETS
 	REG("cpuset",     S_IRUGO, cpuset),
 #endif
+#ifdef CONFIG_CONTAINERS
+	REG("container",  S_IRUGO, container),
+#endif
 	INF("oom_score",  S_IRUGO, oom_score),
 	REG("oom_adj",    S_IRUGO|S_IWUSR, oom_adjust),
 #ifdef CONFIG_AUDITSYSCALL

@@ -2149,6 +2153,9 @@ static struct pid_entry tid_base_stuff[]
 #ifdef CONFIG_CPUSETS
 	REG("cpuset",    S_IRUGO, cpuset),
 #endif
+#ifdef CONFIG_CONTAINERS
+	REG("container",  S_IRUGO, container),
+#endif
 	INF("oom_score", S_IRUGO, oom_score),
 	REG("oom_adj",   S_IRUGO|S_IWUSR, oom_adjust),
 #ifdef CONFIG_AUDITSYSCALL

Index: container-2.6.20-rc1/include/linux/sched.h
===================================================================
@@ -743,8 +743,8 @@ extern unsigned int max_cache_size;
 
 
 struct io_context;			/* See blkdev.h */
+struct container;
 struct cpuset;
-
 #define NGROUPS_SMALL		32
 #define NGROUPS_PER_BLOCK	((int)(PAGE_SIZE / sizeof(gid_t)))
 struct group_info {

@@ -1031,6 +1031,9 @@ struct task_struct {
 	int cpuset_mems_generation;
 	int cpuset_mem_spread_rotor;
 #endif
+#ifdef CONFIG_CONTAINERS
+	struct container *container;
+#endif
 	struct robust_list_head __user *robust_list;
 #ifdef CONFIG_COMPAT
 	struct compat_robust_list_head __user *compat_robust_list;

Index: container-2.6.20-rc1/init/Kconfig
===================================================================
@@ -238,6 +238,15 @@ config IKCONFIG_PROC
 	  This option enables access to the kernel configuration file
 	  through /proc/config.gz.
 
+config CONTAINERS
+	bool "Container support"
+	help
+	  This option will let you create and manage process containers,
+	  which can be used to aggregate multiple processes, e.g. for
+	  the purposes of resource tracking.
+
+	  Say N if unsure
+
 config CPUSETS
 	bool "Cpuset support"
 	depends on SMP

Index: container-2.6.20-rc1/init/main.c
===================================================================
@@ -39,6 +39,7 @@
 #include <linux/writeback.h>
 #include <linux/cpu.h>
 #include <linux/cpuset.h>
+#include <linux/container.h>
 #include <linux/efi.h>
 #include <linux/taskstats_kern.h>
 #include <linux/delayacct.h>

@@ -581,6 +582,7 @@ asmlinkage void __init start_kernel(void
 	}
 #endif
 	vfs_caches_init_early();
+	container_init_early();
 	cpuset_init_early();
 	mem_init();
 	kmem_cache_init();

@@ -611,6 +613,7 @@ asmlinkage void __init start_kernel(void
 #ifdef CONFIG_PROC_FS
 	proc_root_init();
 #endif
+	container_init();
 	cpuset_init();
 	taskstats_init_early();
 	delayacct_init();

Index: container-2.6.20-rc1/kernel/exit.c
===================================================================
@@ -31,6 +31,7 @@
 #include <linux/taskstats_kern.h>
 #include <linux/delayacct.h>
 #include <linux/cpuset.h>
+#include <linux/container.h>
 #include <linux/syscalls.h>
 #include <linux/signal.h>
 #include <linux/posix-timers.h>

@@ -920,6 +921,7 @@ fastcall NORET_TYPE void do_exit(long co
 	__exit_fs(tsk);
 	exit_thread();
 	cpuset_exit(tsk);
+	container_exit(tsk);
 	exit_keys(tsk);
 
 	if (group_dead && tsk->signal->leader)

Index: container-2.6.20-rc1/kernel/fork.c
===================================================================
@@ -31,6 +31,7 @@
 #include <linux/capability.h>
 #include <linux/cpu.h>
 #include <linux/cpuset.h>
+#include <linux/container.h>
 #include <linux/security.h>
 #include <linux/swap.h>
 #include <linux/syscalls.h>

@@ -1058,6 +1059,7 @@ static struct task_struct *copy_process(
 	p->io_context = NULL;
 	p->io_wait = NULL;
 	p->audit_context = NULL;
+	container_fork(p);
 	cpuset_fork(p);
 #ifdef CONFIG_NUMA
  	p->mempolicy = mpol_copy(p->mempolicy);

@@ -1291,6 +1293,7 @@ bad_fork_cleanup_policy:
 bad_fork_cleanup_cpuset:
 #endif
 	cpuset_exit(p);
+	container_exit(p);
 bad_fork_cleanup_delays_binfmt:
 	delayacct_tsk_free(p);
 	if (p->binfmt)

Index: container-2.6.20-rc1/kernel/Makefile
===================================================================
@@ -36,6 +36,7 @@ obj-$(CONFIG_PM) += power/
 obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
 obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_COMPAT) += compat.o
+obj-$(CONFIG_CONTAINERS) += container.o
 obj-$(CONFIG_CPUSETS) += cpuset.o
 obj-$(CONFIG_IKCONFIG) += configs.o
 obj-$(CONFIG_STOP_MACHINE) += stop_machine.o

--

First thank you for bring the conversation here. Given what you are implementing I rather object to the term containers as that is what we have been using to refer to the aggregate whole and not the individual pieces.

I'm still digesting this but do you think you could make the code pid namespace safe before moving it all over creation.

i.e. pid_nr(task_pid(task)) not task->pid.

I hadn't realized we had any users like the one below left.

The whole interface that reads out the processes in your task grouping looks scary. It takes the tasklist_lock and holds it for an indefinite duration. All it currently needs is the rcu_read_lock. Holding the tasklist_lock looks like a good way to kill performance on a big box. Even hold the cpu for an indefinite duration I find a little worrying but no where near as bad as taking a global lock for an indefinite period of time. Although I am curious why this is even needed when we have /proc/<pid>/cpuset which gets us the information in another way.

This interface really belongs in /proc as it is about managing processes.

The filesystem operations to manage cpusets are a little non-intuitive but once you see what they are they appear usable.

I hate attach_task. Allowing movement of a process from one set to another by another process looks like a great way to create subtle races. The very long and exhaustive locking comments seem to verify this. For most of the unix API we have avoided things for precisely this reason. Leaving that set of races to the debugging commands in sys_ptrace.

You are putting a pointer into the task_struct for each class of resource you want to count. Ouch. Andi Kleen was sufficiently paranoid about the space bloat that we were obliged to introduce struct nsproxy.

The more I look at this the more this appears to be completely overkill for process resource control, and currently I am horrified at what currently looks like huge piles of unnecessary complexity in the cpuset implementation.

I still need to do some research but at the moment my feeling that this approach is so wrong that cpusets need to get fixed and nothing should ever look at cloning them.

Process resource control that looks like a good reason to add some more unshare flags or some separate syscalls whichever is simpler. At least that has a simple user interface that is easy to audit.

If nothing else the code needs to find a way to be refactored so it isn't scary too look at.

Please also next time explain the mechanism you are talking about using to track processes and don't grandfather it in with oh this is just a slightly enhanced cpuset. The insanity of this interface would have been a lot easier to have been spotted if it had been described more clearly.

Why does any of this code need a user mode helper? I guess because of the complicated semantics this doesn't do proper reference counting so you can't implicitly free these things on the exit of the last task that uses them. That isn't the unix way and I don't like it. Way over complicated.

Eric

It doesn't look "indefinite" to me. It reads the 'container' field of each task struct, and then is done, dropping the lock.

That's got to be one of the lowest cost, most definite duration, invocations of "do_each_thread(g, p)" in the kernel.

The /proc/<pid>/cpuset interface lets you map one pid to its cpuset.

That's the opposite of mapping a cpuset to the set of all pids that are attached to it.

I suppose one could get all the tasks in a cpuset by doing whatever it takes for an opendir/readdir/closedir loop over the pid entries of /proc, and then each pid in the system doing an open/read/close on its /proc/<pid>/cpuset, and doing a strcmp on its path with the cpuset path of interest, to see if they match.

What kind of locking is done in the kernel when a user task does an opendir/readdir/closedir loop over /proc?

In any case, this would be hecka more expensive than the current quite -definite- "do_each_thread(g, p)" over the task list, with three system calls per pid. And the 'tasks' file in cpusets is an existing and valuable feature, which we can't just remove without serious cause.

The ability to move tasks between cpusets a valued feature for my customers. Sorry you hate it.

I'll try to make my comments shorter and less exhausting next time </sarcarsm>.

The locking is difficult, because:
1) yes, as you note, attach_task() isn't easy,
2) the cpu and memory placement of a whole set of tasks can be changed by a single write system call on some cpuset file,
2) cpusets is on the critical code path for both the memory allocator and task scheduler (controlling where one can allocate and schedule), but needs to avoid putting any sigificant locks on either of these paths.

Not much I can do to help you with your horror, sorry.

If you could be more specific on ways to trim the code while maintaining the API's that we use, then that might be useful.

Let me know when you have patches.

The cpuset reference counting is just fine, thank-you.

Removing nodes from the bottom of a vfs file system, when one got there by an unexpected code path, such as task exit, is not easy. Well, for someone of my limited vfs talents, quite impossible. I had no desire (nor ability) to replicate in the kernel/cpuset.c code whatever voodoo it takes to get the vfs locking correct for a rmdir(2) system call.

Using a user mode helper lets this be handled using the ordinary rmdir(2) system call, with no special vfs locking awareness, from a separate thread.

... Hope you had a Merry Christmas.

                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.925.600.0401

I guess I'm agnostic on the exact term used - but one point is that this isn't intended just for the virtual server support that you're working on, but rather any kernel facility that wants to be able to associate data and/or behaviour with a group of processes (not necessarily related by process inheritance, but where the grouping is inherited at fork time). This includes Cpusets, and general resource isolation/management, without virtualization.

Terms like "process group", "session", etc, are already used up. A "container" seems like a reasonable term for a generic process grouping from which the processes can't escape without root privileges. Since you're not only "containing" processes but also "virtualizing" them, the term "virtual server" would seem better for your work, unless you were wanting to keep the same name as Solaris Containers.

OK, I'll take a look at that.

As PaulJ mentioned, it's much more efficient to read this once for a given container, rather than having to iterate over the whole of /proc. If it's possible to use RCU for this, I'd be very happy to change it to do that.

If the only way to get a process into a new container is to clone the current container and shift the current process into it, that's a very restrictive model, and too restrictive for some of the things that I and others want to do. (E.g. moving a process between different resource containers based on which client it's currently doing work; adding a new process to an existing running job).

I'd be interested in supporting the clone-based model as well, though. With the addition of that, it would always be possible for a subsystem that wants to just support the clone model, to always fail its can_attach_task() call to prevent the container system from moving external processes into the container.

No, I'm putting a pointer for each independent hierarchy that you want to maintain - multiple classes of resources can be tracked in the same hierarchy. The max number of hierarchies is a number that's configurable at compile time.

I think that nsproxy would be a good example of something that could be attached as a generic container subsystem. I'd need to make a couple of additions - a way to dynamically create a new container at fork/unshare time and move the newly unshared process into it, and a way to auto-delete a container (see below), so I'm not suggesting it quite yet.

There are too many different resources and competing views on resource control to be able to handle this via a few extra flags, I think.

That's there for compatibility with cpusets. I was thinking of adding an auto-delete option that does queue_work() to trigger a vfs_rmdir() from the work queue, which would avoid the races that PaulJ was concerned about. But I can also envisage more exotic cases where userspace wants to do something more complex (e.g. read some final accounting values) before deleting the container.

Paul

This patch removes the process grouping code from the cpusets code, instead hooking it into the generic container system. This temporarily adds cpuset-specific code in kernel/container.c, which is removed by the next patch in the series.

---
 Documentation/cpusets.txt |   81 +-
 fs/proc/base.c            |    4 
 fs/super.c                |    5 
 include/linux/container.h |    7 
 include/linux/cpuset.h    |   25 
 include/linux/fs.h        |    2 
 include/linux/mempolicy.h |    2 
 include/linux/sched.h     |    4 
 init/Kconfig              |   14 
 kernel/container.c        |  107 +++
 kernel/cpuset.c           | 1269 +++++-----------------------------------------
 kernel/exit.c             |    2 
 kernel/fork.c             |    7 
 mm/oom_kill.c             |    6 
 14 files changed, 319 insertions(+), 1216 deletions(-)

Index: container-2.6.20-rc1/include/linux/container.h
===================================================================
@@ -47,6 +47,10 @@ struct container {
 
 	struct container *parent;	/* my parent */
 	struct dentry *dentry;		/* container fs entry */
+
+#ifdef CONFIG_CPUSETS
+	struct cpuset *cpuset;
+#endif
 };
 
 /* struct cftype:

@@ -79,6 +83,9 @@ struct cftype {
 int container_add_file(struct container *cont, const struct cftype *cft);
 
 int container_is_removed(const struct container *cont);
+void container_set_release_agent_path(const char *path);
+
+int container_path(const struct container *cont, char *buf, int buflen);
 
 #else /* !CONFIG_CONTAINERS */
 

Index: container-2.6.20-rc1/include/linux/cpuset.h
===================================================================
@@ -11,16 +11,15 @@
 #include <linux/sched.h>
 #include <linux/cpumask.h>
 #include <linux/nodemask.h>
+#include <linux/container.h>
 
 #ifdef CONFIG_CPUSETS
 
-extern int number_of_cpusets;	/* How many cpusets are defined in system? */
+extern int number_of_cpusets;  /* How many cpusets are defined in system? */
 
 extern int cpuset_init_early(void);
 extern int cpuset_init(void);
 extern void cpuset_init_smp(void);
-extern void cpuset_fork(struct task_struct *p);
-extern void cpuset_exit(struct task_struct *p);
 extern cpumask_t cpuset_cpus_allowed(struct task_struct *p);
 extern nodemask_t cpuset_mems_allowed(struct task_struct *p);
 #define cpuset_current_mems_allowed (current->mems_allowed)

@@ -57,10 +56,6 @@ extern void __cpuset_memory_pressure_bum
 
 extern const struct file_operations proc_cpuset_operations;
 extern char *cpuset_task_status_allowed(struct task_struct *task, char *buffer);
-
-extern void cpuset_lock(void);
-extern void cpuset_unlock(void);
-
 extern int cpuset_mem_spread_node(void);
 
 static inline int cpuset_do_page_mem_spread(void)

@@ -75,13 +70,22 @@ static inline int cpuset_do_slab_mem_spr
 
 extern void cpuset_track_online_nodes(void);
 
+extern int cpuset_can_attach_task(struct container *cont,
+				  struct task_struct *tsk);
+extern void cpuset_attach_task(struct container *cont,
+				struct task_struct *tsk);
+extern void cpuset_post_attach_task(struct container *cont,
+				    struct container *oldcont,
+				    struct task_struct *tsk);
+extern int cpuset_populate_dir(struct container *cont);
+extern int cpuset_create(struct container *cont);
+extern void cpuset_destroy(struct container *cont);
+
 #else /* !CONFIG_CPUSETS */
 
 static inline int cpuset_init_early(void) { return 0; }
 static inline int cpuset_init(void) { return 0; }
 static inline void cpuset_init_smp(void) {}
-static inline void cpuset_fork(struct task_struct *p) {}
-static inline void cpuset_exit(struct task_struct *p) {}
 
 static inline cpumask_t cpuset_cpus_allowed(struct task_struct *p)
 {

@@ -126,9 +130,6 @@ static inline char *cpuset_task_status_a
 	return buffer;
 }
 
-static inline void cpuset_lock(void) {}
-static inline void cpuset_unlock(void) {}
-
 static inline int cpuset_mem_spread_node(void)
 {
 	return 0;

Index: container-2.6.20-rc1/kernel/exit.c
===================================================================
@@ -30,7 +30,6 @@
 #include <linux/mempolicy.h>
 #include <linux/taskstats_kern.h>
 #include <linux/delayacct.h>
-#include <linux/cpuset.h>
 #include <linux/container.h>
 #include <linux/syscalls.h>
 #include <linux/signal.h>

@@ -920,7 +919,6 @@ fastcall NORET_TYPE void do_exit(long co
 	__exit_files(tsk);
 	__exit_fs(tsk);
 	exit_thread();
-	cpuset_exit(tsk);
 	container_exit(tsk);
 	exit_keys(tsk);
 

Index: container-2.6.20-rc1/kernel/fork.c
===================================================================
@@ -30,7 +30,6 @@
 #include <linux/nsproxy.h>
 #include <linux/capability.h>
 #include <linux/cpu.h>
-#include <linux/cpuset.h>
 #include <linux/container.h>
 #include <linux/security.h>
 #include <linux/swap.h>

@@ -1060,13 +1059,12 @@ static struct task_struct *copy_process(
 	p->io_wait = NULL;
 	p->audit_context = NULL;
 	container_fork(p);
-	cpuset_fork(p);
 #ifdef CONFIG_NUMA
  	p->mempolicy = mpol_copy(p->mempolicy);
  	if (IS_ERR(p->mempolicy)) {
  		retval = PTR_ERR(p->mempolicy);
  		p->mempolicy = NULL;
- 		goto bad_fork_cleanup_cpuset;
+ 		goto bad_fork_cleanup_container;
  	}
 	mpol_fix_fork_child_flag(p);
 #endif

@@ -1290,9 +1288,8 @@ bad_fork_cleanup_security:
 bad_fork_cleanup_policy:
 #ifdef CONFIG_NUMA
 	mpol_free(p->mempolicy);
-bad_fork_cleanup_cpuset:
+bad_fork_cleanup_container:
 #endif
-	cpuset_exit(p);
 	container_exit(p);
 bad_fork_cleanup_delays_binfmt:
 	delayacct_tsk_free(p);

Index: container-2.6.20-rc1/init/Kconfig
===================================================================
@@ -239,17 +239,12 @@ config IKCONFIG_PROC
 	  through /proc/config.gz.
 
 config CONTAINERS
-	bool "Container support"
-	help
-	  This option will let you create and manage process containers,
-	  which can be used to aggregate multiple processes, e.g. for
-	  the purposes of resource tracking.
-
-	  Say N if unsure
+	bool
 
 config CPUSETS
 	bool "Cpuset support"
 	depends on SMP
+	select CONTAINERS
 	help
 	  This option will let you create and manage CPUSETs which
 	  allow dynamically partitioning a system into sets of CPUs and

@@ -278,6 +273,11 @@ config SYSFS_DEPRECATED
 	  If you are using a distro that was released in 2006 or later,
 	  it should be safe to say N here.
 
+config PROC_PID_CPUSET
+	bool "Include legacy /proc/<pid>/cpuset file"
+	depends on CPUSETS
+	default y
+
 config RELAY
 	bool "Kernel->user space relay support (formerly relayfs)"
 	help

Index: container-2.6.20-rc1/mm/oom_kill.c
===================================================================
@@ -404,7 +404,7 @@ void out_of_memory(struct zonelist *zone
 		show_mem();
 	}
 
-	cpuset_lock();
+	container_lock();
 	read_lock(&tasklist_lock);
 
 	/*

@@ -438,7 +438,7 @@ retry:
 		/* Found nothing?!?! Either we hang forever, or we panic. */
 		if (!p) {
 			read_unlock(&tasklist_lock);
-			cpuset_unlock();
+			container_unlock();
 			panic("Out of memory and no killable processes...\n");
 		}
 

@@ -450,7 +450,7 @@ retry:
 
 out:
 	read_unlock(&tasklist_lock);
-	cpuset_unlock();
+	container_unlock();
 
 	/*
 	 * Give "p" a good chance of killing itself before we

Index: container-2.6.20-rc1/include/linux/sched.h
===================================================================
@@ -744,7 +744,6 @@ extern unsigned int max_cache_size;
 
 struct io_context;			/* See blkdev.h */
 struct container;
-struct cpuset;
 #define NGROUPS_SMALL		32
 #define NGROUPS_PER_BLOCK	((int)(PAGE_SIZE / sizeof(gid_t)))
 struct group_info {

@@ -1026,7 +1025,6 @@ struct task_struct {
 	short il_next;
 #endif
 #ifdef CONFIG_CPUSETS
-	struct cpuset *cpuset;
 	nodemask_t mems_allowed;
 	int cpuset_mems_generation;
 	int cpuset_mem_spread_rotor;

@@ -1471,7 +1469,7 @@ static inline int thread_group_empty(str
 /*
  * Protects ->fs, ->files, ->mm, ->group_info, ->comm, keyring
  * subscriptions and synchronises with wait4().  Also used in procfs.  Also
- * pins the final release of task.io_context.  Also protects ->cpuset.
+ * pins the final release of task.io_context.  Also protects ->container.
  *
  * Nests both inside and outside of read_lock(&tasklist_lock).
  * It must not be nested with write_lock_irq(&tasklist_lock),

Index: container-2.6.20-rc1/fs/super.c
===================================================================
@@ -39,11 +39,6 @@
 #include <linux/mutex.h>
 #include <asm/uaccess.h>
 
-
-void get_filesystem(struct file_system_type *fs);
-void put_filesystem(struct file_system_type *fs);
-struct file_system_type *get_fs_type(const char *name);
-
 LIST_HEAD(super_blocks);
 DEFINE_SPINLOCK(sb_lock);
 

Index: container-2.6.20-rc1/include/linux/fs.h
===================================================================
@@ -1840,6 +1840,8 @@ extern int vfs_fstat(unsigned int, struc
 
 extern int vfs_ioctl(struct file *, unsigned int, unsigned int, unsigned long);
 
+extern void get_filesystem(struct file_system_type *fs);
+extern void put_filesystem(struct file_system_type *fs);
 extern struct file_system_type *get_fs_type(const char *name);
 extern struct super_block *get_super(struct block_device *);
 extern struct super_block *user_get_super(dev_t);

Index: container-2.6.20-rc1/include/linux/mempolicy.h
===================================================================
@@ -152,7 +152,7 @@ extern void mpol_fix_fork_child_flag(str
 
 #ifdef CONFIG_CPUSETS
 #define current_cpuset_is_being_rebound() \
-				(cpuset_being_rebound == current->cpuset)
+ (cpuset_being_rebound == current->container->cpuset)
 #else
 #define current_cpuset_is_being_rebound() 0
 #endif

Index: container-2.6.20-rc1/fs/proc/base.c
===================================================================
@@ -1866,7 +1866,7 @@ static struct pid_entry tgid_base_stuff[
 #ifdef CONFIG_SCHEDSTATS
 	INF("schedstat",  S_IRUGO, pid_schedstat),
 #endif
-#ifdef CONFIG_CPUSETS
+#ifdef CONFIG_PROC_PID_CPUSET
 	REG("cpuset",     S_IRUGO, cpuset),
 #endif
 #ifdef CONFIG_CONTAINERS

@@ -2150,7 +2150,7 @@ static struct pid_entry tid_base_stuff[]
 #ifdef CONFIG_SCHEDSTATS
 	INF("schedstat", S_IRUGO, pid_schedstat),
 #endif
-#ifdef CONFIG_CPUSETS
+#ifdef CONFIG_PROC_PID_CPUSET
 	REG("cpuset",    S_IRUGO, cpuset),
 #endif
 #ifdef CONFIG_CONTAINERS

--

This patch removes all cpuset-specific knowlege from the container system, replacing it with a generic API that can be used by multiple subsystems. Cpusets is adapted to be a container subsystem.

---
 Documentation/containers.txt |  273 ++++++++++---
 Documentation/cpusets.txt    |   20 
 include/linux/container.h    |  137 ++++++
 include/linux/cpuset.h       |   16 
 include/linux/mempolicy.h    |   12 
 include/linux/sched.h        |    4 
 init/Kconfig                 |   12 
 kernel/container.c           |  903 +++++++++++++++++++++++++++++++++----------
 kernel/cpuset.c              |  173 +++++---
 mm/mempolicy.c               |    2 
 10 files changed, 1215 insertions(+), 337 deletions(-)

Index: container-2.6.20-rc1/include/linux/container.h
===================================================================
@@ -14,8 +14,6 @@
 
 #ifdef CONFIG_CONTAINERS
 
-extern int number_of_containers;	/* How many containers are defined in system? */
-
 extern int container_init_early(void);
 extern int container_init(void);
 extern void container_init_smp(void);

@@ -30,6 +28,68 @@ extern void container_unlock(void);
 extern void container_manage_lock(void);
 extern void container_manage_unlock(void);
 
+struct containerfs_root;
+
+/* Per-subsystem/per-container state maintained by the system. */
+struct container_subsys_state {
+	/* The container that this subsystem is attached to. Useful
+	 * for subsystems that want to know about the container
+	 * hierarchy structure */
+	struct container *container;
+
+	/* State maintained by the container system to allow
+	 * subsystems to be "busy". Should be accessed via css_get()
+	 * and css_put() */
+	spinlock_t refcnt_lock;
+	atomic_t refcnt;
+};
+
+/*
+ * Call css_get() to hold a reference on the container; following a
+ * return of 0, this container subsystem state object is guaranteed
+ * not to be destroyed until css_put() is called on it.  A non-zero
+ * return code indicates that a reference could not be taken.
+ *
+ */
+
+static inline int css_get(struct container_subsys_state *css)
+{
+	int retval = 0;
+	unsigned long flags;
+	/* Synchronize with container_rmdir() */
+	spin_lock_irqsave(&css->refcnt_lock, flags);
+	if (atomic_read(&css->refcnt) >= 0) {
+		/* Container is still alive */
+		atomic_inc(&css->refcnt);
+	} else {
+		/* Container removal is in progress */
+		retval = -EINVAL;
+	}
+	spin_unlock_irqrestore(&css->refcnt_lock, flags);
+	return retval;
+}
+
+/*
+ * If you are holding current->alloc_lock then it's impossible for you
+ * to be moved out of your container, and hence it's impossible for
+ * your container to be destroyed. Therefore doing a simple
+ * atomic_inc() on a css is safe.
+ */
+
+static inline void css_get_current(struct container_subsys_state *css)
+{
+	atomic_inc(&css->refcnt);
+}
+
+/*
+ * css_put() should be called to release a reference taken by
+ * css_get() or css_get_current()
+ */
+
+static inline void css_put(struct container_subsys_state *css) {
+	atomic_dec(&css->refcnt);
+}
+
 struct container {
 	unsigned long flags;		/* "unsigned long" so bitops work */
 

@@ -46,11 +106,15 @@ struct container {
 	struct list_head children;	/* my children */
 
 	struct container *parent;	/* my parent */
-	struct dentry *dentry;		/* container fs entry */
+	struct dentry *dentry;	  	/* container fs entry */
+
+	/* Private pointers for each registered subsystem */
+	struct container_subsys_state *subsys[CONFIG_MAX_CONTAINER_SUBSYS];
+
+	int hierarchy;
 
-#ifdef CONFIG_CPUSETS
-	struct cpuset *cpuset;
-#endif
+	struct containerfs_root *root;
+	struct container *top_container;
 };
 
 /* struct cftype:

@@ -67,8 +131,11 @@ struct container {
  */
 
 struct inode;
+#define MAX_CFTYPE_NAME 64
 struct cftype {
-	char *name;
+	/* By convention, the name should begin with the name of the
+	 * subsystem, followed by a period */
+	char name[MAX_CFTYPE_NAME];
 	int private;
 	int (*open) (struct inode *inode, struct file *file);
 	ssize_t (*read) (struct container *cont, struct cftype *cft,

@@ -83,7 +150,61 @@ struct cftype {
 int container_add_file(struct container *cont, const struct cftype *cft);
 
 int container_is_removed(const struct container *cont);
-void container_set_release_agent_path(const char *path);
+
+int container_path(const struct container *cont, char *buf, int buflen);
+
+/* Container subsystem type. See Documentation/containers.txt for details */
+
+struct container_subsys {
+	int (*create)(struct container_subsys *ss,
+		      struct container *cont);
+	void (*destroy)(struct container_subsys *ss, struct container *cont);
+	int (*can_attach)(struct container_subsys *ss,
+			  struct container *cont, struct task_struct *tsk);
+	void (*attach)(struct container_subsys *ss, struct container *cont,
+			struct container *old_cont, struct task_struct *tsk);
+	void (*post_attach)(struct container_subsys *ss,
+			    struct container *cont,
+			    struct container *old_cont,
+			    struct task_struct *tsk);
+	void (*fork)(struct container_subsys *ss, struct task_struct *task);
+	void (*exit)(struct container_subsys *ss, struct task_struct *task);
+	int (*populate)(struct container_subsys *ss,
+			struct container *cont);
+
+	int subsys_id;
+#define MAX_CONTAINER_TYPE_NAMELEN 32
+	const char *name;
+
+	/* Protected by RCU */
+	int hierarchy;
+
+	struct list_head sibling;
+};
+
+int container_register_subsys(struct container_subsys *subsys);
+void container_set_release_agent_path(struct container_subsys *ss,
+				      const char *path);
+
+static inline struct container_subsys_state *container_subsys_state(
+	struct container *cont,
+	struct container_subsys *ss)
+{
+	return cont->subsys[ss->subsys_id];
+}
+
+static inline struct container* task_container(struct task_struct *task,
+					       struct container_subsys *ss)
+{
+	return rcu_dereference(task->container[ss->hierarchy]);
+}
+
+static inline struct container_subsys_state *task_subsys_state(
+	struct task_struct *task,
+	struct container_subsys *ss)
+{
+	return container_subsys_state(task_container(task, ss), ss);
+}
 
 int container_path(const struct container *cont, char *buf, int buflen);
 

Index: container-2.6.20-rc1/include/linux/cpuset.h
===================================================================
@@ -70,16 +70,7 @@ static inline int cpuset_do_slab_mem_spr
 
 extern void cpuset_track_online_nodes(void);
 
-extern int cpuset_can_attach_task(struct container *cont,
-				  struct task_struct *tsk);
-extern void cpuset_attach_task(struct container *cont,
-				struct task_struct *tsk);
-extern void cpuset_post_attach_task(struct container *cont,
-				    struct container *oldcont,
-				    struct task_struct *tsk);
-extern int cpuset_populate_dir(struct container *cont);
-extern int cpuset_create(struct container *cont);
-extern void cpuset_destroy(struct container *cont);
+extern int current_cpuset_is_being_rebound(void);
 
 #else /* !CONFIG_CPUSETS */
 

@@ -147,6 +138,11 @@ static inline int cpuset_do_slab_mem_spr
 
 static inline void cpuset_track_online_nodes(void) {}
 
+static inline int current_cpuset_is_being_rebound(void)
+{
+	return 0;
+}
+
 #endif /* !CONFIG_CPUSETS */
 
 #endif /* _LINUX_CPUSET_H */

Index: container-2.6.20-rc1/include/linux/mempolicy.h
===================================================================
@@ -148,14 +148,6 @@ extern void mpol_rebind_task(struct task
 					const nodemask_t *new);
 extern void mpol_rebind_mm(struct mm_struct *mm, nodemask_t *new);
 extern void mpol_fix_fork_child_flag(struct task_struct *p);
-#define set_cpuset_being_rebound(x) (cpuset_being_rebound = (x))
-
-#ifdef CONFIG_CPUSETS
-#define current_cpuset_is_being_rebound() \
- (cpuset_being_rebound == current->container->cpuset)
-#else
-#define current_cpuset_is_being_rebound() 0
-#endif
 
 extern struct mempolicy default_policy;
 extern struct zonelist *huge_zonelist(struct vm_area_struct *vma,

@@ -173,8 +165,6 @@ static inline void check_highest_zone(en
 int do_migrate_pages(struct mm_struct *mm,
 	const nodemask_t *from_nodes, const nodemask_t *to_nodes, int flags);
 
-extern void *cpuset_being_rebound;	/* Trigger mpol_copy vma rebind */
-
 #else
 
 struct mempolicy {};

@@ -253,8 +243,6 @@ static inline void mpol_fix_fork_child_f
 {
 }
 
-#define set_cpuset_being_rebound(x) do {} while (0)
-
 static inline struct zonelist *huge_zonelist(struct vm_area_struct *vma,
 		unsigned long addr)
 {

Index: container-2.6.20-rc1/include/linux/sched.h
===================================================================
@@ -1030,7 +1030,7 @@ struct task_struct {
 	int cpuset_mem_spread_rotor;
 #endif
 #ifdef CONFIG_CONTAINERS
-	struct container *container;
+	struct container *container[CONFIG_MAX_CONTAINER_HIERARCHIES];
 #endif
 	struct robust_list_head __user *robust_list;
 #ifdef CONFIG_COMPAT

@@ -1469,7 +1469,7 @@ static inline int thread_group_empty(str
 /*
  * Protects ->fs, ->files, ->mm, ->group_info, ->comm, keyring
  * subscriptions and synchronises with wait4().  Also used in procfs.  Also
- * pins the final release of task.io_context.  Also protects ->container.
+ * pins the final release of task.io_context.  Also protects ->container[].
  *
  * Nests both inside and outside of read_lock(&tasklist_lock).
  * It must not be nested with write_lock_irq(&tasklist_lock),

Index: container-2.6.20-rc1/mm/mempolicy.c
===================================================================
@@ -1309,7 +1309,6 @@ EXPORT_SYMBOL(alloc_pages_current);
  * keeps mempolicies cpuset relative after its cpuset moves.  See
  * further kernel/cpuset.c update_nodemask().
  */
-void *cpuset_being_rebound;
 
 /* Slow path of a mempolicy copy */
 struct mempolicy *__mpol_copy(struct mempolicy *old)

@@ -1908,4 +1907,3 @@ out:
 		m->version = (vma != priv->tail_vma) ? vma->vm_start : 0;
 	return 0;
 }
-

Index: container-2.6.20-rc1/init/Kconfig
===================================================================
@@ -241,6 +241,18 @@ config IKCONFIG_PROC
 config CONTAINERS
 	bool
 
+config MAX_CONTAINER_SUBSYS
+	int "Number of container subsystems to support"
+	depends on CONTAINERS
+	range 1 255
+	default 8
+
+config MAX_CONTAINER_HIERARCHIES
+	int "Number of container hierarchies to support"
+	depends on CONTAINERS
+	range 2 255
+	default 4
+
 config CPUSETS
 	bool "Cpuset support"
 	depends on SMP

Index: container-2.6.20-rc1/Documentation/cpusets.txt
===================================================================
@@ -466,7 +466,7 @@ than stress the kernel.
 To start a new job that is to be contained within a cpuset, the steps are:
 
  1) mkdir /dev/cpuset
- 2) mount -t container none /dev/cpuset
+ 2) mount -t container -ocpuset cpuset /dev/cpuset
  3) Create the new cpuset by doing mkdir's and write's (or echo's) in
     the /dev/cpuset virtual file system.
  4) Start a task that will be the "founding father" of the new job.

@@ -478,7 +478,7 @@ For example, the following sequence of c
 named "Charlie", containing just CPUs 2 and 3, and Memory Node 1,
 and then start a subshell 'sh' in that cpuset:
 
-  mount -t container none /dev/cpuset
+  mount -t container -ocpuset cpuset /dev/cpuset
   cd /dev/cpuset
   mkdir Charlie
   cd Charlie

@@ -488,7 +488,7 @@ and then start a subshell 'sh' in that c
   sh
   # The subshell 'sh' is now running in cpuset Charlie
   # The next line should display '/Charlie'
-  cat /proc/self/container
+  cat /proc/self/cpuset
 
 In the future, a C library interface to cpusets will likely be
 available.  For now, the only way to query or modify cpusets is

@@ -510,7 +510,7 @@ Creating, modifying, using the cpusets c
 virtual filesystem.
 
 To mount it, type:
-# mount -t container none /dev/cpuset
+# mount -t container -o cpuset cpuset /dev/cpuset
 
 Then under /dev/cpuset you can find a tree that corresponds to the
 tree of the cpusets in the system. For instance, /dev/cpuset

@@ -550,6 +550,18 @@ To remove a cpuset, just use rmdir:
 This will fail if the cpuset is in use (has cpusets inside, or has
 processes attached).
 
+Note that for legacy reasons, the "cpuset" filesystem exists as a
+wrapper around the container filesystem.
+
+The command
+
+mount -t cpuset X /dev/cpuset
+
+is equivalent to
+
+mount -t container -ocpuset X /dev/cpuset
+echo "/sbin/cpuset_release_agent" > /dev/cpuset/release_agent
+
 2.2 Adding/removing cpus
 ------------------------
 

--

There are two places that the subsystem create() function is called - the first is during the subsystem registration, to create the subsystem state for the root container. That one passes in dummytop since that is the container that all subsystems start attached to.

For clarification, the default (dummy) hierarchy is a placeholder for subsystems that aren't bound to a hierarchy. It always contains exactly one container (dummytop) and all processes are members of that container. It isn't reference-counted, since it can never go away, and it can never have any subcontainers.

When a real subcontainer is created (which must be after a subsystem has been bound to a hierarchy via a filesystem mount), the new subcontainer is passed in. From there you can follow the top_container field in the subcontainer, which leads to the root of the hierarchy.

Andrew has suggested that I need to document this better :-)

Paul

Yes, I saw that.

One of things I was trying to do with cpu_acct was to actually calculate the % load over a defined interval. I have the patch for that ready. When the interval ticks over (which happens in interrupt context - account_xxxxx_time()), I want to reset the load of child containers to 0. To walk the hierarchy, I have no root now since I do not have any task context. I was wondering if exporting the rootnode or providing a function to export the rootnode of the mounter hierarchy will make programming easier.

Something like

struct container *get_root_container(struct container_subsys *ss)

{
	return &rootnode[ss->hierarchy];
}

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs

Ah - I misunderstood what you were looking for before.

Yes, something like that sounds fine - except that it would be

return &rootnode[ss->hierarchy].top_container;

Paul

I'll try and post the changes to cpu_acct once I get the container system working.

Aah! I missed the top_container bit. Do you want me to send you a patch for it? It will be nice to have it in the next version.

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs

Here it is, a simple patch to keep track of percentage cpu load of a container. This patch depends on the add mount callbacks patch and another patch that fixes cpuacct for powerpc boxes (posted previously).

The patch attempts to add a percentage load calculation for each container. It also maintains an accumulated time counter, which accounts for the total cpu time taken by the container.

Compiled and tested on a 4 cpu powerpc box. Paul, please include this in your next series of patches for containers.

---

 include/linux/cpu_acct.h |    4 ++
 kernel/cpu_acct.c        |   90 
+++++++++++++++++++++++++++++++++++++++++++----
 kernel/sched.c           |    7 ++-
 3 files changed, 93 insertions(+), 8 deletions(-)

diff -puN kernel/cpu_acct.c~cpu_acct_load_acct kernel/cpu_acct.c
@@ -13,16 +13,22 @@
 #include <linux/module.h>
 #include <linux/container.h>
 #include <linux/fs.h>
+#include <linux/time.h>
 #include <asm/div64.h>
 
 struct cpuacct {
 	struct container_subsys_state css;
 	spinlock_t lock;
 	cputime64_t time; // total time used by this class
+	cputime64_t accum_time; // total time used by this class
 };
 
 static struct container_subsys cpuacct_subsys;
 static struct container *root;
+static spinlock_t interval_lock;
+static cputime64_t interval_time;
+static unsigned long long timestamp;
+static unsigned long long interval;
 
 static inline struct cpuacct *container_ca(struct container *cont)
 {

@@ -41,6 +47,8 @@ static int cpuacct_create(struct contain
 	if (!ca) return -ENOMEM;
 	spin_lock_init(&ca->lock);
 	cont->subsys[cpuacct_subsys.subsys_id] = &ca->css;
+	ca->time = cputime64_zero;
+	ca->accum_time = cputime64_zero;
 	return 0;
 }
 

@@ -67,17 +75,35 @@ static ssize_t cpuusage_read(struct cont
 			     size_t nbytes, loff_t *ppos)
 {
 	struct cpuacct *ca = container_ca(cont);
-	cputime64_t time;
-	unsigned long time_in_jiffies;
+	unsigned long long time;
+	unsigned long long accum_time;
+	unsigned long long interval_jiffies;
 	char usagebuf[64];
 	char *s = usagebuf;
 
 	spin_lock_irq(&ca->lock);
-	time = ca->time;
+	time = cputime64_to_jiffies64(ca->time);
+	accum_time = cputime64_to_jiffies64(ca->accum_time);
 	spin_unlock_irq(&ca->lock);
 
-	time_in_jiffies = cputime_to_jiffies(time);
-	s += sprintf(s, "%llu\n", (unsigned long long) time_in_jiffies);
+	spin_lock_irq(&interval_lock);
+	interval_jiffies = cputime64_to_jiffies64(interval_time);
+	spin_unlock_irq(&interval_lock);
+
+	s += sprintf(s, "time %llu\n", time);
+	s += sprintf(s, "accumulated time %llu\n", accum_time);
+	s += sprintf(s, "time since interval %llu\n", interval_jiffies);
+
+	/*
+	 * Calculate time in percentage
+	 */
+	time *= 100;
+	if (interval_jiffies)
+		do_div(time, interval_jiffies);
+	else
+		time = 0;
+
+	s += sprintf(s, "load %llu\n", time);
 
 	return simple_read_from_buffer(buf, nbytes, ppos, usagebuf, s - usagebuf);
 }

@@ -96,7 +122,6 @@ static int cpuacct_populate(struct conta
 
 void cpuacct_charge(struct task_struct *task, cputime_t cputime)
 {
-
 	struct cpuacct *ca;
 	unsigned long flags;
 

@@ -106,11 +131,60 @@ void cpuacct_charge(struct task_struct *
 	if (ca) {
 		spin_lock_irqsave(&ca->lock, flags);
 		ca->time = cputime64_add(ca->time, cputime);
+		ca->accum_time = cputime64_add(ca->accum_time, cputime);
 		spin_unlock_irqrestore(&ca->lock, flags);
 	}
 	rcu_read_unlock();
 }
 
+void cpuacct_uncharge(struct task_struct *task, cputime_t cputime)
+{
+	struct cpuacct *ca;
+	unsigned long flags;
+
+	if (cpuacct_subsys.subsys_id < 0 || !root) return;
+	rcu_read_lock();
+	ca = task_ca(task);
+	if (ca) {
+		spin_lock_irqsave(&ca->lock, flags);
+		ca->time = cputime64_sub(ca->time, cputime);
+		ca->accum_time = cputime64_sub(ca->accum_time, cputime);
+		spin_unlock_irqrestore(&ca->lock, flags);
+	}
+	rcu_read_unlock();
+}
+
+static void reset_ca_time(struct container *root)
+{
+	struct container *child;
+	struct cpuacct *ca;
+
+	if (root) {
+		ca = container_ca(root);
+		if (ca) {
+			spin_lock(&ca->lock);
+			ca->time = cputime64_zero;
+			spin_unlock(&ca->lock);
+		}
+		list_for_each_entry(child, &root->children, sibling)
+			reset_ca_time(child);
+	}
+}
+
+void cpuacct_update_time(cputime_t cputime)
+{
+	unsigned long flags;
+	unsigned long long timestamp_now = get_jiffies_64();
+	spin_lock_irqsave(&interval_lock, flags);
+	interval_time += cputime_to_cputime64(cputime);
+	if ((timestamp_now - timestamp) > interval) {
+		timestamp = timestamp_now;
+		reset_ca_time(root);
+		interval_time = cputime64_zero;
+	}
+	spin_unlock_irqrestore(&interval_lock, flags);
+}
+
 static struct container_subsys cpuacct_subsys = {
 	.name = "cpuacct",
 	.create = cpuacct_create,

@@ -127,6 +201,10 @@ int __init init_cpuacct(void)
 	int id;
 
 	root = NULL;
+	interval = 10 * HZ;
+	interval_time = cputime64_zero;
+	timestamp = get_jiffies_64();
+	spin_lock_init(&interval_lock);
 	id = container_register_subsys(&cpuacct_subsys);
 	return id < 0 ? id : 0;
 }

diff -puN kernel/sched.c~cpu_acct_load_acct kernel/sched.c
@@ -3078,7 +3078,7 @@ void account_user_time(struct task_struc
 	 */
 	if (p != rq->idle)
 		cpuacct_charge(p, cputime);
-
+	cpuacct_update_time(cputime);
 	/* Add user time to cpustat. */
 	tmp = cputime_to_cputime64(cputime);
 	if (TASK_NICE(p) > 0)

@@ -3100,6 +3100,7 @@ void account_system_time(struct task_str
 	struct rq *rq = this_rq();
 	cputime64_t tmp;
 
+	cpuacct_update_time(cputime);
 	p->stime = cputime_add(p->stime, cputime);
 
 	/* Add system time to cpustat. */

@@ -3136,8 +3137,10 @@ void account_steal_time(struct task_stru
 			cpustat->iowait = cputime64_add(cpustat->iowait, tmp);
 		else
 			cpustat->idle = cputime64_add(cpustat->idle, tmp);
-	} else
+	} else {
 		cpustat->steal = cputime64_add(cpustat->steal, tmp);
+		cpuacct_uncharge(p, tmp);
+	}
 }
 
 static void task_running_tick(struct rq *rq, struct task_struct *p)

diff -puN include/linux/cpu_acct.h~cpu_acct_load_acct include/linux/cpu_acct.h
@@ -7,8 +7,12 @@
 
 #ifdef CONFIG_CONTAINER_CPUACCT
 extern void cpuacct_charge(struct task_struct *, cputime_t cputime);
+extern void cpuacct_uncharge(struct task_struct *, cputime_t cputime);
+extern void cpuacct_update_time(cputime_t cputime);
 #else
 static void inline cpuacct_charge(struct task_struct *p, cputime_t cputime) {}
+static void inline cpuacct_uncharge(struct task_struct *p, cputime_t cputime) {}
+static void inline cpuacct_update_time(cputime_t cputime) {}
 #endif
 
 #endif

_

	Balbir Singh
	Linux Technology Center
	IBM, ISTL

This demonstrates how to use the generic container subsystem for a simple resource tracker that counts the total CPU time used by all processes in a container, during the time that they're members of the container.

---
 include/linux/cpu_acct.h |   14 +++++
 init/Kconfig             |    7 ++
 kernel/Makefile          |    1 
 kernel/cpu_acct.c        |  117 +++++++++++++++++++++++++++++++++++++++++++++++
 kernel/sched.c           |    6 ++
 5 files changed, 145 insertions(+)

Index: container-2.6.20-rc1/include/linux/cpu_acct.h
===================================================================
@@ -0,0 +1,14 @@
+
+#ifndef _LINUX_CPU_ACCT_H
+#define _LINUX_CPU_ACCT_H
+
+#include <linux/container.h>
+#include <asm/cputime.h>
+
+#ifdef CONFIG_CONTAINER_CPUACCT
+extern void cpuacct_charge(struct task_struct *, cputime_t cputime);
+#else
+static void inline cpuacct_charge(struct task_struct *p, cputime_t cputime) {}
+#endif
+
+#endif

Index: container-2.6.20-rc1/init/Kconfig
===================================================================
@@ -290,6 +290,13 @@ config PROC_PID_CPUSET
 	depends on CPUSETS
 	default y
 
+config CONTAINER_CPUACCT
+	bool "Simple CPU accounting container subsystem"
+	select CONTAINERS
+	help
+	  Provides a simple Resource Controller for monitoring the
+	  total CPU consumed by the tasks in a container
+
 config RELAY
 	bool "Kernel->user space relay support (formerly relayfs)"
 	help

Index: container-2.6.20-rc1/kernel/Makefile
===================================================================
@@ -38,6 +38,7 @@ obj-$(CONFIG_KEXEC) += kexec.o
 obj-$(CONFIG_COMPAT) += compat.o
 obj-$(CONFIG_CONTAINERS) += container.o
 obj-$(CONFIG_CPUSETS) += cpuset.o
+obj-$(CONFIG_CONTAINER_CPUACCT) += cpu_acct.o
 obj-$(CONFIG_IKCONFIG) += configs.o
 obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
 obj-$(CONFIG_AUDIT) += audit.o auditfilter.o

Index: container-2.6.20-rc1/kernel/sched.c
===================================================================
@@ -52,6 +52,7 @@
 #include <linux/tsacct_kern.h>
 #include <linux/kprobes.h>
 #include <linux/delayacct.h>
+#include <linux/cpu_acct.h>
 #include <asm/tlb.h>
 
 #include <asm/unistd.h>

@@ -3068,6 +3069,8 @@ void account_user_time(struct task_struc
 
 	p->utime = cputime_add(p->utime, cputime);
 
+	cpuacct_charge(p, cputime);
+
 	/* Add user time to cpustat. */
 	tmp = cputime_to_cputime64(cputime);
 	if (TASK_NICE(p) > 0)

@@ -3091,6 +3094,9 @@ void account_system_time(struct task_str
 
 	p->stime = cputime_add(p->stime, cputime);
 
+	if (p != rq->idle)
+		cpuacct_charge(p, cputime);
+
 	/* Add system time to cpustat. */
 	tmp = cputime_to_cputime64(cputime);
 	if (hardirq_count() - hardirq_offset)

--

Hi, Paul,

I have run into a problem running this patch on a powerpc box. Basically, the machine panics as soon as I mount the container filesystem with

mount -t container -oall container /<mount point>, I see

cpu 0x2: Vector: 300 (Data Access) at [c0000001e7f2b8e0]

    pc: c000000000098b70: .cpuacct_charge+0x84/0xbc
    lr: c000000000060a3c: .account_user_time+0x60/0xb4
    sp: c0000001e7f2bb60

msr: 8000000000009032
dar: 48
dsisr: 40000000
current = 0xc0000001e7f0e800
paca = 0xc00000000071c300
pid = 0, comm = swapper

Analyzing the dump a bit further lead me to container_subsys_state().

I am trying to figure out the reason for the panic and trying to find a fix. Since the introduction of whole hierarchy system, the debugging has gotten a bit harder and taking longer, hence I was wondering if you had any clues about the problem

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs

This is a multi-processor system?

My guess is that it's a race in the subsystem API that I've been meaning to deal with for some time - basically I've been using (<foo>_subsys.subsys_id != -1) to indicate that <foo> is ready for use, but there's a brief window during subsystem registration where that's not actually true.

I'll add an "active" field in the container_subsys structure, which isn't set until registration is completed, and subsystems should use that instead. container_register_subsys() will set it just prior to releasing callback_mutex, and cpu_acct.c (and other subsystems) will check <foo>_subsys.active rather than (<foo>_subsys.subsys_id != -1)

Yes, the multi-hierarchy support does make the whole code a little more complex - but people presented reasonable scenarios where a single container tree for all resource controllers just wasn't flexible enough.

Paul

Yes, it has 4 cpus

I tried something similar, I added an activated field, which is set to true when the ->create() callback is invoked. That did not help either, the machine still panic'ed.

I see the need for it, but I wonder if we should start with that right away. I understand that people might want to group cpusets differently from their grouping of let's say the cpu resource manager. I would still prefer to start with one hierarchy and then move to multiple hierarchies. I am concerned that adding complexity upfront might turn off people from using the infrastructure.

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs

I think that marking it active when create() is called may be too soon.

Is this with my unchanged cpuacct subsystem, or with the version that you've extended to track load over defined periods? I don't see it when I test under VMware (with two processors in the VM), but I suspect that's not going to be quite as parallel as a real SMP system.

That's what I had originally and people objected to the lack of flexibility :-)

The presence or absence of multiple hierarchies is pretty much exposed to userspace, and presenting the right interface to userspace is a fairly important thing to get right from the start.

Paul

This is with the unchanged cpuacct subsystem. Ok, so the container system needs to mark active internally then.

I understand that the features are exported to userspace. But from the userspace POV only the mount options change - right?

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs

The mount options, plus the fact that you can mount different instances of containerfs with different resource controller sets to get different hierarchies. (Multiple mounts with the same controller sets share the same superblock/hierarchy).

Paul

Hi, Paul,

I figured out the reason for the panic. Here are the fixes

Add mount and umount callbacks. These callbacks can be used by the controller to figure out the correct root container and also know whether the controller is currently acitve.

---

 include/linux/container.h |    2 ++
 kernel/container.c        |   12 ++++++++++++
 2 files changed, 14 insertions(+)

diff -puN include/linux/container.h~add-mount-callback include/linux/container.h
--- linux-2.6.20-rc3/include/linux/container.h~add-mount-callback 2007-01-12
21:23:00.000000000 +0530
+++ linux-2.6.20-rc3-balbir/include/linux/container.h 2007-01-12
21:23:00.000000000 +0530
@@ -171,6 +171,8 @@ struct container_subsys {
void (*exit)(struct container_subsys *ss, struct task_struct *task); int (*populate)(struct container_subsys *ss,
struct container *cont);
+ void (*mount)(struct container_subsys *ss, struct container *cont);
+ void (*umount)(struct container_subsys *ss, struct container *cont);

int subsys_id;
#define MAX_CONTAINER_TYPE_NAMELEN 32
diff -puN kernel/container.c~add-mount-callback kernel/container.c
--- linux-2.6.20-rc3/kernel/container.c~add-mount-callback 2007-01-12
21:23:00.000000000 +0530
+++ linux-2.6.20-rc3-balbir/kernel/container.c 2007-01-12 21:42:59.000000000 +0530
@@ -394,6 +394,7 @@ static void container_put_super(struct s
int i;
struct container *cont = &root->top_container; struct task_struct *g, *p;
+ struct container_subsys *ss;

root->sb = NULL;
sb->s_fs_info = NULL;
@@ -407,6 +408,11 @@ static void container_put_super(struct s

mutex_lock(&callback_mutex);

+ for_each_subsys(hierarchy, ss) {
+ if (ss->umount)
+ ss->umount(ss, cont);
+ }
+
/* Remove all tasks from this container hierarchy */ read_lock(&tasklist_lock);
do_each_thread(g, p) {
@@ -607,6 +613,12 @@ static int container_get_sb(struct file_
rcu_assign_pointer(subsys[i]->hierarchy,
hierarchy);
}
+
+ for_each_subsys(hierarchy, ss) {
+ if (ss->mount)
+ ss->mount(ss, cont);
+ }
+
mutex_unlock(&callback_mutex); synchronize_rcu();

_

Balbir Singh,
Linux Technology Center,
IBM Software Labs

PS: I hope my mailer does not word wrap the patches.

Here is the second patch and the real fix in sched.c

Fix coding style in cpuacct_charge()

In sched.c, account_user_time() can be called with the task p set to rq->idle. Since idle tasks do not belong to any container, this was causing a panic in task_ca() in cpu_acct.c.

Multiplying the time by 1000 is not correct in cpuusage_read(). The code has been converted to use the correct cputime API.

Add mount/umount callbacks.

---

 kernel/cpu_acct.c |   29 +++++++++++++++++++++++------
 kernel/sched.c    |   17 +++++++++++------
 2 files changed, 34 insertions(+), 12 deletions(-)

diff -puN kernel/cpu_acct.c~fix-cpuacct-panic-on-mount kernel/cpu_acct.c
--- linux-2.6.20-rc3/kernel/cpu_acct.c~fix-cpuacct-panic-on-mount 2007-01-15
14:23:20.000000000 +0530
+++ linux-2.6.20-rc3-balbir/kernel/cpu_acct.c 2007-01-15 14:23:20.000000000 +0530
@@ -22,6 +22,7 @@ struct cpuacct {
};

static struct container_subsys cpuacct_subsys;
+static struct container *root;

static inline struct cpuacct *container_ca(struct container *cont) {
@@ -49,6 +50,16 @@ static void cpuacct_destroy(struct conta
kfree(container_ca(cont));
}

+static void cpuacct_mount(struct container_subsys *ss, struct container *cont) +{
+ root = cont;
+}
+
+static void cpuacct_umount(struct container_subsys *ss, struct container *cont) +{
+ root = NULL;
+}
+
static ssize_t cpuusage_read(struct container *cont,
struct cftype *cft,
struct file *file,
@@ -57,6 +68,7 @@ static ssize_t cpuusage_read(struct cont
{
struct cpuacct *ca = container_ca(cont);
cputime64_t time;
+ unsigned long time_in_jiffies;
char usagebuf[64];
char *s = usagebuf;

@@ -64,9 +76,8 @@ static ssize_t cpuusage_read(struct cont
time = ca->time; spin_unlock_irq(&ca->lock);

- time *= 1000;
- do_div(time, HZ);
- s += sprintf(s, "%llu", (unsigned long long) time);
+ time_in_jiffies = cputime_to_jiffies(time);
+ s += sprintf(s, "%llu\n", (unsigned long long) time_in_jiffies);

return simple_read_from_buffer(buf, nbytes, ppos, usagebuf, s - usagebuf);
}
@@ -83,12 +94,13 @@ static int cpuacct_populate(struct conta
}

-void cpuacct_charge(struct task_struct *task, cputime_t cputime) { +void cpuacct_charge(struct task_struct *task, cputime_t cputime) +{

struct cpuacct *ca;
unsigned long flags;

- if (cpuacct_subsys.subsys_id < 0) return;
+ if (cpuacct_subsys.subsys_id < 0 || !root) return;
rcu_read_lock();
ca = task_ca(task);
if (ca) {
@@ -104,13 +116,18 @@ static struct container_subsys cpuacct_s
.create = cpuacct_create,
.destroy = cpuacct_destroy,
.populate = cpuacct_populate,
+ .mount = cpuacct_mount,
+ .umount = cpuacct_umount,
.subsys_id = -1,
};

int __init init_cpuacct(void)
{
- int id = container_register_subsys(&cpuacct_subsys);
+ int id;
+
+ root = NULL;
+ id = container_register_subsys(&cpuacct_subsys);
return id < 0 ? id : 0;
}

diff -puN kernel/sched.c~fix-cpuacct-panic-on-mount kernel/sched.c
--- linux-2.6.20-rc3/kernel/sched.c~fix-cpuacct-panic-on-mount 2007-01-15
14:23:20.000000000 +0530
+++ linux-2.6.20-rc3-balbir/kernel/sched.c 2007-01-15 14:23:20.000000000 +0530
@@ -3067,10 +3067,17 @@ void account_user_time(struct task_struc
{
struct cpu_usage_stat *cpustat = &kstat_this_cpu.cpustat; cputime64_t tmp;
+ struct rq *rq = this_rq();

p->utime = cputime_add(p->utime, cputime);

- cpuacct_charge(p, cputime);
+ /*
+ * On powerpc this routine can be called with p set to the idle
+ * task of the cpu. idle tasks don't really belong to any
+ * container.
+ */
+ if (p != rq->idle)
+ cpuacct_charge(p, cputime);

/* Add user time to cpustat. */
tmp = cputime_to_cputime64(cputime);
@@ -3095,18 +3102,16 @@ void account_system_time(struct task_str

p->stime = cputime_add(p->stime, cputime);

- if (p != rq->idle)
- cpuacct_charge(p, cputime);
-
/* Add system time to cpustat. */
tmp = cputime_to_cputime64(cputime);
if (hardirq_count() - hardirq_offset)
cpustat->irq = cputime64_add(cpustat->irq, tmp);
else if (softirq_count())
cpustat->softirq = cputime64_add(cpustat->softirq, tmp);
- else if (p != rq->idle)
+ else if (p != rq->idle) {
cpustat->system = cputime64_add(cpustat->system, tmp);
- else if (atomic_read(&rq->nr_iowait) > 0)
+ cpuacct_charge(p, cputime);
+ } else if (atomic_read(&rq->nr_iowait) > 0)
cpustat->iowait = cputime64_add(cpustat->iowait, tmp);
else
cpustat->idle = cputime64_add(cpustat->idle, tmp);
_

Balbir Singh
Linux Technology Center
Bangalore, IBM ISTL

How come that didn't cause a problem on x86_64? If this is an inconsistency between architectures then perhaps it ought to be cleaned up.

Additionally, I think that we should make the idle tasks members of the root container(s), to remove this special case. (I'm a bit surprised that they're not already - I thought that the early container initialization was early enough that the idle tasks hadn't yet been forked. Is that different on PowerPC?

Thanks.

I'm not sure I like the mount/unmount callbacks. What exactly are you trying to gain from them? My intention was that the

cont->subsys[i]->container = cont;

line in container_get_sb() was doing essentially this - i.e. the container_subsys_state for the root container in a subsystem is already kept up to date by the container system, and the subsystem can rely on the "container" field in the container_subsys_state.

Thanks,

Paul

That is because account_system/user_time() is also called from account_process_vtime() which is called from __switch_to in power pc. vtime is for virtual time accounting. Enabled by CONFIG_VIRT_CPU_ACCOUNTING.

idle threads are associated only with the runqueue and not visible by the do_each_thread()/while_each_thread() loop. They are not added to the tasklist (please see init_idle() in kernel/sched.c).

While writing/extending the cpuacct container, I found it useful to know if the container resource group we are controlling is really mounted. Controllers can try and avoid doing work when not mounted and start when the subsystem is mounted. Also, without these callbacks, one has no definite way of checking if the top_container is dummy or for real.

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs

That's somewhat intentional - my aim was that the controllers shouldn't really care whether they're connected to the default hierarchy or have been bound to some mounted hierarchy. Having said thay, they can determine it by checking <foo>_subsys.hierarchy if they really want to. If that's 0 then they're in the default hierarchy (and can assume that all tasks are in one top-level container).

Paul

That makes sense, the only additional thing required is to know when the subsystem really got mounted (we cannot keep polling hierarchy for it:-))

	Balbir Singh,
	Linux Technology Center,
	IBM Software Labs

This patch provides the RG core and numtasks controller as container subsystems, intended as an example of how to implement a more complex resource control system over generic process containers. The changes to the core involve primarily removing the group management, task membership and configfs support and adding interface layers to talk to the generic container layer instead.

Each resource controller becomes an independent container subsystem; the RG core is essentially a library that the resource controllers can use to provide the RG API to userspace. Rather than a single shares and stats file in each group, there's a <controller>_shares and a <controller>_stats file, each linked to the appropriate resource controller.

 include/linux/moduleparam.h  |   12 -
 include/linux/numtasks.h     |   28 ++
 include/linux/res_group.h    |   87 ++++++++
 include/linux/res_group_rc.h |   97 ++++++++
 init/Kconfig                 |   22 ++
 kernel/Makefile              |    1 
 kernel/fork.c                |    7 
 kernel/res_group/Makefile    |    2 
 kernel/res_group/local.h     |   38 +++
 kernel/res_group/numtasks.c  |  467 +++++++++++++++++++++++++++++++++++++++++++
 kernel/res_group/res_group.c |  160 ++++++++++++++
 kernel/res_group/rgcs.c      |  302 +++++++++++++++++++++++++++
 kernel/res_group/shares.c    |  228 ++++++++++++++++++++
 13 files changed, 1447 insertions(+), 4 deletions(-)

Index: container-2.6.20-rc1/include/linux/moduleparam.h
===================================================================
@@ -78,11 +78,17 @@ struct kparam_array
 /* Helper functions: type is byte, short, ushort, int, uint, long,
    ulong, charp, bool or invbool, or XXX if you define param_get_XXX,
    param_set_XXX and param_check_XXX. */
-#define module_param_named(name, value, type, perm)			   \
-	param_check_##type(name, &(value));				   \
-	module_param_call(name, param_set_##type, param_get_##type, &value, perm); \
+#define module_param_named_call(name, value, type, set, perm)		\
+	param_check_##type(name, &(value));				\
+	module_param_call(name, set, param_get_##type, &(value), perm); \
 	__MODULE_PARM_TYPE(name, #type)
 
+#define module_param_named(name, value, type, perm)			   \
+	module_param_named_call(name, value, type, param_set_##type, perm)
+
+#define module_param_set_call(name, type, setfn, perm) \
+	module_param_named_call(name, name, type, setfn, perm)
+
 #define module_param(name, type, perm)				\
 	module_param_named(name, name, type, perm)
 

Index: container-2.6.20-rc1/include/linux/numtasks.h
===================================================================
@@ -0,0 +1,28 @@
+/* numtasks.h - No. of tasks resource controller for Resource Groups
+ *
+ * Copyright (C) Chandra Seetharaman, IBM Corp. 2003, 2004, 2005
+ *
+ * Provides No. of tasks resource controller for Resource Groups
+ *
+ * Latest version, more details at http://ckrm.sf.net
+ *
+ * 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.
+ *
+ */
+#ifndef _LINUX_NUMTASKS_H
+#define _LINUX_NUMTASKS_H
+
+#ifdef CONFIG_RES_GROUPS_NUMTASKS
+#include <linux/res_group_rc.h>
+
+extern int numtasks_allow_fork(struct task_struct *);
+
+#else /* CONFIG_RES_GROUPS_NUMTASKS */
+
+#define numtasks_allow_fork(task) (0)
+
+#endif /* CONFIG_RES_GROUPS_NUMTASKS */
+#endif /* _LINUX_NUMTASKS_H */

Index: container-2.6.20-rc1/init/Kconfig
===================================================================
@@ -341,6 +341,28 @@ config TASK_IO_ACCOUNTING
 
 	  Say N if unsure.
 
+menu "Resource Groups"
+
+config RES_GROUPS
+	bool "Resource Groups"
+	depends on EXPERIMENTAL
+	select CONTAINERS
+	help
+	  Resource Groups is a framework for controlling and monitoring
+	  resource allocation of user-defined groups of tasks. For more
+	  information, please visit http://ckrm.sf.net.
+
+config RES_GROUPS_NUMTASKS
+	bool "Number of Tasks Resource Controller"
+	depends on RES_GROUPS
+	default y
+	help
+	  Provides a Resource Controller for Resource Groups that allows
+	  limiting number of tasks a resource group can have.
+
+	  Say N if unsure, Y to use the feature.
+
+endmenu
 config SYSCTL
 	bool
 

Index: container-2.6.20-rc1/kernel/Makefile
===================================================================
@@ -53,6 +53,7 @@ obj-$(CONFIG_RELAY) += relay.o
 obj-$(CONFIG_UTS_NS) += utsname.o
 obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
 obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
+obj-$(CONFIG_RES_GROUPS) += res_group/
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is

Index: container-2.6.20-rc1/kernel/fork.c
===================================================================
@@ -49,6 +49,7 @@
 #include <linux/delayacct.h>
 #include <linux/taskstats_kern.h>
 #include <linux/random.h>
+#include <linux/numtasks.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>

@@ -1355,7 +1356,7 @@ long do_fork(unsigned long clone_flags,
 	      int __user *child_tidptr)
 {
 	struct task_struct *p;
-	int trace = 0;
+	int trace = 0, rc;
 	struct pid *pid = alloc_pid();
 	long nr;
 

@@ -1368,6 +1369,10 @@ long do_fork(unsigned long clone_flags,
 			clone_flags |= CLONE_PTRACE;
 	}
 
+	rc = numtasks_allow_fork(current);
+	if (rc)
+		return rc;
+
 	p = copy_process(clone_flags, stack_start, regs, stack_size, parent_tidptr, child_tidptr, nr);
 	/*
 	 * Do this prior waking up the new thread - the thread pointer

Index: container-2.6.20-rc1/kernel/res_group/Makefile
===================================================================
@@ -0,0 +1,2 @@
+obj-y = res_group.o shares.o rgcs.o
+obj-$(CONFIG_RES_GROUPS_NUMTASKS) += numtasks.o

Index: container-2.6.20-rc1/kernel/res_group/local.h
===================================================================
@@ -0,0 +1,38 @@
+/*
+ * Contains function definitions that are local to the Resource Groups.
+ * NOT to be included by controllers.
+ */
+
+#include <linux/res_group_rc.h>
+
+extern struct res_controller *get_controller_by_name(const char *);
+extern struct res_controller *get_controller_by_id(unsigned int);
+extern void put_controller(struct res_controller *);
+extern struct resource_group *alloc_res_group(struct resource_group *,
+							const char *);
+extern int free_res_group(struct resource_group *);
+extern void release_res_group(struct kref *);
+extern int set_controller_shares(struct resource_group *,
+			struct res_controller *, const struct res_shares *);
+/* Set shares for the given resource group and resource to default values */
+extern void set_shares_to_default(struct resource_group *,
+						struct res_controller *);
+extern void res_group_teardown(void);
+extern int set_res_group(pid_t, struct resource_group *);
+extern void move_tasks_to_parent(struct resource_group *);
+
+ssize_t res_group_file_read(struct container *cont,
+			    struct cftype *cft,
+			    struct file *file,
+			    char __user *buf,
+			    size_t nbytes, loff_t *ppos);
+ssize_t res_group_file_write(struct container *cont,
+			     struct cftype *cft,
+			     struct file *file,
+			     const char __user *userbuf,
+			     size_t nbytes, loff_t *ppos);
+
+enum {
+	RG_FILE_SHARES,
+	RG_FILE_STATS,
+};

--

This patch implements the BeanCounter resource control abstraction over generic process containers. It contains the beancounter core code, plus the numfiles resource counter. It doesn't currently contain any of the memory tracking code or the code for switching beancounter context in interrupts.

Currently all the beancounters resource counters are lumped into a single hierarchy; ideally it would be possible for each resource counter to be a separate container subsystem, allowing them to be connected to different hierarchies.

---
 fs/file_table.c          |   11 +
 include/bc/beancounter.h |  192 ++++++++++++++++++++++++
 include/bc/misc.h        |   27 +++
 include/linux/fs.h       |    3 
 init/Kconfig             |    4 
 init/main.c              |    3 
 kernel/Makefile          |    1 
 kernel/bc/Kconfig        |   17 ++
 kernel/bc/Makefile       |    7 
 kernel/bc/beancounter.c  |  371 +++++++++++++++++++++++++++++++++++++++++++++++
 kernel/bc/misc.c         |   56 +++++++
 11 files changed, 691 insertions(+), 1 deletion(-)

Index: container-2.6.20-rc1/init/Kconfig
===================================================================
@@ -619,6 +619,10 @@ config STOP_MACHINE
 	  Need stop_machine() primitive.
 endmenu
 
+menu "Beancounters"
+source "kernel/bc/Kconfig"
+endmenu
+
 menu "Block layer"
 source "block/Kconfig"
 endmenu

Index: container-2.6.20-rc1/kernel/Makefile
===================================================================
@@ -12,6 +12,7 @@ obj-y     = sched.o fork.o exec_domain.o
 
 obj-$(CONFIG_STACKTRACE) += stacktrace.o
 obj-y += time/
+obj-$(CONFIG_BEANCOUNTERS) += bc/
 obj-$(CONFIG_DEBUG_MUTEXES) += mutex-debug.o
 obj-$(CONFIG_LOCKDEP) += lockdep.o
 ifeq ($(CONFIG_PROC_FS),y)

Index: container-2.6.20-rc1/kernel/bc/Kconfig
===================================================================
@@ -0,0 +1,17 @@
+config BEANCOUNTERS
+	bool "Enable resource accounting/control"
+	default n
+	select CONTAINERS
+	help
+	  When Y this option provides accounting and allows configuring
+	  limits for user's consumption of exhaustible system resources.
+	  The most important resource controlled by this patch is unswappable
+	  memory (either mlock'ed or used by internal kernel structures and
+	  buffers). The main goal of this patch is to protect processes
+	  from running short of important resources because of accidental
+	  misbehavior of processes or malicious activity aiming to ``kill''
+	  the system. It's worth mentioning that resource limits configured
+	  by setrlimit(2) do not give an acceptable level of protection
+	  because they cover only a small fraction of resources and work on a
+	  per-process basis.  Per-process accounting doesn't prevent malicious
+	  users from spawning a lot of resource-consuming processes.

Index: container-2.6.20-rc1/kernel/bc/Makefile
===================================================================
@@ -0,0 +1,7 @@
+#
+# kernel/bc/Makefile
+#
+# Copyright (C) 2006 OpenVZ SWsoft Inc.
+#
+
+obj-y = beancounter.o misc.o

Index: container-2.6.20-rc1/init/main.c
===================================================================
@@ -55,6 +55,8 @@
 #include <linux/pid_namespace.h>
 #include <linux/compile.h>
 
+#include <bc/beancounter.h>
+
 #include <asm/io.h>
 #include <asm/bugs.h>
 #include <asm/setup.h>

@@ -494,6 +496,7 @@ asmlinkage void __init start_kernel(void
 	char * command_line;
 	extern struct kernel_param __start___param[], __stop___param[];
 
+	bc_init_early();
 	smp_setup_processor_id();
 
 	/*

Index: container-2.6.20-rc1/include/bc/misc.h
===================================================================
@@ -0,0 +1,27 @@
+/*
+ * include/bc/misc.h
+ *
+ * Copyright (C) 2006 OpenVZ SWsoft Inc
+ *
+ */
+
+#ifndef __BC_MISC_H__
+#define __BC_MISC_H__
+
+struct file;
+
+#ifdef CONFIG_BEANCOUNTERS
+int __must_check bc_file_charge(struct file *);
+void bc_file_uncharge(struct file *);
+#else
+static inline int __must_check bc_file_charge(struct file *f)
+{
+	return 0;
+}
+
+static inline void bc_file_uncharge(struct file *f)
+{
+}
+#endif
+
+#endif

Index: container-2.6.20-rc1/fs/file_table.c
===================================================================
@@ -22,6 +22,8 @@
 #include <linux/sysctl.h>
 #include <linux/percpu_counter.h>
 
+#include <bc/misc.h>
+
 #include <asm/atomic.h>
 
 /* sysctl tunables... */

@@ -43,6 +45,7 @@ static inline void file_free_rcu(struct 
 static inline void file_free(struct file *f)
 {
 	percpu_counter_dec(&nr_files);
+	bc_file_uncharge(f);
 	call_rcu(&f->f_u.fu_rcuhead, file_free_rcu);
 }
 

@@ -107,8 +110,10 @@ struct file *get_empty_filp(void)
 	if (f == NULL)
 		goto fail;
 
-	percpu_counter_inc(&nr_files);
 	memset(f, 0, sizeof(*f));
+	if (bc_file_charge(f))
+		goto fail_charge;
+	percpu_counter_inc(&nr_files);
 	if (security_file_alloc(f))
 		goto fail_sec;
 

@@ -135,6 +140,10 @@ fail_sec:
 	file_free(f);
 fail:
 	return NULL;
+
+ fail_charge:
+	kmem_cache_free(filp_cachep, f);
+	return NULL;
 }
 
 EXPORT_SYMBOL(get_empty_filp);

Index: container-2.6.20-rc1/include/linux/fs.h
===================================================================
@@ -738,6 +738,9 @@ struct file {
 	spinlock_t		f_ep_lock;
 #endif /* #ifdef CONFIG_EPOLL */
 	struct address_space	*f_mapping;
+#ifdef CONFIG_BEANCOUNTERS
+	struct beancounter      *f_bc;
+#endif
 };
 extern spinlock_t files_lock;
 #define file_list_lock() spin_lock(&files_lock);

--

I don't like it, it looks bloated and probably adds plenty of overhead (similar to the OVZ implementation where this seems to be taken from) here are some comments/questions:

Yes - perhaps I should have been clearer in the patch description. It's basically code taken from the OpenVZ bean counters patches that have been posted recently, but with the filesystem and process tracking code ripped out (since it's implemented over the generic containers).

The main point of this patch is to demonstrate that UBC can be implemented effectively over generic containers, rather than to be a proposal in favour of UBC versus any of the other potential resource control mechanisms.

Most of your comments are about code taken pretty much directly from the UBC patches, so I won't address them.

The locking model (taken originally from the Cpusets code) in generic containers is that while you can use RCU to guarantee that a pointer read from current->container remains valid until you exit the RCU critical section, if you want to make consistent changes to data referenced from a task P's container, you need to hold either P->alloc_lock or one of the two container mutexes (manage_mutex and/or callback_mutex).

In this particular case (sorry, not on the VPN right now to be able to figure out the potential code changes) the fact that the call to css_get_current() uses atomic operations (currently a spinlock, but I suspect I could optimize it to be a cmpxchg) could mean that we can skip the task_lock(), at the cost of occasionally accounting a file to the container that the task had just left.

Paul

Herbert,

Look like you have commented anything, but OpenVZ :)
sure you don't like it, cause it doesn't add racy dcache accounting and works :) speaking about overhead: have you done a single measurement of BC code?

1. some resources can contribute to multiple BC params, thus need to be accounted atomically into 2 counters.
2. spin_lock()/spin_unlock() has the same 1 lock operation on SMP on most archs
3. using atomic counters you can't get a snapshot of current usages.
4. all the performance critical resources should be handled with pre-charges, as it is done in TCP/IP accounting in mainstream and as it is done for files/kmemsize in OpenVZ.

<<<< cause it simply doesn't worth optimizing.
its overhead is minor compared to accounting itself (atomic operations).

in this patchset it is not used AFAICS.
however, it was taken from the full BC patch where it is used to handle resource denials as most gracefully as possible.

<<<< I guess it's countainers patch issue...

Agree with this. This patch doesn't prove that BCs can be integrated to the containers infrastructure.

Do you see any problems with these except for not-needed-anywhere-now? P)

Kirill

What concerns do you have in particular? Are there any changes that you'd like to see?

Paul

FULL BC patch w/o pages fractions accounting doesn't add any noticeable overhead to mainstream kernel. Pages fractions accounting will be optimized as well. The part you're talking about is only 1/100 of the complete patch.

Because approach

if (atomic_read(&bc->barrier) > aromic_read(&bc->held))

        atomic_inc(&bc->held);

used in vserver accounting is not atomic ;)

Look at the comment below about charging two resources at once.

Let's look at

    if (parm->held < parm->barrier &&
             bc_resources[res]->bcr_barrier_hit)
         bc_resources[res]->bcr_barrier_hit(bc);

code one more time.

In case of BC_LIMIT charge BC code informs resource controller about BARRIER hit to take some actions before hard resource shortage.

This came from containers patches. BC code doesn't take locks on fast paths.

We do know it and we have "pre-charges" optimization for frequent calls. bc->lock we've seen is used to make two or more resources charge in only one atomic operation, that is faster than doing atomic_inc() for each resource as you've proposed above.

I promise, that if Linus will include patch that adds a syscall to open 64 or "as many as available whatever is smaller" files at once we'll add this functionality.

From: Serge E. Hallyn <[email protected]>
Subject: [RFC] [PATCH 1/1] container: define a namespace container subsystem

Here's a stab at a namespace container subsystem based on Paul Menage's containers patch, just to experiment with how semantics suit what we want.

A few things we'll want to address:

1. We'll want to be able to hook things like rmdir, so that we can rm -rf /containers/vserver1 to kill all processes in that container and all child containers.

2. We need a semantic difference between attaching to a container, and being the first to join the container you just created.

3. We will want to be able to give the container attach function more info, so that we can ask to attach to just the network namespace, but none of the others, in the container we're attaching to.

I'm sure there'll be more, but that's a start...

Note that this is far more user-controled than my previous namespace naming patch. In particular, ns actions - clone/unshare - do not automatically create new containers. That may be for the best, so I didn't try to move in that direction this time. However it may be desirable to at least change the creation semantics such that (after a container create request) an unshare or clone simultaneously creates the container and joins the new process as the container's first process.

This version does not point to an nsproxy from the ns_container, but it probably should, as a definitive way to pick an nsproxy to attach to if a process wants to enter the container.

---
 init/Kconfig          |    9 ++++++
 kernel/Makefile       |    1 +
 kernel/ns_container.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 0 deletions(-)

index ebaec57..c00b19c 100644
@@ -297,6 +297,15 @@ config CONTAINER_CPUACCT
 	  Provides a simple Resource Controller for monitoring the
 	  total CPU consumed by the tasks in a container
 
+config CONTAINER_NS
+	bool "Namespace container subsystem"
+	select CONTAINERS
+	help
+	  Provides a simple namespace container subsystem to
+	  provide hierarchical naming of sets of namespaces,
+	  for instance virtual servers and checkpoint/restart
+	  jobs.
+
 config RELAY
 	bool "Kernel->user space relay support (formerly relayfs)"
 	help

index feba860..6c73a5e 100644
@@ -39,6 +39,7 @@ obj-$(CONFIG_COMPAT) += compat.o
 obj-$(CONFIG_CONTAINERS) += container.o
 obj-$(CONFIG_CPUSETS) += cpuset.o
 obj-$(CONFIG_CONTAINER_CPUACCT) += cpu_acct.o
+obj-$(CONFIG_CONTAINER_NS) += ns_container.o
 obj-$(CONFIG_IKCONFIG) += configs.o
 obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
 obj-$(CONFIG_AUDIT) += audit.o auditfilter.o

1.4.1

Hi Serge,

Thanks for looking at this.

What you have here is the basic boilerplate for any generic container subsystem. I realise that my current containers patch has some incompatibilities with the way that nsproxy wants to work.

The current model is that rmdir fails if there are any processes still in the container; so you'd have to kill processes by looking for pids in the "tasks" info file. This was behaviour inherited from the cpusets code; I'd be open to making this more configurable (e.g. specifying that rmdir should try to kill any remaining tasks).

Right - the way to do this would probably be some kind of "container_clone()" function that duplicates the properties of the current container in a child, and immediately moves the current process into that container.

If you want to be able to attach to different namespaces separately, then possibly they should be separate container subsystems?

Paul

In retrospect I don't like the changes in behavior. So my next version will aim for closer to the original (non-containerfs) behavior.

Ok - of course I suspect I'll have to just start coding away before i can guess at what help I might need from your code.

That's one possibility, but imo somewhat unpalatable.

As I mentioned in the last email, I really like the idea of having files representing each namespace under each namespace container directory, creating a new container by linking some of those namespace files, and entering containers by echoing the pathname to the new container into /proc/$$/ns_container. (either upon the echo, or, I think preferably, upon a subsequent exec)

-serge