OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/7] containers (V7): Generic Process Containers
items: [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
items: [PATCH 2/7] containers (V7): Cpusets hooked into containers
items: [PATCH 3/7] containers (V7): Add generic multi-subsystem API to containers
items: [PATCH 4/7] containers (V7): Simple CPU accounting container subsystem
items: [PATCH 5/7] containers (V7): Resource Groups over generic containers
items: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
items: [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
items: Re: [PATCH 0/7] containers (V7): Generic Process Containers
items: Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
items: Re: [PATCH 3/7] containers (V7): Add generic multi-subsystem API to containers
items: Re: [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
items: Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
items: Re: [PATCH 2/7] containers (V7): Cpusets hooked into containers
items: Re: [ckrm-tech] [PATCH 2/7] containers (V7): Cpusets hooked into containers
items: Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem

[PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: -- This is an update to my multi-hierarchy multi-subsystem generic process containers patch. Changes since V6 (22nd December) include: - updated to 2.6.20 - added more details about multiple hierarchy support in the documentation - reduced the per-task memory overhead to one pointer (previously it was one pointer for each hierarchy). Now each task has a pointer to a container_group, which holds the pointers to the containers (one per active hierarchy) that the task is attached to and the associated per-subsystem state (one per active subsystem). This container group is shared (with reference counts) between all tasks that have the same set of container mappings. - added API support for binding/unbinding subsystems to/from active hierarchies, by remounting with -oremount,<new-subsys-list>. Currently this fails with EBUSY if the hierarchy has a child containers; full implementation support is left to a later patch. - added a bind() subsystem callback to indicate when a subsystem is moved between hierarchies - added container_clone(subsys, task), which creates a child container for the hierarchy that the specified subsystem is bound to, and moves the given task into that container. An example use of this would be in sys_unshare, which could, if the namespace container subsystem is active, create a child container when the new namespace is created. - temporarily removed the "release agent" support. It's only currently used by CPUsets, and intrudes somewhat on the per-container reference counting. If necessary it can be re-added, either as a generic subsystem feature or a CPUset-specific feature, via a kernel thread that periodically polls containers that have been designated as notify_on_release to see if they are releasable Generic Process Containers -------------------------- There have recently been various proposals floating around for resource management/accounting and other task grouping subsystems in the kernel, including ResGroups, User BeanCounters, NSProxy con…
creator: Paul Menage
date: 2007-02-12T08:15:21-00:00

[PATCH 1/7] containers (V7): 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/fs/proc/base.c ============================================================ ======= --- container-2.6.20.orig/fs/proc/base.c +++ container-2.6.20/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> @@ -1870,6 +1871,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 @@ -2151,6 +2155,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/include/linux/container.h ============================================================ ======= --- /dev/null +++ container-2.6.20/include/linux/container.h @@ -0,0 +1,96 @@ +#ifndef _LINUX_CONTAINER_H +#define _LINUX_CONTAINER_H +/* + * container interface + * + * Copyright (C) 2003 BU…
creator: Paul Menage
date: 2007-02-12T08:15:22-00:00

[PATCH 2/7] containers (V7): 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/include/linux/container.h ============================================================ ======= --- container-2.6.20.orig/include/linux/container.h +++ container-2.6.20/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/include/linux/cpuset.h ============================================================ ======= --- container-2.6.20.orig/include/linux/cpuset.h +++ container-2.6.20/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 …
creator: Paul Menage
date: 2007-02-12T08:15:23-00:00

[PATCH 3/7] containers (V7): 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 | 415 +++++++++-- Documentation/cpusets.txt | 20 include/linux/container.h | 178 ++++ include/linux/cpuset.h | 16 include/linux/mempolicy.h | 12 include/linux/sched.h | 4 init/Kconfig | 12 kernel/container.c | 1601 ++++++++++++++++++++++++++++++------------- kernel/cpuset.c | 170 ++-- mm/mempolicy.c | 2 10 files changed, 1808 insertions(+), 622 deletions(-) Index: container-2.6.20/include/linux/container.h =================================================================== --- container-2.6.20.orig/include/linux/container.h +++ container-2.6.20/include/linux/container.h @@ -9,13 +9,12 @@ */ #include <linux/sched.h> +#include <linux/kref.h> #include <linux/cpumask.h> #include <linux/nodemask.h> #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,13 +29,105 @@ 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; +}; + +/* A container_group is a structure holding pointers to a set of + * containers. This saves space in the task struct object and speeds + * up fork()/exit(), since …
creator: Paul Menage
date: 2007-02-12T08:15:24-00:00

[PATCH 4/7] containers (V7): 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 | 213 +++++++++++++++++++++++++++++++++++++++++++++++ kernel/sched.c | 14 ++- 5 files changed, 246 insertions(+), 3 deletions(-) Index: container-2.6.20/include/linux/cpu_acct.h ============================================================ ======= --- /dev/null +++ container-2.6.20/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/init/Kconfig ============================================================ ======= --- container-2.6.20.orig/init/Kconfig +++ container-2.6.20/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/kernel/cpu_acct.c ============================================================ ======= --- /dev/null +++ container-2.6.20/kernel/cpu_acct.c @@ -0,0 +1,213 @@ +/* + * kernel/cpu_acct.c - CPU accounting container subsystem + * + * Copyright (C) Google Inc, 2006 + * + * Developed by Paul Menage (menage@google.com) and Balbir Singh + * (balbir@in.ibm.com) + * + */ + +/* + * Container subsystem for reporting total CPU usage of ta…
creator: Paul Menage
date: 2007-02-12T08:15:25-00:00

[PATCH 5/7] containers (V7): 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/include/linux/moduleparam.h ============================================================ ======= --- container-2.6.20.orig/include/linux/moduleparam.h +++ container-2.6.20/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,…
creator: Paul Menage
date: 2007-02-12T08:15:26-00:00

[PATCH 6/7] containers (V7): 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/init/Kconfig ============================================================ ======= --- container-2.6.20.orig/init/Kconfig +++ container-2.6.20/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/kernel/Makefile ============================================================ ======= --- container-2.6.20.orig/kernel/Makefile +++ container-2.6.20/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/kernel/bc/Kconfig ============================================================ ======= --- /dev/null +++ container-2.6.20/kernel/bc/Kconfig @@ -0,0 +1,17 @@ +config BEANCOUNTERS + bool "Enable resource accounting/control" + default n + selec…
creator: Paul Menage
date: 2007-02-12T08:15:27-00:00

[PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: When a task enters a new namespace via a clone() or unshare(), a new container is created and the task moves into it. Developed by Serge Hallyn <serue@us.ibm.com>, adapted by Paul Menage <menage@google.com> --- include/linux/nsproxy.h | 6 ++ init/Kconfig | 9 +++ kernel/Makefile | 1 kernel/fork.c | 4 + kernel/ns_container.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++ kernel/nsproxy.c | 6 ++ 6 files changed, 136 insertions(+) Index: container-2.6.20/include/linux/nsproxy.h ============================================================ ======= --- container-2.6.20.orig/include/linux/nsproxy.h +++ container-2.6.20/include/linux/nsproxy.h @@ -53,4 +53,10 @@ static inline void exit_task_namespaces( put_nsproxy(ns); } } +#ifdef CONFIG_CONTAINER_NS +int ns_container_clone(struct task_struct *tsk); +#else +static inline int ns_container_clone(struct task_struct *tsk) { return 0; } +#endif + #endif Index: container-2.6.20/init/Kconfig ============================================================ ======= --- container-2.6.20.orig/init/Kconfig +++ container-2.6.20/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 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: container-2.6.20/kernel/Makefile ============================================================ ======= --- container-2.6.20.orig/kernel/Makefile +++ container-2.6.20/kernel/Makefile @@ -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…
creator: Paul Menage
date: 2007-02-12T08:15:28-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: > - temporarily removed the "release agent" support. ouch > ... it can be re-added ... via a kernel thread that periodically polls containers ... double ouch. You'll have a rough time selling me on the idea that some kernel thread should be waking up every few seconds, grabbing system-wide locks, on a big honkin NUMA box, for the few times per hour, or less, that a cpuset is abandoned. Offhand, that sounds mildly insane to me. And how would this get the edge-triggered, rather than level-triggered, release? In other words, if a new cpuset is created, and marked with the notify_on_release flag, but otherwise not yet used (no child cpusets and no tasks in it) then it is not to be released (removed.) Only children and/or tasks are added, then later removed, is it a candidate for release. I guess you'll need yet another state bit, set when the cpuset is abandoned (last child removed or last pid exits/leaves), and cleared when this kernel thread visits the cpuset to see if it should be removed. Can you explain to me how this intruded on the reference counting? -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-02-12T09:18:43-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/12/07, Paul Jackson <pj@sgi.com> wrote: > > You'll have a rough time selling me on the idea that some kernel thread > should be waking up every few seconds, grabbing system-wide locks, on a > big honkin NUMA box, for the few times per hour, or less, that a cpuset is > abandoned. I think it could be made smarter than that, e.g. have a workqueue task that's only woken when a refcount does actually reach zero. (I think that waking a workqueue task is something that can be done without too much worry about locks) > > Can you explain to me how this intruded on the reference counting? > Essentially, it means that anything that releases a reference count on a container needs to be able to trigger a call to the release agent. The reference count is often released at a point when important locks are held, so you end up having to pass buffers into any function that might drop a ref count, in order to store a path to a release agent to be invoked. In particular, the new container_clone() function can be called during the task fork path; at which point forking a new release_agent process would be impossible, or at least nasty. Additionally, if containers are potentially going to be used for virtual servers, having the release agent run from a top-level process rather than the process context that released the refcount sounds like a sane option. Paul
creator: Paul Menage
date: 2007-02-12T09:32:51-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Paul M, responding to Paul J: > I think it could be made smarter than that, e.g. have a workqueue task > that's only woken when a refcount does actually reach zero. (I think > that waking a workqueue task is something that can be done without too > much worry about locks) > > > > > Can you explain to me how this intruded on the reference counting? > > > > Essentially, it means that anything that releases a reference count on > a container needs to be able to trigger a call to the release agent. > The reference count is often released at a point when important locks > are held, so you end up having to pass buffers into any function that > might drop a ref count, in order to store a path to a release agent to > be invoked. Ok - now that you put it like that - it's much more persuasive. Consider me sold on this aspect of your proposal, until and unless I protest otherwise, which is not likely. Thanks. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-02-12T09:52:00-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: On Mon, Feb 12, 2007 at 12:15:27AM -0800, menage@google.com wrote: > This patch implements the BeanCounter resource control abstraction > over generic process containers. Forgive my confusion, but do we really need two-levels of resource control abstraction here? Why can't resource controllers directly work with containers (just like cpu accounting does)? -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-02-12T15:34:10-00:00

Re: [PATCH 3/7] containers (V7): Add generic multi-subsystem API to containers
type: item
description: >> +#include <asm/proto.h> > > I did have a problem with this include. On s390 it didn't exist so I've > just been running without it (with no problems). A quick 'find' > suggests it only exists on x86_64, so I'd expect failures on all other > arches. confirmed on x86 also. C. _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Cedric Le Goater
date: 2007-02-12T15:56:27-00:00

Re: [PATCH 3/7] containers (V7): Add generic multi-subsystem API to containers
type: item
description: On 2/12/07, Cedric Le Goater <clg@fr.ibm.com> wrote: > >> +#include <asm/proto.h> > > > > I did have a problem with this include. On s390 it didn't exist so I've > > just been running without it (with no problems). A quick 'find' > > suggests it only exists on x86_64, so I'd expect failures on all other > > arches. > > confirmed on x86 also. > Sorry, this turned out to be left over from when I was debugging an initialization problem that required early_printk(). I should have removed it. You can safely remove it from the patch. Paul _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Paul Menage
date: 2007-02-12T18:31:48-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: On 2/12/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > On Mon, Feb 12, 2007 at 12:15:27AM -0800, menage@google.com wrote: > > This patch implements the BeanCounter resource control abstraction > > over generic process containers. > > Forgive my confusion, but do we really need two-levels of resource control > abstraction here? Why can't resource controllers directly work with containers > (just like cpu accounting does)? > The generic containers patch represents a pretty low-level view of task grouping - it doesn't try to prescribe how to do accounting, nor exactly what API to present to the user (beyond providing a filesystem-based interface). Resource controllers certainly can be written directly over it, but equally having additional abstractions to provide a common user API and kernel API for multiple resources is a reasonable goal. I would imagine that each different resource being controlled would be represented as a container subsystem, which is how I structured the ResGroups example patch - ResGroups becomes a library that provides a common set of file manipulations for different resource controllers, each of which is a containers subsystem. The same could potentially be done for BeanCounters if people wanted. But the main point of the latter four patches in this series is to illustrate to the various folks writing resource controller systems (and other observers) that this patch provides sufficient features to act as a base for their work. I don't presume to claim that one higher-level resource control abstraction is better than another. Paul
creator: Paul Menage
date: 2007-02-12T18:49:58-00:00

Re: [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On 2/12/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > On Mon, Feb 12, 2007 at 12:15:22AM -0800, menage@google.com wrote: > > +void container_fork(struct task_struct *child) > > +{ > > + task_lock(current); > > Can't this be just rcu_read_lock()? > In this particular patch (which is an almost verbatim extraction/renaming of the generic bits of the cpusets code into container.c) it probably could - but the main patch that adds the container_group support would lose it since we use kref to refcount container_group objects, and hence they're freed when their refcount reaches zero. RCU is still fine for reading the container_group pointers, but it's no good for updating them, since by the time you update it it may no longer be your container_group structure, and may instead be about to be deleted as soon as the other thread's rcu_synchronize() completes. Paul
creator: Paul Menage
date: 2007-02-12T19:26:34-00:00

Re: [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On 2/12/07, Paul Menage <menage@google.com> wrote: > reaches zero. RCU is still fine for reading the container_group > pointers, but it's no good for updating them, since by the time you > update it it may no longer be your container_group structure, and may > instead be about to be deleted as soon as the other thread's > rcu_synchronize() completes. On further reflection, this probably would be safe after all. Since we don't call put_container_group() in attach_task() until after synchronize_rcu() completes, that implies that a container_group_get() from the RCU section would have already completed. So we should be fine. Paul
creator: Paul Menage
date: 2007-02-12T19:46:20-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: menage@google.com wrote: > Generic Process Containers > -------------------------- > > There have recently been various proposals floating around for > resource management/accounting and other task grouping 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, or control > other behaviour of the processes, and all implement this grouping in > different ways. > I know I'm a bit out of touch, but AIUI the NSProxy *is* the container. We decided a long time ago that a container was basically just a set of namespaces, which includes all of the subsystems you mention. This would suggesting re-write this patchset, part 2 as a "CPUSet namespace", part 4 as a "CPU scheduling namespace", parts 5 and 6 as "Resource Limits Namespace" (drop this "BeanCounter" brand), and of course part 7 falls away. Sam. _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Sam Vilain
date: 2007-02-12T22:38:32-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Quoting Sam Vilain (sam@vilain.net): > menage@google.com wrote: > > Generic Process Containers > > -------------------------- > > > > There have recently been various proposals floating around for > > resource management/accounting and other task grouping 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, or control > > other behaviour of the processes, and all implement this grouping in > > different ways. > > > > I know I'm a bit out of touch, but AIUI the NSProxy *is* the container. The nsproxy is an implementation detail, actually, and might go away at any time. But yes we had been talking for quite awhile about sets of namespaces as containers, so that a vserver would be a container, as would a lightweight checkpoint/restart job. > We decided a long time ago that a container was basically just a set of > namespaces, which includes all of the subsystems you mention. > > This would suggesting re-write this patchset, part 2 as a "CPUSet Well it's an unfortunate conflict, but I don't see where we have any standing to make Paul change his terminology :) Lately I've been talking about "Paul's containers", our "namespaces", and the "namespace container subsystem" in patch 7. > namespace", part 4 as a "CPU scheduling namespace", parts 5 and 6 as > "Resource Limits Namespace" (drop this "BeanCounter" brand), and of > course part 7 falls away. Part 7 does not fall away, it uses Paul's patchset to start to provide a management interface for namespaces. -serge _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: serue
date: 2007-02-12T22:47:17-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/12/07, Sam Vilain <sam@vilain.net> wrote: > > I know I'm a bit out of touch, but AIUI the NSProxy *is* the container. > We decided a long time ago that a container was basically just a set of > namespaces, which includes all of the subsystems you mention. You may have done that, but the CKRM/ResGroups independently decided a long time ago that the fundamental unit was the resource class, and the OpenVZ folks decided that the fundamental unit was the BeanCounter, and the CPUSet folks decided that the fundamental unit was the CPUSet, etc ... :-) But there's a lot of common ground between these different approaches, and potential for synergy, so the point of this patch set is to provide a unification point for all of them, and a stepping stone for other new resource controllers and process control modules. Paul _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Paul Menage
date: 2007-02-12T23:15:51-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/12/07, Serge E. Hallyn <serue@us.ibm.com> wrote: > > Well it's an unfortunate conflict, but I don't see where we have any > standing to make Paul change his terminology :) I have no huge problem with changing my terminology in the interest of wider adoption. "Container" seems like an appropriate name for the abstraction, and possibly more appropriate than for a virtual server, but if it was decreed that the only thing stopping the container patch being merged was that it should be called, say, "Process Sets", I would happily s/container/pset/g across the entire patchset. Paul _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Paul Menage
date: 2007-02-12T23:18:20-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Paul Menage wrote: >> I know I'm a bit out of touch, but AIUI the NSProxy *is* the container. >> We decided a long time ago that a container was basically just a set of >> namespaces, which includes all of the subsystems you mention. >> > You may have done that, but the CKRM/ResGroups independently decided a > long time ago that the fundamental unit was the resource class, and > the OpenVZ folks decided that the fundamental unit was the > BeanCounter, and the CPUSet folks decided that the fundamental unit > was the CPUSet, etc ... :-) > There was a consensus that emerged that resulted in the nsproxy being included in the kernel. That is why I used "we". What was included varies greatly from the patch I put forward, as I mentioned. You missed the point of what I was trying to say. Let me try again. Originally I too thought that in order to begin on the path of virtualisation merging we would just make a simple container/vserver/whatever structure and hang everything off that. I'll now say the same thing that was said before of my patch, that I don't think that adding the containers structure inside the kernel adds anything interesting or useful. In fact, I'd go further to say that very thing you think is a useful abstraction is locking yourself into inflexibility. This is what Eric recognised early on and eventually brought me around to the idea of too. So, we have the current implementation - individual subsystems are virtualised, and it is the subsystems that you can see and work with. You can group together your virtualisation decisions and call that a container, great. Ask yourself this - what do you need the container structure for so badly, that virtualising the individual resources does not provide for? You don't need it to copy the namespaces of another process ("enter") and you don't need it for checkpoint/migration. What does it mean to make a new container? You're making a "nothing" namespace for some as-yet-unspecified grouping of tasks. That's a great id…
creator: Sam Vilain
date: 2007-02-13T00:30:05-00:00

Re: [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Mon, Feb 12, 2007 at 11:46:20AM -0800, Paul Menage wrote: > On further reflection, this probably would be safe after all. Since we > don't call put_container_group() in attach_task() until after > synchronize_rcu() completes, that implies that a container_group_get() > from the RCU section would have already completed. So we should be > fine. Right. Which make me wonder why we need task_lock() at all ..I can understand the need for a lock like that if we are reading/updating multiple words in task_struct under the lock. In this case, it is used to read/write just one pointer, isnt it? I think it can be eliminated all-together with the use of RCU. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-02-13T05:48:57-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Tue, Feb 13, 2007 at 11:18:57AM +0530, Srivatsa Vaddagiri wrote: > Which make me wonder why we need task_lock() at all ..I can understand > the need for a lock like that if we are reading/updating multiple words > in task_struct under the lock. In this case, it is used to read/write > just one pointer, isnt it? I think it can be eliminated all-together > with the use of RCU. I see that cpuset.c uses task_lock to read/write multiple words (cpuset_update_task_memory_state) ..So yes it is necessary in attach_task() .. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-02-13T08:16:25-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: menage@google.com 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. Numfiles is not the most interesting place in beancounters. Kmemsize accounting is much more important actually. > 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(-) > [snip] > Index: container-2.6.20/kernel/bc/misc.c > ============================================================ ======= > --- /dev/null > +++ container-2.6.20/kernel/bc/misc.c > @@ -0,0 +1,56 @@ > + > +#include <linux/fs.h> > +#include <bc/beancounter.h> > + > +int bc_file_charge(struct file *file) > +{ > + int sev; > + struct beancounter *bc; > + > + task_lock(current); > + bc = task_bc(current); > + css_get_current(&bc->css); > + task_unlock(current); > + > + sev = (capable(CAP_SYS_ADMIN) ? BC_LIMIT : BC_BARRIER); > + > + if (bc_charge(bc, BC_NUMFILES, 1, sev)) { > + css_put(&bc->css); > + return -EMFILE; > + } > + > + file->f_bc = bc; > + return 0; > +} > + I have already pointed out the fact that this place will hurt performance too much. If we have some context on task this context must 1. be get-ed without any locking 2. be settable to some temporary one without locking…
creator: xemul
date: 2007-02-13T08:52:24-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: On 2/13/07, Pavel Emelianov <xemul@sw.ru> wrote: > menage@google.com 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. > > Numfiles is not the most interesting place in beancounters. > Kmemsize accounting is much more important actually. Right, but the memory accouting was a much bigger and more intrusive patch than I wanted to include as an example. > > I have already pointed out the fact that this place > will hurt performance too much. If we have some context > on task this context must > 1. be get-ed without any locking Would you also be happy with the restriction that a task couldn't be moved in/out of a beancounter container by any task other than itself? If so, the beancounter can_attach() method could simply return false if current != tsk, and then you'd not need to worry about locking in this situation. > 2. be settable to some temporary one without > locking as well I thought that we solved that problem by having a tmp_bc field in the task_struct that would take precedence over the main bc if it was non-null? Paul
creator: Paul Menage
date: 2007-02-13T09:03:12-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: Paul Menage wrote: > On 2/13/07, Pavel Emelianov <xemul@sw.ru> wrote: >> menage@google.com 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. >> >> Numfiles is not the most interesting place in beancounters. >> Kmemsize accounting is much more important actually. > > Right, but the memory accouting was a much bigger and more intrusive > patch than I wanted to include as an example. I know it, but numfile doesn't show how good this infrastructure is. >> >> I have already pointed out the fact that this place >> will hurt performance too much. If we have some context >> on task this context must >> 1. be get-ed without any locking > > Would you also be happy with the restriction that a task couldn't be > moved in/out of a beancounter container by any task other than itself? I have implementation that moves arbitrary task :) May be we can do context (container-on-task) handling lockless? > If so, the beancounter can_attach() method could simply return false > if current != tsk, and then you'd not need to worry about locking in > this situation. I may not, but this patch contains locking that is not good even for example. >> 2. be settable to some temporary one without >> locking as well > > I thought that we solved that problem by having a tmp_bc field in the > task_struct that would take precedence over the main bc if it was > non-null? Of course, but I'm commenting this patchset which doesn't have this facility. > Paul >
creator: xemul
date: 2007-02-13T09:18:18-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: On 2/13/07, Pavel Emelianov <xemul@sw.ru> wrote: > > I have implementation that moves arbitrary task :) Is that the one that calls stop_machine() in order to move a task around? That seemed a little heavyweight ... > May be we can do context (container-on-task) handling lockless? What did you have in mind? > > I thought that we solved that problem by having a tmp_bc field in the > > task_struct that would take precedence over the main bc if it was > > non-null? > > Of course, but I'm commenting this patchset which doesn't have > this facility. OK, I can add the concept in to the example too. Paul
creator: Paul Menage
date: 2007-02-13T09:37:27-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: Paul Menage wrote: > On 2/13/07, Pavel Emelianov <xemul@sw.ru> wrote: >> >> I have implementation that moves arbitrary task :) > > Is that the one that calls stop_machine() in order to move a task > around? That seemed a little heavyweight ... Nope :) I've rewritten it completely. >> May be we can do context (container-on-task) handling lockless? > > What did you have in mind? The example patch is attached. Fits 2.6.20-rc6-mm3. >> > I thought that we solved that problem by having a tmp_bc field in the >> > task_struct that would take precedence over the main bc if it was >> > non-null? >> >> Of course, but I'm commenting this patchset which doesn't have >> this facility. > > OK, I can add the concept in to the example too. > > Paul > --- ./kernel/bc/misc.c.bcctx 2007-01-31 13:56:45.000000000 +0300 +++ ./kernel/bc/misc.c 2007-01-31 14:20:32.000000000 +0300 @@ -0,0 +1,63 @@ +/* + * kernel/bc/misc.c + * + * Copyright (C) 2007 OpenVZ SWsoft Inc + * + */ + +#include <linux/sched.h> +#include <linux/stop_machine.h> +#include <linux/module.h> + +#include <bc/beancounter.h> +#include <bc/task.h> +#include <bc/misc.h> + +static DEFINE_MUTEX(task_move_mutex); + +int copy_beancounter(struct task_struct *tsk, struct task_struct *parent) +{ + struct beancounter *bc; + + bc = parent->exec_bc; + tsk->exec_bc = bc_get(bc); + BUG_ON(tsk->tmp_exec_bc != NULL); + return 0; +} + +void free_beancounter(struct task_struct *tsk) +{ + struct beancounter *bc; + + BUG_ON(tsk->tmp_exec_bc != NULL); + bc = tsk->exec_bc; + bc_put(bc); +} + +int bc_task_move(int pid, struct beancounter *bc) +{ + struct task_struct *tsk; + struct beancounter *old_bc; + + read_lock(&tasklist_lock); + tsk = find_task_by_pid(pid); + if (tsk) + get_task_struct(tsk); + read_unlock(&tasklist_lock); + if (tsk == NULL) + return -ESRCH; + + mutex_lock(&task_move_mutex); + old_bc = tsk->exec_bc; + + bc_get(bc); + rcu_assign_pointer(tsk->exec_bc, bc); + + /* wait for all users if any get this beancounter */ + synchronize_rc…
creator: xemul
date: 2007-02-13T09:49:36-00:00

Re: [PATCH 3/7] containers (V7): Add generic multi-subsystem API to containers
type: item
description: menage@google.com wrote: > 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> > Hi, Paul, This patch fails to apply for me balbir@balbir-laptop:~/ocbalbir/images/kernels/containers/linux-2.6.20$ pushpatch patching file include/linux/container.h patching file include/linux/cpuset.h patching file kernel/container.c patch: **** malformed patch at line 640: @@ -202,15 +418,18 @@ static DEFINE_MUTEX(callback_mutex); multiuser_container does not apply Is anybody else seeing this problem? -- Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2007-02-14T08:49:24-00:00

Re: [PATCH 6/7] containers (V7): BeanCounters over generic process containers
type: item
description: On 2/13/07, Pavel Emelianov <xemul@sw.ru> wrote: > > The example patch is attached. Fits 2.6.20-rc6-mm3. > OK, I've incorporated some of those changes into my patchset. bc_file_charge() and bc_file_uncharge() are now lockless, and tmp_exec_bc is supported too. I've attached the new versions of patch 3 (main multi-user container support) and patch 7 (beancounters example) Does that look more like what you'd like to see? Paul
creator: Paul Menage
date: 2007-02-15T00:47:54-00:00

Re: [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: Quoting menage@google.com (menage@google.com): > Index: container-2.6.20/init/Kconfig > ============================================================ ======= > --- container-2.6.20.orig/init/Kconfig > +++ container-2.6.20/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 Hi, why did you do this? thanks, -serge
creator: serue
date: 2007-02-15T20:35:17-00:00

Re: [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: On 2/15/07, Serge E. Hallyn <serue@us.ibm.com> wrote: > > > > 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 > > Hi, > > why did you do this? > Containers on their own aren't all that useful, other than for process tracking, so it seemed reasonable to only build containers if there was some subsystem using them. If people wanted to use them for process tracking alone, it could certainly be made an individually-selectable option. Paul
creator: Paul Menage
date: 2007-02-15T20:49:04-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: "Paul Menage" <menage@google.com> writes: > On 2/12/07, Sam Vilain <sam@vilain.net> wrote: >> >> I know I'm a bit out of touch, but AIUI the NSProxy *is* the container. >> We decided a long time ago that a container was basically just a set of >> namespaces, which includes all of the subsystems you mention. > > You may have done that, but the CKRM/ResGroups independently decided a > long time ago that the fundamental unit was the resource class, and > the OpenVZ folks decided that the fundamental unit was the > BeanCounter, and the CPUSet folks decided that the fundamental unit > was the CPUSet, etc ... :-) Using the container name is bad and it led to this stupid argument. The fundamental unit of what we have merged into the kernel is the namespace. The aggregate of all namespaces and everything is the container. Please, please pick a different name so people don't take one look at your stuff and get confused like Sam did. > But there's a lot of common ground between these different approaches, > and potential for synergy, so the point of this patch set is to > provide a unification point for all of them, and a stepping stone for > other new resource controllers and process control modules. For the case of namespaces I don't see how your code makes things better. I do not see a real problem that you are solving. I do agree that a common interface to the code and a common set of infrastructure may help. Personally until the pid namespace (which includes within it process groupings) is complete I'm not certain the problem will be concrete enough to solve. Hopefully we can have a working version of that shortly. Eric _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: ebiederm
date: 2007-02-20T17:34:00-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/20/07, Eric W. Biederman <ebiederm@xmission.com> wrote: > "Paul Menage" <menage@google.com> writes: > > > On 2/12/07, Sam Vilain <sam@vilain.net> wrote: > >> > >> I know I'm a bit out of touch, but AIUI the NSProxy *is* the container. > >> We decided a long time ago that a container was basically just a set of > >> namespaces, which includes all of the subsystems you mention. > > > > You may have done that, but the CKRM/ResGroups independently decided a > > long time ago that the fundamental unit was the resource class, and > > the OpenVZ folks decided that the fundamental unit was the > > BeanCounter, and the CPUSet folks decided that the fundamental unit > > was the CPUSet, etc ... :-) > > Using the container name is bad and it led to this stupid argument. > > The fundamental unit of what we have merged into the kernel is the > namespace. The aggregate of all namespaces and everything is the > container. What are you defining here as "everything"? If you mean "all things that could be applied to a segregated group of processes such as a virtual server", then "container" seems like a good name for my patches, since it allows you to aggregate namespaces, resource control, other virtualization, etc. Sam said "the NSProxy *is* the container". You appear to be planning to have some namespaces, possibly not aggregated within the nsproxy (pid namespace?) but are you planning to have some higher-level "container" object that aggregates the nsproxy and the other namespaces? If so, what is it? Does it track process membership, etc? What's the userspace API? In what fundamental ways would it be different from my generic containers patches with Serge Hallyn's namespace subsystem. (If these questions are answered already by designs or code, I'd be happy to be pointed at them). I guess what it comes down to, is why is an aggregation of namespaces suitable for the name "container", when an aggregation of namespaces and other resource controllers isn't? What do you think mig…
creator: Paul Menage
date: 2007-02-20T17:55:11-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: "Paul Menage" <menage@google.com> writes: > What are you defining here as "everything"? If you mean "all things > that could be applied to a segregated group of processes such as a > virtual server", then "container" seems like a good name for my > patches, since it allows you to aggregate namespaces, resource > control, other virtualization, etc. The aggregation happens at the task struct when you apply different pieces to a task we don't need a structure to allow aggregation. > Sam said "the NSProxy *is* the container". You appear to be planning > to have some namespaces, possibly not aggregated within the nsproxy > (pid namespace?) but are you planning to have some higher-level > "container" object that aggregates the nsproxy and the other > namespaces? No. A reverse mapping is not needed and is not interesting. As long as I can walk all processes and ask what namespace are you in I don't care. > If so, what is it? Does it track process membership, etc? > What's the userspace API? In what fundamental ways would it be > different from my generic containers patches with Serge Hallyn's > namespace subsystem. (If these questions are answered already by > designs or code, I'd be happy to be pointed at them). > I guess what it comes down to, is why is an aggregation of namespaces > suitable for the name "container", when an aggregation of namespaces > and other resource controllers isn't? To me at least the interesting part of your work is not the aggregation portion. But the infrastructure for building the different process groups. You seem to be calling your groups of processes lumped together for one purpose or another a container. > What do you think might be a better name for the generic process > groups that I'm pushing? As I said, I'm happy to do a simple > search/replace on my code to give a different name if that turned out > to be the gating factor to getting it merged. But I'd be inclined to > leave that decision up to Andrew/Linus. As we have previously bee…
creator: ebiederm
date: 2007-02-20T19:29:48-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Paul Menage wrote: >> Using the container name is bad and it led to this stupid argument. >> >> The fundamental unit of what we have merged into the kernel is the >> namespace. The aggregate of all namespaces and everything is the >> container. >> > What are you defining here as "everything"? If you mean "all things > that could be applied to a segregated group of processes such as a > virtual server", The term "segregated group of processes" is too vague. Segregated for what? What is the kernel supposed to do with this information? > I guess what it comes down to, is why is an aggregation of namespaces > suitable for the name "container", when an aggregation of namespaces > and other resource controllers isn't? > This argument goes away if you just rename these resource groups to resource namespaces. > What do you think might be a better name for the generic process > groups that I'm pushing? As I said, I'm happy to do a simple > search/replace on my code to give a different name if that turned out > to be the gating factor to getting it merged. But I'd be inclined to > leave that decision up to Andrew/Linus. > Did you like the names I came up with in my original reply? - CPUset namespace for CPU partitioning - Resource namespaces: - cpusched namespace for CPU - ulimit namespace for memory - quota namespace for disk space - io namespace for disk activity - etc >> For the case of namespaces I don't see how your code makes things >> better. I do not see a real problem that you are solving. >> > I'm trying to solve the problem that lots of different folks > (including us) are trying to do things that aggregate multiple process > into some kind of constrained group, and are all trying to use > different and incompatible ways of grouping/tracking those processes. > Maybe what's missing is a set of helper macros/functions that assist with writing new namespaces. Perhaps you can give some more examples and we can consider these on a case by case basis. Sam. ______________…
creator: Sam Vilain
date: 2007-02-20T21:58:31-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/20/07, Sam Vilain <sam@vilain.net> wrote: > > The term "segregated group of processes" is too vague. Segregated for > what? What is the kernel supposed to do with this information? The generic part of the kernel just keeps track of the fact that they're segregated (and their children, etc). It's the clients of this subsystem (virtual servers, resource controllers) that can decide to give different per-process behaviour based on the membership of various groups. > Did you like the names I came up with in my original reply? > > - CPUset namespace for CPU partitioning > - Resource namespaces: > - cpusched namespace for CPU > - ulimit namespace for memory > - quota namespace for disk space > - io namespace for disk activity > - etc This is a strange abuse of the term "namespace". http://en.wikipedia.org/wiki/Namespace_%28computer_science%29 For the virtual server work that you're doing, namespace is a good term: pids name processes, hence a pid namespace lets you have multiple distinct mappings from pids to processes filenames name files, so a filename (or mount) namespace lets you have multiple distinct mappings from filenames to files. For resource QoS control, it doesn't really make sense to talk about namespaces. We're not virtualizing resources to rename them for different virtual servers, we're limiting the quality of access to the resources. But the semantics of the term "namespace" notwithstanding, you're equating a virtual server namespace (pid, ipc, uts, mounts, etc) with a resource controller (memory, I/O, etc) in terms of their place in a hierarchy, which is something I agree with. All of these subsystems can be considered to be units that can be associated with groups of processes; the ultimate grouping of processes is something that we're both ultimately referring to as a container. > > Maybe what's missing is a set of helper macros/functions that assist > with writing new namespaces. That's pretty much what my containers patch does - provides a gener…
creator: Paul Menage
date: 2007-02-20T22:19:39-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/20/07, Eric W. Biederman <ebiederm@xmission.com> wrote: > > Sam said "the NSProxy *is* the container". You appear to be planning > > to have some namespaces, possibly not aggregated within the nsproxy > > (pid namespace?) but are you planning to have some higher-level > > "container" object that aggregates the nsproxy and the other > > namespaces? > > No. A reverse mapping is not needed and is not interesting. ... to you. > As long as I can walk all processes and ask what namespace are > you in I don't care. How do you currently do that? > > To me at least the interesting part of your work is not the aggregation > portion. But the infrastructure for building the different process > groups. In that case you can easily use it to just assign one namespace type to each tree of process groups. The aggregation is something that other groups find useful, but isn't required for the user to actually make use of. > > You seem to be calling your groups of processes lumped together for > one purpose or another a container. Correct. > > We need a term for the non-aggregated case for the individual process > groups we build this out of in your infrastructure. Because you > clearly have more than one kind of process group a set of processes > can belong to. Yes. That's why my system supports multiple unrelated hierarchies of groups. > > I agree that namespaces fit slightly less well into this model than > > some other subsystems like resource management. But by integrating > > with it you'd get automatic access to all the various different > > resource controller work that's being done. > > I don't need to integrate with it to get access to the resource > controller work. Right, you could certainly do the extra work of tying your virtual servers together with resource controllers in userspace. But you'd still need an API to allow those resource controllers to be associated with groups of processes and configured with limits/guarantees, which is one of the main aims of my cont…
creator: Paul Menage
date: 2007-02-20T22:47:36-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Paul Menage wrote: >> The term "segregated group of processes" is too vague. Segregated for >> what? What is the kernel supposed to do with this information? >> > The generic part of the kernel just keeps track of the fact that > they're segregated (and their children, etc). > > It's the clients of this subsystem (virtual servers, resource > controllers) that can decide to give different per-process behaviour > based on the membership of various groups. > So those clients can use helper functions to use their own namespaces. If they happen to group things in the same way they they'll all end up using the same nsproxy. >> Did you like the names I came up with in my original reply? >> >> - CPUset namespace for CPU partitioning >> - Resource namespaces: >> - cpusched namespace for CPU >> - ulimit namespace for memory >> - quota namespace for disk space >> - io namespace for disk activity >> - etc >> > > This is a strange abuse of the term "namespace". > http://en.wikipedia.org/wiki/Namespace_%28computer_science%29 > > For the virtual server work that you're doing, namespace is a good term: > > pids name processes, hence a pid namespace lets you have multiple > distinct mappings from pids to processes > filenames name files, so a filename (or mount) namespace lets you have > multiple distinct mappings from filenames to files. > > For resource QoS control, it doesn't really make sense to talk about > namespaces. We're not virtualizing resources to rename them for > different virtual servers, we're limiting the quality of access to the > resources. > > But the semantics of the term "namespace" notwithstanding, you're > equating a virtual server namespace (pid, ipc, uts, mounts, etc) with > a resource controller (memory, I/O, etc) in terms of their place in a > hierarchy, which is something I agree with. All of these subsystems > can be considered to be units that can be associated with groups of > processes; the ultimate grouping of processes is something that we're > bot…
creator: Sam Vilain
date: 2007-02-20T22:58:13-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Paul Menage wrote: >> No. A reverse mapping is not needed and is not interesting. >> > ... to you. > You're missing the point of Eric's next sentence. If you can achieve everything you need to achieve and get all the information you are after without it, then it is uninteresting. >> As long as I can walk all processes and ask what namespace are >> you in I don't care. >> > > How do you currently do that? > Take a look at /proc/PID/mounts for example. >> All that is necessary to have a group of processes do something >> in an unnamed fashion is to hang a pointer off of the task_struct. >> That's easy. >> > Right, adding a pointer to task_struct is easy. Configuring how/when > to not directly inherit it from the parent, or to change it for a > running task, or configuring state associated with the thing that the > pointer is pointing to, naming that group, and determining which group > a given process is assocaited with, is something that's effectively > repeated boiler plate for each different subsystem, and which can be > accomplished more generically via an abstraction like my containers > patch. > So make helpers. Macros. Anything, just don't introduce model limitations like the container structure, because we've already got the structure; the nsproxy. Sam. _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Sam Vilain
date: 2007-02-20T23:08:05-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/20/07, Sam Vilain <sam@vilain.net> wrote: > > I don't necessarily agree with the 'heirarchy' bit. It doesn't have to > be so segregated. But I think we already covered that in this thread. OK, but it's much easier to use a hierarchical system as a flat system (just don't create children) than a flat system as a hierarchical system. And others do seem to want a hierarchy for their process groupings. > > I agree with the comment on the abuse of the term "namespace", though > consider that it has already been abused with the term IPC namespaces. That doesn't seem like an abuse to me at all - you're controlling what IPC object a given name (shm_id, sem_id or msg_id) refers to for any given group of processes. > We have for some time been using it to refer to groupable entities > within the kernel that are associated with tasks, even if they don't > involve named entities that clash within a particular domain. But there > is always an entity and a domain, and that is the key point I'm trying > to make - the features you are putting forward are no different to the > examples that we made specifically for the purpose of setting the > standard for further features to follow. They're very similar, I agree. An important difference is that things like pid/mount namespaces are simply ways of controlling the visibility of existing unix concepts, such as processes or filesystems. You don't need additional configuration to make them useful, as unix already has standard ways of manipulating them. Things like resource controllers typically require additional configuration to control how much resources each group of processes can consume, etc. Also, it appears to be much more common to want to move tasks between different resource controllers than to move them between different namespaces. (And in order to configure and move, you need to be able to name) The configuration, naming and movement are the features that my container patch provides on top of the features that nsproxy p…
creator: Paul Menage
date: 2007-02-20T23:28:07-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Quoting Paul Menage (menage@google.com): > On 2/20/07, Eric W. Biederman <ebiederm@xmission.com> wrote: > >All that is necessary to have a group of processes do something > >in an unnamed fashion is to hang a pointer off of the task_struct. > >That's easy. > > Right, adding a pointer to task_struct is easy. Configuring how/when > to not directly inherit it from the parent, or to change it for a > running task, or configuring state associated with the thing that the > pointer is pointing to, naming that group, and determining which group > a given process is assocaited with, is something that's effectively > repeated boiler plate for each different subsystem, and which can be > accomplished more generically via an abstraction like my containers > patch. Eric, what you gain with this patchset is, one very simple container subsystem can tie a container to a cpu, another can limit it's RSS, and suddenly you can mount -t container -o ns,rss,cpuwhatever ns /container And each virtual server you create by unsharing can get automatic cpu and rss controls. That is worthwhile imo. -serge _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: serue
date: 2007-02-20T23:32:22-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: On 2/20/07, Sam Vilain <sam@vilain.net> wrote: > Paul Menage wrote: > >> No. A reverse mapping is not needed and is not interesting. > >> > > ... to you. > > > > You're missing the point of Eric's next sentence. If you can achieve > everything you need to achieve and get all the information you are after > without it, then it is uninteresting. Yes, you can do it with an exhaustive trawl of /proc. That can be very expensive on busy machines. > > >> As long as I can walk all processes and ask what namespace are > >> you in I don't care. > >> > > > > How do you currently do that? > > > > Take a look at /proc/PID/mounts for example. That doesn't tell you what mounts namespace a process is in - it tells you what the process can *view* in the namespace. > > So make helpers. Macros. Anything, just don't introduce model > limitations like the container structure, because we've already got the > structure; the nsproxy. > As I mentioned in another email, nsproxy is fine for things that don't need explicit configuration or reporting, or which already have configuration methods (such as fork(), mount(), etc) available, and which don't need to support movement of processes between different "namespaces". If it was extended to support the naming/config/movement, then it would be fine to use it as the equivalent of a container. I'd actually be interested in trying to combine my container object and the nsproxy object into a single concept. Paul _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: Paul Menage
date: 2007-02-20T23:36:18-00:00

Re: [PATCH 0/7] containers (V7): Generic Process Containers
type: item
description: Quoting Sam Vilain (sam@vilain.net): > Paul Menage wrote: > We talked about naming a bit before, see > http://lkml.org/lkml/2006/3/21/403 and possibly other threads. So, > anyway, feel free to flog this old dead horse and suggest different > terms. We've all had long enough to think about it since so maybe it's Bah, Paul, how about s/container/rug/, as in resource usage group? At least you can be pretty sure that (a) rug won't conflict with any other computer science therm, and (b) for that exact reason it should be pretty memorable. -serge _______________________________________________ Containers mailing list Containers@lists.osdl.org https://lists.osdl.org/mailman/listinfo/containers
creator: serue
date: 2007-02-20T23:37:06-00:00

Re: [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Mon, Feb 12, 2007 at 12:15:22AM -0800, menage@google.com wrote: > +/** > + * container_lock - lock out any changes to container structures > + * > + * The out of memory (oom) code needs to mutex_lock containers > + * from being changed while it scans the tasklist looking for a > + * task in an overlapping container. Which specific portion of oom code cares abt container structure being intact? If I understand correctly, only cpuset requires this double locking. More specifically, cpusets cares about walking cpuset->parent list safely with callback_mutex held correct? If that is the case, I think we can push container_lock entirely inside cpuset.c and not have others exposed to this double-lock complexity. This is possible because cpuset.c (build on top of containers) still has cpuset->parent and walking cpuset->parent list safely can be made possible with a second lock which is local to only cpuset.c. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-07T12:21:17-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Wed, Mar 07, 2007 at 05:51:17PM +0530, Srivatsa Vaddagiri wrote: > If that is the case, I think we can push container_lock entirely inside > cpuset.c and not have others exposed to this double-lock complexity. > This is possible because cpuset.c (build on top of containers) still has > cpuset->parent and walking cpuset->parent list safely can be made > possible with a second lock which is local to only cpuset.c. Hope I am not shooting in the dark here! If we can indeed avoid container_lock in generic code, it will simplify code like attach_task (for ex: post_attach/attach can be clubbed into one single callback). -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-07T14:06:04-00:00

Re: [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: On Mon, Feb 12, 2007 at 12:15:23AM -0800, menage@google.com wrote: > /* > @@ -913,12 +537,14 @@ static int update_nodemask(struct cpuset > int migrate; > int fudge; > int retval; > + struct container *cont; This seems to be redundant? -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-07T14:34:12-00:00

Re: [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: On Mon, Feb 12, 2007 at 12:15:23AM -0800, menage@google.com wrote: > - mutex_lock(&callback_mutex); > - list_add(&cs->sibling, &cs->parent->children); > + cont->cpuset = cs; > + cs->container = cont; > number_of_cpusets++; > - mutex_unlock(&callback_mutex); What's the rule to read/write number_of_cpusets? The earlier cpuset code was incrementing/decrementing under callback_mutex, but now we aren't. How safe is that? The earlier cpuset code also was reading number_of_cpusets w/o the callback_mutex held (atleast in cpuset_zone_allowed_softwall). Is that safe? -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-07T14:52:57-00:00

Re: [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: On 3/7/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > On Mon, Feb 12, 2007 at 12:15:23AM -0800, menage@google.com wrote: > > /* > > @@ -913,12 +537,14 @@ static int update_nodemask(struct cpuset > > int migrate; > > int fudge; > > int retval; > > + struct container *cont; > > This seems to be redundant? It gets used in the lower loop checking for processes whose memory policies we should be rebinding. Paul
creator: Paul Menage
date: 2007-03-07T16:01:32-00:00

Re: [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: On 3/7/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > On Mon, Feb 12, 2007 at 12:15:23AM -0800, menage@google.com wrote: > > - mutex_lock(&callback_mutex); > > - list_add(&cs->sibling, &cs->parent->children); > > + cont->cpuset = cs; > > + cs->container = cont; > > number_of_cpusets++; > > - mutex_unlock(&callback_mutex); > > What's the rule to read/write number_of_cpusets? The earlier cpuset code was > incrementing/decrementing under callback_mutex, but now we aren't. How safe is > that? We're still inside manage_mutex, so we guarantee that no-one else is changing it. > > The earlier cpuset code also was reading number_of_cpusets w/o the > callback_mutex held (atleast in cpuset_zone_allowed_softwall). Is that safe? Yes, I think so. Unless every memory allocator was to hold a lock for the duration of alloc_pages(), number_of_cpusets can theoretically be out of date by the time you're using it. But since the process could have allocated just before you created the first cpuset and moved it into that cpuset anywa, it's not really a race (and the consequences are inconsequential). Paul
creator: Paul Menage
date: 2007-03-07T16:12:20-00:00

Re: [ckrm-tech] [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: On Wed, Mar 07, 2007 at 08:01:32AM -0800, Paul Menage wrote: > > > @@ -913,12 +537,14 @@ static int update_nodemask(struct cpuset > > > int migrate; > > > int fudge; > > > int retval; > > > + struct container *cont; > > > > This seems to be redundant? > > It gets used in the lower loop checking for processes whose memory > policies we should be rebinding. It makes sense in the first cpuset patch (cpusets_using_containers.patch), but should be removed in the second cpuset patch (multiuser_container.patch). In the 2nd patch, we use this comparison: if (task_cs(p) != cs) continue; cont variable introduced in the 1st patch essentially becomes unused after 2nd patch. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-07T16:24:15-00:00

Re: [ckrm-tech] [PATCH 2/7] containers (V7): Cpusets hooked into containers
type: item
description: On 3/7/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > It makes sense in the first cpuset patch > (cpusets_using_containers.patch), but should be removed in the second > cpuset patch (multiuser_container.patch). In the 2nd patch, we use this > comparison: > > if (task_cs(p) != cs) > continue; > > cont variable introduced in the 1st patch essentially becomes unused > after 2nd patch. > Yes, you're right - it could be removed in the 3rd patch. Paul
creator: Paul Menage
date: 2007-03-07T16:31:26-00:00

Re: [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On 3/7/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > If that is the case, I think we can push container_lock entirely inside > cpuset.c and not have others exposed to this double-lock complexity. > This is possible because cpuset.c (build on top of containers) still has > cpuset->parent and walking cpuset->parent list safely can be made > possible with a second lock which is local to only cpuset.c. > The callback mutex (which is what container_lock() actually locks) is also used to synchronize fork/exit against subsystem additions, in the event that some subsystem has registered fork or exit callbacks. We could probably have a separate subsystem_mutex for that instead. Apart from that, yes, it may well be possible to move callback lock entirely inside cpusets. Paul
creator: Paul Menage
date: 2007-03-07T20:50:03-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Wed, Mar 07, 2007 at 12:50:03PM -0800, Paul Menage wrote: > The callback mutex (which is what container_lock() actually locks) is > also used to synchronize fork/exit against subsystem additions, in the > event that some subsystem has registered fork or exit callbacks. We > could probably have a separate subsystem_mutex for that instead. Why can't manage_mutex itself be used there (to serialize fork/exit callbacks against modification to hierarchy)? > Apart from that, yes, it may well be possible to move callback lock > entirely inside cpusets. Yes, that way only the hierarchy hosting cpusets takes the hit of double-locking. cpuset_subsys->create/destroy can take this additional lock inside cpuset.c. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-08T10:31:05-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On 3/8/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > On Wed, Mar 07, 2007 at 12:50:03PM -0800, Paul Menage wrote: > > The callback mutex (which is what container_lock() actually locks) is > > also used to synchronize fork/exit against subsystem additions, in the > > event that some subsystem has registered fork or exit callbacks. We > > could probably have a separate subsystem_mutex for that instead. > > Why can't manage_mutex itself be used there (to serialize fork/exit callbacks > against modification to hierarchy)? Because manage_mutex can be held for very long periods of time. I think that a combination of a new lock that's only taken by fork/exit and register_subsys, plus task_lock (which prevents the current task from being moved) would be more lightweight. Paul
creator: Paul Menage
date: 2007-03-08T10:40:30-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: vatsa wrote: > Yes, that way only the hierarchy hosting cpusets takes the hit of > double-locking. cpuset_subsys->create/destroy can take this additional lock > inside cpuset.c. The primary reason for the cpuset double locking, as I recall, was because cpusets needs to access cpusets inside the memory allocator. A single, straight forward, cpuset lock failed under the following common scenario: 1) user does cpuset system call (writes some file below /dev/cpuset, e.g.) 2) kernel cpuset code locks its lock 3) cpuset code asks to allocate some memory for some cpuset structure 4) memory allocator tries to lock the cpuset lock - deadlock! The reason that the memory allocator needs the cpuset lock is to check whether the memory nodes the current task is allowed to use have changed, due to changes in the current tasks cpuset. A secondary reason that the cpuset code needs two locks is because the main cpuset lock is a long held, system wide lock, and various low level bits of performance critical code sometimes require quick, read-only access to cpusets. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-03-11T19:38:43-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Sun, Mar 11, 2007 at 12:38:43PM -0700, Paul Jackson wrote: > The primary reason for the cpuset double locking, as I recall, was because > cpusets needs to access cpusets inside the memory allocator. "needs to access cpusets" - can you be more specific? Being able to safely walk cpuset->parent list - is it the only access you are talking of or more? -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-12T14:19:05-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Mon, Feb 12, 2007 at 12:15:22AM -0800, menage@google.com wrote: > +static void remove_dir(struct dentry *d) > +{ > + struct dentry *parent = dget(d->d_parent); Don't we need to lock parent inode's mutex here? sysfs seems to take that lock. > + > + d_delete(d); > + simple_rmdir(parent->d_inode, d); > + dput(parent); > +} -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-22T09:56:51-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Thu, Mar 22, 2007 at 03:26:51PM +0530, Srivatsa Vaddagiri wrote: > On Mon, Feb 12, 2007 at 12:15:22AM -0800, menage@google.com wrote: > > +static void remove_dir(struct dentry *d) > > +{ > > + struct dentry *parent = dget(d->d_parent); > > Don't we need to lock parent inode's mutex here? sysfs seems to take > that lock. Never mind. Maneesh pointed me to do_rmdir() in VFS which takes that mutex as well. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-22T10:20:32-00:00

Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: On Mon, Feb 12, 2007 at 12:15:28AM -0800, menage@google.com wrote: > +/* > + * Rules: you can only create a container if > + * 1. you are capable(CAP_SYS_ADMIN) > + * 2. the target container is a descendant of your own container > + */ > +static int ns_create(struct container_subsys *ss, struct container *cont) > +{ > + struct nscont *ns; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; Does this check break existing namespace semantics in a subtle way? It now requires that unshare() of namespaces by any task requires CAP_SYS_ADMIN capabilities. clone(.., CLONE_NEWUTS, ..)->copy_namespaces()->ns_container_clone()-> ->container_clone()-> .. -> container_create() -> ns_create() Earlier, one could unshare his uts namespace w/o CAP_SYS_ADMIN capabilities. Now it is required. Is that fine? Don't know. I feel we can avoid this check totally and let the directory permissions take care of these checks. Serge, what do you think? -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-24T04:58:36-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Mon, Feb 12, 2007 at 12:15:22AM -0800, menage@google.com wrote: > +static int attach_task(struct container *cont, char *pidbuf, char **ppathbuf) > +{ > + pid_t pid; > + struct task_struct *tsk; > + struct container *oldcont; > + int retval; > + > + if (sscanf(pidbuf, "%d", &pid) != 1) > + return -EIO; > + > + if (pid) { > + read_lock(&tasklist_lock); > + > + tsk = find_task_by_pid(pid); > + if (!tsk || tsk->flags & PF_EXITING) { This is probably carrying over code from cpuset.c, but : /me thinks that there is a ugly race here with 'tsk' exiting. What happens if the tsk is marked PF_EXITING just after this check? If that happens, then: > + read_unlock(&tasklist_lock); > + return -ESRCH; > + } > + > + get_task_struct(tsk); > + read_unlock(&tasklist_lock); > + > + if ((current->euid) && (current->euid != tsk->uid) > + && (current->euid != tsk->suid)) { > + put_task_struct(tsk); > + return -EACCES; > + } > + } else { > + tsk = current; > + get_task_struct(tsk); > + } > + > + retval = security_task_setscheduler(tsk, 0, NULL); > + if (retval) { > + put_task_struct(tsk); > + return retval; > + } > + > + mutex_lock(&callback_mutex); > + > + task_lock(tsk); > + oldcont = tsk->container; > + if (!oldcont) { > + task_unlock(tsk); > + mutex_unlock(&callback_mutex); > + put_task_struct(tsk); > + return -ESRCH; > + } > + atomic_inc(&cont->count); > + rcu_assign_pointer(tsk->container, cont); Above assignment A1 can race with below assignment A2 in container_exit() : tsk->container = &top_container; /* the_top_container_hack - see above */ What happens if A1 follows after A2? I feel very uncomfortable abt it. IMO, we need to use task_lock() in container_exit() to avoid this race. (I think this race already exists in mainline cpuset.c?) P.S : cpuset.c checks for PF_EXITING twice in attach_task(), while this patch seems to be checking only once. Is that fine? -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-24T14:58:00-00:00

Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: On Sat, Mar 24, 2007 at 10:35:37AM +0530, Srivatsa Vaddagiri wrote: > > +static int ns_create(struct container_subsys *ss, struct container *cont) > > +{ > > + struct nscont *ns; > > + > > + if (!capable(CAP_SYS_ADMIN)) > > + return -EPERM; > > Does this check break existing namespace semantics in a subtle way? > It now requires that unshare() of namespaces by any task requires > CAP_SYS_ADMIN capabilities. I should clarify that I am referring to unshare thr' clone here (and not thr' sys_unshare) > clone(.., CLONE_NEWUTS, ..)->copy_namespaces()->ns_container_clone()-> > ->container_clone()-> .. -> container_create() -> ns_create() > > Earlier, one could unshare his uts namespace w/o CAP_SYS_ADMIN > capabilities. Now it is required. Is that fine? Don't know. > > I feel we can avoid this check totally and let the directory permissions > take care of these checks. > > Serge, what do you think? -- Regards, vatsa _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Srivatsa Vaddagiri
date: 2007-03-24T16:23:26-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: > IMO, we need to use task_lock() in container_exit() to avoid this race. > > (I think this race already exists in mainline cpuset.c?) > > P.S : cpuset.c checks for PF_EXITING twice in attach_task(), while this > patch seems to be checking only once. Is that fine? I think the cpuset code is ok, because, as you note, it locks the task, picks off the cpuset pointer, and then checks a second time that the task still does not have PF_EXITING set: In the kernel/cpuset.c code for attach_task(): task_lock(tsk); oldcs = tsk->cpuset; /* * After getting 'oldcs' cpuset ptr, be sure still not exiting. * If 'oldcs' might be the top_cpuset due to the_top_cpuset_hack * then fail this attach_task(), to avoid breaking top_cpuset.count. */ if (tsk->flags & PF_EXITING) { task_unlock(tsk); mutex_unlock(&callback_mutex); put_task_struct(tsk); return -ESRCH; } -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-03-24T19:25:59-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Sat, Mar 24, 2007 at 12:25:59PM -0700, Paul Jackson wrote: > > P.S : cpuset.c checks for PF_EXITING twice in attach_task(), while this > > patch seems to be checking only once. Is that fine? > > I think the cpuset code is ok, because, as you note, it locks the task, > picks off the cpuset pointer, and then checks a second time that the > task still does not have PF_EXITING set: Well afaics, PF_EXITING is set for the exiting task w/o taking any lock, which makes this racy always. > In the kernel/cpuset.c code for attach_task(): > > task_lock(tsk); > oldcs = tsk->cpuset; > /* > * After getting 'oldcs' cpuset ptr, be sure still not exiting. > * If 'oldcs' might be the top_cpuset due to the_top_cpuset_hack > * then fail this attach_task(), to avoid breaking top_cpuset.count. > */ > if (tsk->flags & PF_EXITING) { What if PF_EXITING is set after this check? If that happens then, > task_unlock(tsk); > mutex_unlock(&callback_mutex); > put_task_struct(tsk); > return -ESRCH; > } the following code becomes racy with cpuset_exit() ... atomic_inc(&cs->count); rcu_assign_pointer(tsk->cpuset, cs); task_unlock(tsk); -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-25T00:38:29-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: vatsa wrote: > > if (tsk->flags & PF_EXITING) { > > What if PF_EXITING is set after this check? If that happens then, > > > task_unlock(tsk); > > mutex_unlock(&callback_mutex); > > put_task_struct(tsk); > > return -ESRCH; > > } > > the following code becomes racy with cpuset_exit() ... > > atomic_inc(&cs->count); > rcu_assign_pointer(tsk->cpuset, cs); > task_unlock(tsk); eh ... so ... ? I don't know of any sequence where that causes any problem. Do you see one? -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-03-25T01:41:28-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Sat, Mar 24, 2007 at 06:41:28PM -0700, Paul Jackson wrote: > > the following code becomes racy with cpuset_exit() ... > > > > atomic_inc(&cs->count); > > rcu_assign_pointer(tsk->cpuset, cs); > > task_unlock(tsk); > > eh ... so ... ? > > I don't know of any sequence where that causes any problem. > > Do you see one? Let's say we had two cpusets CS1 amd CS2 (both different from top_cpuset). CS1 has just one task T1 in it (CS1->count = 0) while CS2 has no tasks in it (CS2->count = 0). Now consider: ------------------------------------------------------------ -------- CPU0 (attach_task T1 to CS2) CPU1 (T1 is exiting) ------------------------------------------------------------ -------- task_lock(T1); oldcs = tsk->cpuset; [oldcs = CS1] T1->flags & PF_EXITING? (No) T1->flags = PF_EXITING; atomic_inc(&CS2->count); cpuset_exit() cs = tsk->cpuset; (cs = CS1) T1->cpuset = CS2; T1->cpuset = &top_cpuset; task_unlock(T1); CS2 has one bogus count now (with no tasks in it), which may prevent it from being removed/freed forever. Not just this, continuing further we have more trouble: ------------------------------------------------------------ -------- CPU0 (attach_task T1 to CS2) CPU1 (T1 is exiting) ------------------------------------------------------------ -------- synchronize_rcu() atomic_dec(&CS1->count); [CS1->count = 0] if atomic_dec_and_test(&oldcs->count)) [CS1->count = -1] We now have CS1->count negative. Is that good? I am uncomfortable .. We need a task_lock() in cpuset_exit to avoid this race. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-25T02:21:09-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Sun, Mar 25, 2007 at 07:58:16AM +0530, Srivatsa Vaddagiri wrote: > Not just this, continuing further we have more trouble: > > ------------------------------------------------------------ -------- > CPU0 (attach_task T1 to CS2) CPU1 (T1 is exiting) > ------------------------------------------------------------ -------- > > synchronize_rcu() > atomic_dec(&CS1->count); > [CS1->count = 0] > > if atomic_dec_and_test(&oldcs->count)) > [CS1->count = -1] > > > > We now have CS1->count negative. Is that good? I am uncomfortable .. > > We need a task_lock() in cpuset_exit to avoid this race. 2nd race is tricky. We probably need to do this to avoid it: task_lock(tsk); /* Check if tsk->cpuset is still same. We may have raced with * cpuset_exit changing tsk->cpuset again under our feet. */ if (tsk->cpuset == cs && atomic_dec_and_test(&oldcs->count)) { task_unlock(tsk); check_for_release(oldcs, ppathbuf); goto done; } task_unlock(tsk); done: return 0; -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-25T04:16:02-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: vatsa wrote: > Now consider: Nice work - thanks. Yes, both an extra cpuset count and a negative cpuset count are bad news, opening the door to the usual catastrophes. Would you like the honor of submitting the patch to add a task_lock to cpuset_exit()? If you do, be sure to fix, or at least remove, the cpuset_exit comment lines: * We don't need to task_lock() this reference to tsk->cpuset, * because tsk is already marked PF_EXITING, so attach_task() won't * mess with it, or task is a failed fork, never visible to attach_task. I guess that taking task_lock() in cpuset_exit() should not be a serious performance issue. It's taking a spinlock that is in the current exiting tasks task struct, so it should be a cache hot memory line and a rarely contested lock. And I guess I've not see this race in real life, as one side of it has to execute quite a bit of code in the task exit path, from when it sets PF_EXITING until it gets into the cpuset_exit() call, while the other side does the three lines: if (tsk->flags & PF_EXITING) ... atomic_inc(&cs->count); rcu_assign_pointer(tsk->cpuset, cs); So, in real life, this would be a difficult race to trigger. Thanks for finding this. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-03-25T04:45:50-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Sat, Mar 24, 2007 at 09:45:50PM -0700, Paul Jackson wrote: > Nice work - thanks. Yes, both an extra cpuset count and a negative > cpuset count are bad news, opening the door to the usual catastrophes. > > Would you like the honor of submitting the patch to add a task_lock > to cpuset_exit()? If you do, be sure to fix, or at least remove, > the cpuset_exit comment lines: I will try to send out a patch later today to fix this bug in mainline cpuset code. I happened to notice this race with my rcfs patch and observed same is true with cpuset/container code also. > * We don't need to task_lock() this reference to tsk->cpuset, > * because tsk is already marked PF_EXITING, so attach_task() won't > * mess with it, or task is a failed fork, never visible to attach_task. Sure, I had seen that. > So, in real life, this would be a difficult race to trigger. Agreed, but good to keep code clean isn't it? :) > Thanks for finding this. Wellcome! -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-25T04:57:52-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: > I will try to send out a patch later today to fix Thanks! > Agreed, but good to keep code clean isn't it? :) Definitely. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-03-25T04:59:16-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: vatsa wrote: > Not just this, continuing further we have more trouble: > > ------------------------------------------------------------ -------- > CPU0 (attach_task T1 to CS2) CPU1 (T1 is exiting) > ------------------------------------------------------------ -------- > > synchronize_rcu() > atomic_dec(&CS1->count); > [CS1->count = 0] > > if atomic_dec_and_test(&oldcs->count)) > [CS1->count = -1] > ... > 2nd race is tricky. We probably need to do this to avoid it: > > task_lock(tsk); > > /* Check if tsk->cpuset is still same. We may have raced with > * cpuset_exit changing tsk->cpuset again under our feet. > */ > if (tsk->cpuset == cs && atomic_dec_and_test(&oldcs->count)) { I'm unsure here, but this 'tsk->cpuset == cs' test feels fragile to me. How about a bit earlier in attach_task(), right at the point we overwrite the victim tasks cpuset pointer, we decrement the count on the old cpuset, and if it went to zero, remember that we'll need to release it, once we've dropped some locks: static int attach_task(struct cpuset *cs, char *pidbuf, char **ppathbuf) { ... struct cpuset *oldcs; struct cpuset *oldcs_tobe_released; ... task_lock(tsk); oldcs = tsk->cpuset; ... if (tsk->flags & PF_EXITING) { ... } atomic_inc(&cs->count); rcu_assign_pointer(tsk->cpuset, cs); oldcs_tobe_released = NULL; if (atomic_dec_and_test(&oldcs->count)) oldcs_tobe_released = oldcs; task_unlock(tsk); ... put_task_struct(tsk); synchronize_rcu(); if (oldcs_tobe_released) check_for_release(oldcs_tobe_released, ppathbuf); return 0; } -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2007-03-25T05:43:48-00:00

Re: [ckrm-tech] [PATCH 1/7] containers (V7): Generic container system abstracted from cpusets code
type: item
description: On Sat, Mar 24, 2007 at 10:43:48PM -0700, Paul Jackson wrote: > I'm unsure here, but this 'tsk->cpuset == cs' test feels fragile to me. > > How about a bit earlier in attach_task(), right at the point we overwrite the > victim tasks cpuset pointer, we decrement the count on the old cpuset, and if > it went to zero, remember that we'll need to release it, once we've dropped > some locks: Looks neater. Will adopt this when I send the patch. Thanks. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-25T08:14:40-00:00

Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: Quoting Srivatsa Vaddagiri (vatsa@in.ibm.com): > On Mon, Feb 12, 2007 at 12:15:28AM -0800, menage@google.com wrote: > > +/* > > + * Rules: you can only create a container if > > + * 1. you are capable(CAP_SYS_ADMIN) > > + * 2. the target container is a descendant of your own container > > + */ > > +static int ns_create(struct container_subsys *ss, struct container *cont) > > +{ > > + struct nscont *ns; > > + > > + if (!capable(CAP_SYS_ADMIN)) > > + return -EPERM; > > Does this check break existing namespace semantics in a subtle way? > It now requires that unshare() of namespaces by any task requires > CAP_SYS_ADMIN capabilities. > > clone(.., CLONE_NEWUTS, ..)->copy_namespaces()->ns_container_clone()-> > ->container_clone()-> .. -> container_create() -> ns_create() > > Earlier, one could unshare his uts namespace w/o CAP_SYS_ADMIN > capabilities. Not so, CAP_SYS_ADMIN is required. Depending on your tree, check kernel/utsname.c or kernel/nsproxy.c. Mind you, whether there is a reason to require CAP_SYS_ADMIN for utsname I'm not sure. But if there is any way of finding privileged software which would behave differently based on utsname information, then we should. > Now it is required. Is that fine? Don't know. > > I feel we can avoid this check totally and let the directory permissions > take care of these checks. > > Serge, what do you think? There is no way we can think about doing that until we determine that every kernel will have the ns container subsystem compiled in and mounted, right? -serge
creator: serue
date: 2007-03-26T21:55:30-00:00

Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: Quoting Srivatsa Vaddagiri (vatsa@in.ibm.com): > On Sat, Mar 24, 2007 at 10:35:37AM +0530, Srivatsa Vaddagiri wrote: > > > +static int ns_create(struct container_subsys *ss, struct container *cont) > > > +{ > > > + struct nscont *ns; > > > + > > > + if (!capable(CAP_SYS_ADMIN)) > > > + return -EPERM; > > > > Does this check break existing namespace semantics in a subtle way? > > It now requires that unshare() of namespaces by any task requires > > CAP_SYS_ADMIN capabilities. > > I should clarify that I am referring to unshare thr' clone here (and not > thr' sys_unshare) That is still not true, see kernel/utsname:copy_utsname(). Now you might have run a userspace testcase in a kernel with CONFIG_UTS_NS=n, which at the moment erroneously returns 0 rather than -EINVAL when you clone(CLONE_NEWUTS). But you didn't get a new uts namespace, you were just lied to :) -serge _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: serue
date: 2007-03-26T21:57:55-00:00

Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: On Mon, Mar 26, 2007 at 04:57:55PM -0500, Serge E. Hallyn wrote: > That is still not true, see kernel/utsname:copy_utsname(). > > Now you might have run a userspace testcase in a kernel with > CONFIG_UTS_NS=n, which at the moment erroneously returns 0 rather than > -EINVAL when you clone(CLONE_NEWUTS). But you didn't get a new uts > namespace, you were just lied to :) I think you are right here, in that CONFIG_UTS_NS was not turned on, although I was thinking it was on. However as a result of this experiment, I found this anomaly: - On a kernel with CONFIG_UTS_NS=n, a test which does clone(CLONE_NEWUTS) works fine. clone() succeeds and the child starts running with no error. - On the same kernel, if ns container hierarchy is mounted, then the test fails. clone() returns failure and child is never created. As soon as the ns container hierarchy is unmounted, the test works again. I would have expected a consistent behavior here, irrespective of whether ns hierarchy is mounted or not. Is this difference in behavior acceptable? Returning -EINVAL in copy_utsname() when CONFIG_UTS_NS=n, as you say above, would fix this anomaly. -- Regards, vatsa _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Srivatsa Vaddagiri
date: 2007-03-28T14:55:34-00:00

Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: On Mon, Feb 12, 2007 at 12:15:28AM -0800, menage@google.com wrote: > +int ns_container_clone(struct task_struct *tsk) > +{ > + return container_clone(tsk, &ns_subsys); > +} This function is a no-op if ns hierarchy is not mounted at this point. This would mean that we will miss out on some directories in ns hierarchy if it happened to be mounted later. It would be nice to recreate such missing directories upon mount. However I suspect it would not be easy ..Maybe we need to scan the task list and (re-)invoke ns_container_clone() for every new tsk->nsproxy we find in the list. Alternately perhaps we could auto mount (kern_mount) ns hierarchy very early at bootup? On the flip side that would require remount support so that additional controllers (like cpuset, mem) can be bound to (non-empty) ns hierarchy after bootup. -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-03-31T02:47:22-00:00

Re: [ckrm-tech] [PATCH 7/7] containers (V7): Container interface to nsproxy subsystem
type: item
description: Quoting Srivatsa Vaddagiri (vatsa@in.ibm.com): > On Mon, Feb 12, 2007 at 12:15:28AM -0800, menage@google.com wrote: > > +int ns_container_clone(struct task_struct *tsk) > > +{ > > + return container_clone(tsk, &ns_subsys); > > +} > > This function is a no-op if ns hierarchy is not mounted at this point. > This would mean that we will miss out on some directories in ns > hierarchy if it happened to be mounted later. It would be nice to > recreate such missing directories upon mount. However I suspect it would > not be easy ..Maybe we need to scan the task list and (re-)invoke > ns_container_clone() for every new tsk->nsproxy we find in the list. > Alternately perhaps we could auto mount (kern_mount) ns hierarchy very early at > bootup? On the flip side that would require remount support so that additional > controllers (like cpuset, mem) can be bound to (non-empty) ns hierarchy after > bootup. Losing the directory isn't a big deal though. And both unsharing a namespace (which causes a ns_container_clone) and mounting the hierarchy are done by userspace, so if for some installation it is a big deal, the init scripts on initrd can mount the hierarchy before doing any unsharing. Can you think of a reason why losing a few directories matters? -serge
creator: serue
date: 2007-04-02T14:09:39-00:00