OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/6] containers: Generic Process Containers (V6)
items: [PATCH 1/6] containers: Generic container system abstracted from cpusets code
items: [PATCH 2/6] containers: Cpusets hooked into containers
items: [PATCH 3/6] containers: Add generic multi-subsystem API to containers
items: [PATCH 4/6] containers: Simple CPU accounting container subsystem
items: [PATCH 5/6] containers: Resource Groups over generic containers
items: [PATCH 6/6] containers: BeanCounters over generic process containers
items: Re: [PATCH 6/6] containers: BeanCounters over generic process containers
items: Re: [PATCH 1/6] containers: Generic container system abstracted from cpusets code
items: Re: [PATCH 0/6] containers: Generic Process Containers (V6)
items: Re: [ckrm-tech] [PATCH 4/6] containers: Simple CPU accounting container subsystem
items: Re: [ckrm-tech] [PATCH 3/6] containers: Add generic multi-subsystem API to containers
items: [PATCH 0/1] Add mount/umount callbacks to containers (Re: [ckrm-tech] [PATCH 4/6] containers: Simple
items: [PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cleanups (Re: [ckrm
items: Re: [PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cleanups (Re: [
items: Re: [ckrm-tech] [PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cle

[PATCH 0/6] containers: Generic Process Containers (V6)
type: item
description: -- 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. rathe…
creator: Paul Menage
date: 2006-12-22T14:14:42-00:00

[PATCH 1/6] containers: Generic container system abstracted from cpusets code
type: item
description: 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. Signed-off-by: Paul Menage <menage@google.com> --- 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 ============================================================ ======= --- container-2.6.20-rc1.orig/fs/proc/base.c +++ 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/container.h ============================================================ ======= --- /dev/null +++ container-2.6.20-rc1/include/linux/container.h @@ -0,0 +1,96 @@ +#ifndef _LINUX_CONTAINER_H +#define _LINUX_CONTAINER_H +/* + * container interface + * + * C…
creator: Paul Menage
date: 2006-12-22T14:14:43-00:00

[PATCH 2/6] containers: Cpusets hooked into containers
type: item
description: 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. Signed-off-by: Paul Menage <menage@google.com> --- 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 ============================================================ ======= --- container-2.6.20-rc1.orig/include/linux/container.h +++ 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 ============================================================ ======= --- container-2.6.20-rc1.orig/include/linux/cpuset.h +++ 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 i…
creator: Paul Menage
date: 2006-12-22T14:14:44-00:00

[PATCH 3/6] containers: Add generic multi-subsystem API to containers
type: item
description: 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. Signed-off-by: Paul Menage <menage@google.com> --- 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 ============================================================ ======= --- container-2.6.20-rc1.orig/include/linux/container.h +++ 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 c…
creator: Paul Menage
date: 2006-12-22T14:14:45-00:00

[PATCH 4/6] containers: Simple CPU accounting container subsystem
type: item
description: 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. Signed-off-by: Paul Menage <menage@google.com> --- 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 ============================================================ ======= --- /dev/null +++ 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 ============================================================ ======= --- container-2.6.20-rc1.orig/init/Kconfig +++ 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/cpu_acct.c ============================================================ ======= --- /dev/null +++ container-2.6.20-rc1/kernel/cpu_acct.c @@ -0,0 +1,117 @@ +/* + * kernel/cpu_acct.c - CPU accounting container subsystem + * + * Copyright (C) Google Inc, 2006 + * + */ + +/* + * Container subsystem for reporting total CPU usage of tasks in a + * container. + */ + +#include <linux/module.h> +#include <linux/contai…
creator: Paul Menage
date: 2006-12-22T14:14:46-00:00

[PATCH 5/6] containers: Resource Groups over generic containers
type: item
description: 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 ============================================================ ======= --- container-2.6.20-rc1.orig/include/linux/moduleparam.h +++ 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_…
creator: Paul Menage
date: 2006-12-22T14:14:47-00:00

[PATCH 6/6] containers: BeanCounters over generic process containers
type: item
description: 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 ============================================================ ======= --- container-2.6.20-rc1.orig/init/Kconfig +++ 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 ============================================================ ======= --- container-2.6.20-rc1.orig/kernel/Makefile +++ 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 ============================================================ ======= --- /dev/null +++ container-2.6.20-rc1/kernel/bc/Kconfig @@ -0,0 +1,17 @@ +config BEANCOUNTERS + bool "Enable resource account…
creator: Paul Menage
date: 2006-12-22T14:14:48-00:00

Re: [PATCH 6/6] containers: BeanCounters over generic process containers
type: item
description: On Fri, Dec 22, 2006 at 06:14:48AM -0800, Paul Menage wrote: > 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. 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: > 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. > > +static inline void bc_uncharge(struct beancounter *bc, int res_id, > + unsigned long val) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&bc->bc_lock, flags); > + bc_uncharge_locked(bc, res_id, val); > + spin_unlock_irqrestore(&bc->bc_lock, flags); why use a spinlock, when we could use atomic counters? > +int bc_charge_locked(struct beancounter *bc, int res, unsigned long val, > + int strict, unsigned long flags) > +{ > + struct bc_resource_parm *parm; > + unsigned long new_held; > + > + BUG_ON(val > BC_MAXVALUE); > + > + parm = &bc->bc_parms[res]; > + new_held = parm->held + val; > + > + switch (strict) { > + case BC_LIMIT: > + if (new_held > parm->limit) > + break; > + /* fallthrough */ > + case BC_BARRIER: > + if (new_held > parm->barrier) { > + if (strict == BC_BARRIER) > + break; > + if (parm->held < parm->barrier && > + bc_resources[res]->bcr_barrier_hit) > + bc_resources[res]->bcr_barrier_hit(bc); > + } why do barrier checks with every accounting? there are probably a few cases where the checks could be independant from the accounting > + /* fallthrough */ > + case BC_FORCE: > + parm->held = new_held; > + bc_adjust_maxheld(parm); in what cases do we want to cross the barrier?…
creator: Herbert Poetzl
date: 2006-12-23T19:49:55-00:00

Re: [PATCH 6/6] containers: BeanCounters over generic process containers
type: item
description: On 12/23/06, Herbert Poetzl <herbert@13thfloor.at> wrote: > On Fri, Dec 22, 2006 at 06:14:48AM -0800, Paul Menage wrote: > > 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. > > 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) 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. > > > +int bc_file_charge(struct file *file) > > +{ > > + int sev; > > + struct beancounter *bc; > > + > > + task_lock(current); > > why do we lock current? it won't go away that > easily, and for switching the bc, it might be > better to use RCU or a separate lock, no? > 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) …
creator: Paul Menage
date: 2006-12-24T11:32:25-00:00

Re: [PATCH 6/6] containers: BeanCounters over generic process containers
type: item
description: Herbert, >>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. > > > 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: 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? >>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. >> >>+static inline void bc_uncharge(struct beancounter *bc, int res_id, >>+ unsigned long val) >>+{ >>+ unsigned long flags; >>+ >>+ spin_lock_irqsave(&bc->bc_lock, flags); >>+ bc_uncharge_locked(bc, res_id, val); >>+ spin_unlock_irqrestore(&bc->bc_lock, flags); > > > why use a spinlock, when we could use atomic > counters? 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. >>+int bc_charge_locked(struct beancounter *bc, int res, unsigned long val, >>+ int strict, unsigned long flags) >>+{ >>+ struct bc_resource_parm *parm; >>+ unsigned long new_held; >>+ >>+ BUG_ON(val > BC_MAXVALUE); >>+ >>+ parm = &bc->bc_parms[res]; >>+ new_held = parm->held + val; >>+ …
creator: dev
date: 2006-12-25T10:16:48-00:00

Re: [PATCH 6/6] containers: BeanCounters over generic process containers
type: item
description: Herbert Poetzl wrote: > On Fri, Dec 22, 2006 at 06:14:48AM -0800, Paul Menage wrote: >> 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. > > 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) 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. > here are some comments/questions: > >> 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. >> >> +static inline void bc_uncharge(struct beancounter *bc, int res_id, >> + unsigned long val) >> +{ >> + unsigned long flags; >> + >> + spin_lock_irqsave(&bc->bc_lock, flags); >> + bc_uncharge_locked(bc, res_id, val); >> + spin_unlock_irqrestore(&bc->bc_lock, flags); > > why use a spinlock, when we could use atomic > counters? 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. > >> +int bc_charge_locked(struct beancounter *bc, int res, unsigned long val, >> + int strict, unsigned long flags) >> +{ >> + struct bc_resource_parm *parm; >> + unsigned long new_held; >> + >> + BUG_ON(val > BC_MAXVALUE); >> + >> + parm = &bc->bc_parms[res]; >> + new_held = parm->held + val; >> + >> + switch (strict) { >> + case BC_LIMIT: >> + if (new_held > parm->limit) >> + break; >> + /* fallthrough …
creator: Pavel Emelianov
date: 2006-12-25T10:35:15-00:00

Re: [PATCH 6/6] containers: BeanCounters over generic process containers
type: item
description: On 12/25/06, Kirill Korotaev <dev@sw.ru> wrote: > > also note that certain limits are much more > > complicated than the (very simple) file limits > > and the code will be called at higher frequency > Agree with this. This patch doesn't prove that BCs can be integrated to the > containers infrastructure. What concerns do you have in particular? Are there any changes that you'd like to see? Paul _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Paul Menage
date: 2006-12-26T00:54:13-00:00

Re: [PATCH 1/6] containers: Generic container system abstracted from cpusets code
type: item
description: Paul Menage <menage@google.com> writes: > 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. 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 Kl…
creator: ebiederm
date: 2006-12-30T13:10:32-00:00

Re: [PATCH 1/6] containers: Generic container system abstracted from cpusets code
type: item
description: Eric wrote: > 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. 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. > Although I am curious why this is even needed when > we have /proc/<pid>/cpuset which gets us the information > in another way. 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. > 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. 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…
creator: Paul Jackson
date: 2006-12-31T05:17:56-00:00

Re: [PATCH 1/6] containers: Generic container system abstracted from cpusets code
type: item
description: On 12/30/06, Eric W. Biederman <ebiederm@xmission.com> wrote: > Paul Menage <menage@google.com> writes: > > > 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. > > 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 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. > > 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. OK, I'll take a look at that. > > 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 ta…
creator: Paul Menage
date: 2007-01-02T22:15:03-00:00

Re: [PATCH 0/6] containers: Generic Process Containers (V6)
type: item
description: From: Serge E. Hallyn <serue@us.ibm.com> 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. Signed-off-by: Serge E. Hallyn <serue@us.ibm.com> --- init/Kconfig | 9 ++++++ kernel/Makefile | 1 + kernel/ns_container.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 0 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index ebaec57..c00b19c 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -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 CONTA…
creator: serue
date: 2007-01-03T14:43:30-00:00

Re: [PATCH 0/6] containers: Generic Process Containers (V6)
type: item
description: Hi Serge, On 1/3/07, Serge E. Hallyn <serue@us.ibm.com> wrote: > From: Serge E. Hallyn <serue@us.ibm.com> > 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. 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. > > 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. 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). > > 2. We need a semantic difference between attaching > to a container, and being the first to join the > container you just created. 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. > > 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. If you want to be able to attach to different namespaces separately, then possibly they should be separate container subsystems? Paul
creator: Paul Menage
date: 2007-01-05T00:25:59-00:00

Re: [ckrm-tech] [PATCH 4/6] containers: Simple CPU accounting container subsystem
type: item
description: Paul Menage wrote: > 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. > > Signed-off-by: Paul Menage <menage@google.com> > 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
creator: Balbir Singh
date: 2007-01-10T14:21:41-00:00

Re: [ckrm-tech] [PATCH 3/6] containers: Add generic multi-subsystem API to containers
type: item
description: Paul Menage wrote: > +/* The set of hierarchies in use. Hierarchy 0 is the "dummy > + * container", reserved for the subsystems that are otherwise > + * unattached - it never has more than a single container, and all > + * tasks are part of that container. */ > + > +static struct containerfs_root rootnode[CONFIG_MAX_CONTAINER_HIERARCHIES]; > + > +/* dummytop is a shorthand for the dummy hierarchy's top container */ > +#define dummytop (&rootnode[0].top_container) > + With these changes, is there a generic way to determine the root container for the hierarchy the subsystem is in? Calls to ->create() pass the dummytop container. It would be useful and is often required to walk the hierarchy and know the root of the container hierarchy. -- Thanks, Balbir Singh, Linux Technology Center, IBM Software Labs
creator: Balbir Singh
date: 2007-01-10T15:56:21-00:00

Re: [ckrm-tech] [PATCH 3/6] containers: Add generic multi-subsystem API to containers
type: item
description: On 1/10/07, Balbir Singh <balbir@in.ibm.com> wrote: > Paul Menage wrote: > > +/* The set of hierarchies in use. Hierarchy 0 is the "dummy > > + * container", reserved for the subsystems that are otherwise > > + * unattached - it never has more than a single container, and all > > + * tasks are part of that container. */ > > + > > +static struct containerfs_root rootnode[CONFIG_MAX_CONTAINER_HIERARCHIES]; > > + > > +/* dummytop is a shorthand for the dummy hierarchy's top container */ > > +#define dummytop (&rootnode[0].top_container) > > + > > With these changes, is there a generic way to determine the root container > for the hierarchy the subsystem is in? Calls to ->create() pass the dummytop > container. 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
creator: Paul Menage
date: 2007-01-11T22:53:55-00:00

Re: [ckrm-tech] [PATCH 4/6] containers: Simple CPU accounting container subsystem
type: item
description: On 1/10/07, Balbir Singh <balbir@in.ibm.com> wrote: > > 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 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) > 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 > 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
creator: Paul Menage
date: 2007-01-12T00:33:48-00:00

Re: [ckrm-tech] [PATCH 4/6] containers: Simple CPU accounting container subsystem
type: item
description: Paul Menage wrote: > On 1/10/07, Balbir Singh <balbir@in.ibm.com> wrote: >> 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 > > This is a multi-processor system? Yes, it has 4 cpus > > 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) > 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 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 >> > > 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. > 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. > Paul -- Balbir Sing…
creator: Balbir Singh
date: 2007-01-12T06:24:41-00:00

Re: [ckrm-tech] [PATCH 3/6] containers: Add generic multi-subsystem API to containers
type: item
description: Paul Menage wrote: > On 1/10/07, Balbir Singh <balbir@in.ibm.com> wrote: >> Paul Menage wrote: >>> +/* The set of hierarchies in use. Hierarchy 0 is the "dummy >>> + * container", reserved for the subsystems that are otherwise >>> + * unattached - it never has more than a single container, and all >>> + * tasks are part of that container. */ >>> + >>> +static struct containerfs_root rootnode[CONFIG_MAX_CONTAINER_HIERARCHIES]; >>> + >>> +/* dummytop is a shorthand for the dummy hierarchy's top container */ >>> +#define dummytop (&rootnode[0].top_container) >>> + >> With these changes, is there a generic way to determine the root container >> for the hierarchy the subsystem is in? Calls to ->create() pass the dummytop >> container. > > 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. > Yes, I saw that. > 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 :-) > 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 t…
creator: Balbir Singh
date: 2007-01-12T06:29:52-00:00

Re: [ckrm-tech] [PATCH 3/6] containers: Add generic multi-subsystem API to containers
type: item
description: On 1/11/07, Balbir Singh <balbir@in.ibm.com> wrote: > 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. Ah - I misunderstood what you were looking for before. > > Something like > > struct container *get_root_container(struct container_subsys *ss) > { > return &rootnode[ss->hierarchy]; > } Yes, something like that sounds fine - except that it would be return &rootnode[ss->hierarchy].top_container; Paul
creator: Paul Menage
date: 2007-01-12T08:10:32-00:00

Re: [ckrm-tech] [PATCH 4/6] containers: Simple CPU accounting container subsystem
type: item
description: On 1/11/07, Balbir Singh <balbir@in.ibm.com> wrote: > > 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 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. > > 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. 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
creator: Paul Menage
date: 2007-01-12T08:15:14-00:00

Re: [ckrm-tech] [PATCH 3/6] containers: Add generic multi-subsystem API to containers
type: item
description: Paul Menage wrote: > On 1/11/07, Balbir Singh <balbir@in.ibm.com> wrote: >> 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. > > Ah - I misunderstood what you were looking for before. I'll try and post the changes to cpu_acct once I get the container system working. > >> Something like >> >> struct container *get_root_container(struct container_subsys *ss) >> { >> return &rootnode[ss->hierarchy]; >> } > > Yes, something like that sounds fine - except that it would be > > return &rootnode[ss->hierarchy].top_container; > 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. > Paul -- Balbir Singh, Linux Technology Center, IBM Software Labs
creator: Balbir Singh
date: 2007-01-12T08:22:47-00:00

Re: [ckrm-tech] [PATCH 4/6] containers: Simple CPU accounting container subsystem
type: item
description: Paul Menage wrote: > On 1/11/07, Balbir Singh <balbir@in.ibm.com> wrote: >> 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 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. This is with the unchanged cpuacct subsystem. Ok, so the container system needs to mark active internally then. > >> 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. > > 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. > I understand that the features are exported to userspace. But from the userspace POV only the mount options change - right? > Paul > -- Balbir Singh, Linux Technology Center, IBM Software Labs
creator: Balbir Singh
date: 2007-01-12T08:26:28-00:00

Re: [ckrm-tech] [PATCH 4/6] containers: Simple CPU accounting container subsystem
type: item
description: On 1/12/07, Balbir Singh <balbir@in.ibm.com> wrote: > > I understand that the features are exported to userspace. But from > the userspace POV only the mount options change - right? > 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
creator: Paul Menage
date: 2007-01-12T17:32:26-00:00

Re: [PATCH 0/6] containers: Generic Process Containers (V6)
type: item
description: Quoting Paul Menage (menage@google.com): > Hi Serge, > > On 1/3/07, Serge E. Hallyn <serue@us.ibm.com> wrote: > >From: Serge E. Hallyn <serue@us.ibm.com> > >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. > > 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. In retrospect I don't like the changes in behavior. So my next version will aim for closer to the original (non-containerfs) behavior. > >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. > > 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). 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. > > > > 2. We need a semantic difference between attaching > > to a container, and being the first to join the > > container you just created. > > 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. > > > 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, …
creator: serue
date: 2007-01-12T18:42:07-00:00

[PATCH 0/1] Add mount/umount callbacks to containers (Re: [ckrm-tech] [PATCH 4/6] containers: Simple
type: item
description: Paul Menage wrote: > On 1/10/07, Balbir Singh <balbir@in.ibm.com> wrote: >> 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 > > 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) > >> 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 >> > 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. Signed-off-by: Balbir Singh <balbir@in.ibm.com> --- 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-callbac k 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 contain…
creator: Balbir Singh
date: 2007-01-15T09:01:26-00:00

[PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cleanups (Re: [ckrm
type: item
description: Balbir Singh wrote: > Paul Menage wrote: >> On 1/10/07, Balbir Singh <balbir@in.ibm.com> wrote: >>> 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 >> This is a multi-processor system? >> > Hi, Paul, > > I figured out the reason for the panic. Here are the fixes > 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. Signed-off-by: Balbir Singh <balbir@in.ibm.com> --- 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-moun t 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 = usageb…
creator: Balbir Singh
date: 2007-01-15T09:04:53-00:00

Re: [PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cleanups (Re: [
type: item
description: On 1/15/07, Balbir Singh <balbir@in.ibm.com> wrote: > > 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. 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? > > Multiplying the time by 1000 is not correct in cpuusage_read(). The code > has been converted to use the correct cputime API. Thanks. > > Add mount/umount callbacks. 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 > > Signed-off-by: Balbir Singh <balbir@in.ibm.com> > --- > > 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-moun t 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 …
creator: Paul Menage
date: 2007-01-15T09:22:16-00:00

Re: [ckrm-tech] [PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cle
type: item
description: Paul Menage wrote: > On 1/15/07, Balbir Singh <balbir@in.ibm.com> wrote: >> 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. > > 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. > 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. > 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? > 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). >> Multiplying the time by 1000 is not correct in cpuusage_read(). The code >> has been converted to use the correct cputime API. > > Thanks. > >> Add mount/umount callbacks. > > 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. > 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,…
creator: Balbir Singh
date: 2007-01-15T09:51:28-00:00

Re: [ckrm-tech] [PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cle
type: item
description: On 1/15/07, Balbir Singh <balbir@in.ibm.com> wrote: > > 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. > 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
creator: Paul Menage
date: 2007-01-15T10:01:19-00:00

Re: [ckrm-tech] [PATCH 1/1] Fix a panic while mouting containers on powerpc and some other small cle
type: item
description: Paul Menage wrote: > On 1/15/07, Balbir Singh <balbir@in.ibm.com> wrote: >> 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. >> > > 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). > That makes sense, the only additional thing required is to know when the subsystem really got mounted (we cannot keep polling hierarchy for it:-)) > Paul -- Balbir Singh, Linux Technology Center, IBM Software Labs
creator: Balbir Singh
date: 2007-01-15T10:10:37-00:00

Re: [ckrm-tech] [PATCH 3/6] containers: Add generic multi-subsystem API to containers
type: item
description: Paul Menage wrote: > On 1/11/07, Balbir Singh <balbir@in.ibm.com> wrote: >> 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. > > Ah - I misunderstood what you were looking for before. 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. Signed-off-by: <balbir@in.ibm.com> --- 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 --- linux-2.6.20-rc5/kernel/cpu_acct.c~cpu_acct_load_acct 2007-01-20 18:28:26.000000000 +0530 +++ linux-2.6.20-rc5-balbir/kernel/cpu_acct.c 2007-01-20 22:32:49.000000000 +0530 @@ -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_creat…
creator: Balbir Singh
date: 2007-01-20T17:27:44-00:00