OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/9] Containers (V9): Generic Process Containers
items: [PATCH 1/9] Containers (V9): Basic container framework
items: [PATCH 2/9] Containers (V9): Example CPU accounting subsystem
items: [PATCH 3/9] Containers (V9): Add tasks file interface
items: [PATCH 4/9] Containers (V9): Add fork/exit hooks
items: [PATCH 6/9] Containers (V9): Add procfs interface
items: [PATCH 7/9] Containers (V9): Make cpusets a client of containers
items: [PATCH 8/9] Containers (V9): Share css_group arrays between tasks with same container memberships
items: [PATCH 9/9] Containers (V9): Simple debug info subsystem
items: Re: [PATCH 0/9] Containers (V9): Generic Process Containers
items: Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
items: Re: [ckrm-tech] [PATCH 0/9] Containers (V9): Generic Process Containers
items: Re: [PATCH 1/9] Containers (V9): Basic container framework
items: Re: [PATCH 2/9] Containers (V9): Example CPU accounting subsystem
items: Re: [PATCH 3/9] Containers (V9): Add tasks file interface
items: Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface

[PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: -- This is an update to my multi-hierarchy multi-subsystem generic process containers patch. Changes since V8 (April 6th) include: - The patchset has been rebased over 2.6.21-rc7-mm1 - The patchset has been restructured based on feedback; more functionality is now split out into separate patches where practical. - The container_group structure has been renamed css_group since this is more descriptive of its true function - Added a simplified file registration interface, and a simple interface for the common operation of returning a single number to userspace from a container control file - Added a simple "debug" subsystem that is both an example of how to use the container system and a useful debugging tool for checking reference counts, etc. Still TODO: - decide whether "Containers" is an acceptable name for the system given its usage by some other development groups, or whether something else (ProcessSets? ResourceGroups? TaskGroups?) would be better - decide whether merging css_group and nsproxy is desirable - add a hash-table based lookup for css_group objects. - use seq_file properly in container tasks files to avoid having to allocate a big array for all the container's task pointers. - add back support for the "release agent" functionality - lots more testing - define standards for container file names 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. Already existing in the kernel is the cpuset subsystem; this has a process grouping mechanism that is mature, tested, and well doc…
creator: Paul Menage
date: 2007-04-27T10:46:07-00:00

[PATCH 1/9] Containers (V9): Basic container framework
type: item
description: This patch adds the main containers framework - the container filesystem, and the basic structures for tracking membership and associating subsystem state objects to tasks. Signed-off-by: Paul Menage <menage@google.com> --- Documentation/containers.txt | 524 +++++++++++++++++ include/linux/container.h | 198 ++++++ include/linux/container_subsys.h | 10 include/linux/sched.h | 34 + init/Kconfig | 3 init/main.c | 3 kernel/Makefile | 1 kernel/container.c | 1151 +++++++++++++++++++++++++++++++++++++++ 8 files changed, 1923 insertions(+), 1 deletion(-) Index: container-2.6.21-rc7-mm1/Documentation/containers.txt ============================================================ ======= --- /dev/null +++ container-2.6.21-rc7-mm1/Documentation/containers.txt @@ -0,0 +1,524 @@ + CONTAINERS + ------- + +Written by Paul Menage <menage@google.com> based on Documentation/cpusets.txt + +Original copyright statements from cpusets.txt: +Portions Copyright (C) 2004 BULL SA. +Portions Copyright (c) 2004-2006 Silicon Graphics, Inc. +Modified by Paul Jackson <pj@sgi.com> +Modified by Christoph Lameter <clameter@sgi.com> + +CONTENTS: +========= + +1. Containers + 1.1 What are containers ? + 1.2 Why are containers needed ? + 1.3 How are containers implemented ? + 1.4 What does notify_on_release do ? + 1.5 How do I use containers ? +2. Usage Examples and Syntax + 2.1 Basic Usage + 2.2 Attaching processes +3. Kernel API + 3.1 Overview + 3.2 Synchronization + 3.3 Subsystem API +4. Questions + +1. Containers +========== + +1.1 What are containers ? +---------------------- + +Containers provide a mechanism for aggregating/partitioning sets of +tasks, and all their future children, into hierarchical groups with +specialized behaviour. + +Definitions: + +A *container* associates a set of tasks with a set of parameters for one +or more subsystems. + +A *subsystem* is a module that makes use of the task grouping +facilities provided by containers to treat groups of tasks in +particular ways. A subsyste…
creator: Paul Menage
date: 2007-04-27T10:46:08-00:00

[PATCH 2/9] Containers (V9): Example CPU accounting subsystem
type: item
description: This example demonstrates how to use the generic container subsystem for a simple resource tracker that counts, for the processes in a container, the total CPU time used and the %CPU used in the last complete 10 second interval. Portions contributed by Balbir Singh <balbir@in.ibm.com> Signed-off-by: Paul Menage <menage@google.com> --- include/linux/container_subsys.h | 6 + include/linux/cpu_acct.h | 14 ++ init/Kconfig | 7 + kernel/Makefile | 1 kernel/cpu_acct.c | 185 +++++++++++++++++++++++++++++++++++++++ kernel/sched.c | 14 ++ 6 files changed, 224 insertions(+), 3 deletions(-) Index: container-2.6.21-rc7-mm1/include/linux/container_subsys.h ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/include/linux/container_subsys .h +++ container-2.6.21-rc7-mm1/include/linux/container_subsys.h @@ -7,4 +7,10 @@ /* */ +#ifdef CONFIG_CONTAINER_CPUACCT +SUBSYS(cpuacct) +#endif + +/* */ + /* */ Index: container-2.6.21-rc7-mm1/include/linux/cpu_acct.h ============================================================ ======= --- /dev/null +++ container-2.6.21-rc7-mm1/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.21-rc7-mm1/init/Kconfig ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/init/Kconfig +++ container-2.6.21-rc7-mm1/init/Kconfig @@ -322,6 +322,13 @@ config SYSFS_DEPRECATED If you are using a distro that was released in 2006 or later, it should be safe to say N here. +config CONTAINER_CPUACCT + bool "Simple CPU accounting container subsystem" + select CONTAINERS + help + Provides a simple Resource Controller for monitoring the + total CPU…
creator: Paul Menage
date: 2007-04-27T10:46:09-00:00

[PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: This patch adds the per-directory "tasks" file for containerfs mounts; this allows the user to determine which tasks are members of a container by reading a container's "tasks", and to move a task into a container by writing its pid to its "tasks". Signed-off-by: Paul Menage <menage@google.com> --- include/linux/container.h | 2 kernel/container.c | 344 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 346 insertions(+) Index: container-2.6.21-rc7-mm1/include/linux/container.h ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/include/linux/container.h +++ container-2.6.21-rc7-mm1/include/linux/container.h @@ -128,6 +128,8 @@ int container_is_removed(const struct co int container_path(const struct container *cont, char *buf, int buflen); +int container_task_count(const struct container *cont); + /* Return true if the container is a descendant of the current container */ int container_is_descendant(const struct container *cont); Index: container-2.6.21-rc7-mm1/kernel/container.c ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/kernel/container.c +++ container-2.6.21-rc7-mm1/kernel/container.c @@ -676,6 +676,111 @@ static inline void get_first_subsys(cons *subsys_id = test_ss->subsys_id; } +/* + * Attach task 'tsk' to container 'cont' + * + * Call holding container_mutex. May take task_lock of + * the task 'pid' during call. + */ + +static int attach_task(struct container *cont, struct task_struct *tsk) +{ + int retval = 0; + struct container_subsys *ss; + struct container *oldcont; + struct css_group *cg = &tsk->containers; + struct containerfs_root *root = cont->root; + int i; + + int subsys_id; + get_first_subsys(cont, NULL, &subsys_id); + + /* Nothing to do if the task is already in that container */ + oldcont = task_container(tsk, subsys_id); + if (cont == oldcont) + return 0; + + for_each_subsys(root, ss) { + if (ss->can_attach) { + retval = ss-…
creator: Paul Menage
date: 2007-04-27T10:46:10-00:00

[PATCH 4/9] Containers (V9): Add fork/exit hooks
type: item
description: This patch adds the necessary hooks to the fork() and exit() paths to ensure that new children inherit their parent's container assignments, and that exiting processes release reference counts on their containers. Signed-off-by: Paul Menage <menage@google.com> --- include/linux/container.h | 6 ++ kernel/container.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++ kernel/exit.c | 2 kernel/fork.c | 14 ++++- 4 files changed, 146 insertions(+), 2 deletions(-) Index: container-2.6.21-rc7-mm1/kernel/exit.c ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/kernel/exit.c +++ container-2.6.21-rc7-mm1/kernel/exit.c @@ -32,6 +32,7 @@ #include <linux/taskstats_kern.h> #include <linux/delayacct.h> #include <linux/cpuset.h> +#include <linux/container.h> #include <linux/syscalls.h> #include <linux/signal.h> #include <linux/posix-timers.h> @@ -939,6 +940,7 @@ fastcall NORET_TYPE void do_exit(long co __exit_fs(tsk); exit_thread(); cpuset_exit(tsk); + container_exit(tsk, 1); exit_keys(tsk); if (group_dead && tsk->signal->leader) Index: container-2.6.21-rc7-mm1/kernel/fork.c ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/kernel/fork.c +++ container-2.6.21-rc7-mm1/kernel/fork.c @@ -30,6 +30,7 @@ #include <linux/capability.h> #include <linux/cpu.h> #include <linux/cpuset.h> +#include <linux/container.h> #include <linux/security.h> #include <linux/swap.h> #include <linux/syscalls.h> @@ -962,6 +963,7 @@ static struct task_struct *copy_process( { int retval; struct task_struct *p = NULL; + int container_callbacks_done = 0; if ((clone_flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS)) return ERR_PTR(-EINVAL); @@ -1061,12 +1063,13 @@ static struct task_struct *copy_process( p->io_wait = NULL; p->audit_context = NULL; cpuset_fork(p); + container_fork(p); #ifdef CONFIG_NUMA p->mempolicy = mpol_copy(p->mempolicy); if (IS_ERR(p->mempolicy)) { retval = PTR_ERR(p->mempolicy); p-…
creator: Paul Menage
date: 2007-04-27T10:46:11-00:00

[PATCH 6/9] Containers (V9): Add procfs interface
type: item
description: This patch adds: /proc/containers - general system info /proc/*/container - per-task container membership info Signed-off-by: Paul Menage <menage@google.com> --- fs/proc/base.c | 7 ++ kernel/container.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) Index: container-2.6.21-rc7-mm1/fs/proc/base.c ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/fs/proc/base.c +++ container-2.6.21-rc7-mm1/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> @@ -1980,6 +1981,9 @@ static const struct pid_entry tgid_base_ #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 @@ -2270,6 +2274,9 @@ static const struct pid_entry tid_base_s #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.21-rc7-mm1/kernel/container.c ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/kernel/container.c +++ container-2.6.21-rc7-mm1/kernel/container.c @@ -247,6 +247,7 @@ static int container_mkdir(struct inode static int container_rmdir(struct inode *unused_dir, struct dentry *dentry); static int container_populate_dir(struct container *cont); static struct inode_operations container_dir_inode_operations; +struct file_operations proc_containerstats_operations; static struct backing_dev_info container_backing_dev_info = { .ra_pages = 0, /* No readahead */ @@ -1507,6 +1508,7 @@ int __init containe…
creator: Paul Menage
date: 2007-04-27T10:46:13-00:00

[PATCH 7/9] Containers (V9): Make cpusets a client of containers
type: item
description: This patch removes the filesystem support logic from the cpusets system and makes cpusets a container subsystem Signed-off-by: Paul Menage <menage@google.com> --- Documentation/cpusets.txt | 91 +-- fs/proc/base.c | 4 include/linux/container_subsys.h | 6 include/linux/cpuset.h | 12 include/linux/mempolicy.h | 12 include/linux/sched.h | 3 init/Kconfig | 6 kernel/cpuset.c | 1146 +++++---------------------------------- kernel/exit.c | 2 kernel/fork.c | 3 mm/mempolicy.c | 2 11 files changed, 236 insertions(+), 1051 deletions(-) Index: container-2.6.21-rc7-mm1/Documentation/cpusets.txt ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/Documentation/cpusets.txt +++ container-2.6.21-rc7-mm1/Documentation/cpusets.txt @@ -7,6 +7,7 @@ Written by Simon.Derr@bull.net Portions Copyright (c) 2004-2006 Silicon Graphics, Inc. Modified by Paul Jackson <pj@sgi.com> Modified by Christoph Lameter <clameter@sgi.com> +Modified by Paul Menage <menage@google.com> CONTENTS: ========= @@ -16,10 +17,9 @@ CONTENTS: 1.2 Why are cpusets needed ? 1.3 How are cpusets implemented ? 1.4 What are exclusive cpusets ? - 1.5 What does notify_on_release do ? - 1.6 What is memory_pressure ? - 1.7 What is memory spread ? - 1.8 How do I use cpusets ? + 1.5 What is memory_pressure ? + 1.6 What is memory spread ? + 1.7 How do I use cpusets ? 2. Usage Examples and Syntax 2.1 Basic Usage 2.2 Adding/removing cpus @@ -43,18 +43,19 @@ hierarchy visible in a virtual file syst hooks, beyond what is already present, required to manage dynamic job placement on large systems. -Each task has a pointer to a cpuset. Multiple tasks may reference -the same cpuset. Requests by a task, using the sched_setaffinity(2) -system call to include CPUs in its CPU affinity mask, and using the -mbind(2) and set_mempolicy(2) system calls to include Memory Nodes -in its memory policy, are both filtered through that tasks cpuset, -filtering out any CPUs or Memory Nodes not in that cpuset…
creator: Paul Menage
date: 2007-04-27T10:46:14-00:00

[PATCH 8/9] Containers (V9): Share css_group arrays between tasks with same container memberships
type: item
description: This patch replaces the struct css_group embedded in task_struct with a pointer; all tasks with the same set of memberships across all hierarchies will share a css_group object. The css_group used by init isn't refcounted, since it can't ever be freed; this speeds up fork/exit for any systems that have containers compiled in but haven't actually created any containers other than the default one. With more than one registered subsystem, this reduces the number of atomic inc/dec operations required when tasks fork/exit; Assuming that many tasks share the same container assignments, this reduces overall space usage and keeps the size of the task_struct down (only one pointer added to task_struct compared to a non-containers kernel). Signed-off-by: Paul Menage <menage@google.com> --- include/linux/container.h | 35 ++++++ include/linux/sched.h | 30 ----- kernel/container.c | 248 +++++++++++++++++++++++++++++++++++++--------- 3 files changed, 238 insertions(+), 75 deletions(-) Index: container-2.6.21-rc7-mm1/include/linux/container.h ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/include/linux/container.h +++ container-2.6.21-rc7-mm1/include/linux/container.h @@ -29,6 +29,14 @@ extern void container_unlock(void); struct containerfs_root; +/* Define the enumeration of all container subsystems */ +#define SUBSYS(_x) _x ## _subsys_id, +enum container_subsys_id { +#include <linux/container_subsys.h> + CONTAINER_SUBSYS_COUNT +}; +#undef SUBSYS + /* Per-subsystem/per-container state maintained by the system. */ struct container_subsys_state { /* The container that this subsystem is attached to. Useful @@ -87,6 +95,31 @@ struct container { struct container *top_container; }; +/* A css_group is a structure holding pointers to a set of + * container_subsys_state objects. This saves space in the task struct + * object and speeds up fork()/exit(), since a single inc/dec can bump + * the reference count on the entire container s…
creator: Paul Menage
date: 2007-04-27T10:46:15-00:00

[PATCH 9/9] Containers (V9): Simple debug info subsystem
type: item
description: This example subsystem exports debugging information as an aid to diagnosing refcount leaks, etc, in the container framework. Signed-off-by: Paul Menage <menage@google.com> --- include/linux/container_subsys.h | 4 + init/Kconfig | 10 ++++ kernel/Makefile | 1 kernel/container_debug.c | 89 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+) Index: container-2.6.21-rc7-mm1/include/linux/container_subsys.h ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/include/linux/container_subsys .h +++ container-2.6.21-rc7-mm1/include/linux/container_subsys.h @@ -19,4 +19,8 @@ SUBSYS(cpuset) /* */ +#ifdef CONFIG_CONTAINER_DEBUG +SUBSYS(debug) +#endif + /* */ Index: container-2.6.21-rc7-mm1/init/Kconfig ============================================================ ======= --- container-2.6.21-rc7-mm1.orig/init/Kconfig +++ container-2.6.21-rc7-mm1/init/Kconfig @@ -291,6 +291,16 @@ config IKCONFIG_PROC config CONTAINERS bool +config CONTAINER_DEBUG + bool "Example debug container subsystem" + select CONTAINERS + help + This option enables a simple container subsystem that + exports useful debugging information about the containers + framework + + Say N if unsure + config CPUSETS bool "Cpuset support" depends on SMP Index: container-2.6.21-rc7-mm1/kernel/container_debug.c ============================================================ ======= --- /dev/null +++ container-2.6.21-rc7-mm1/kernel/container_debug.c @@ -0,0 +1,89 @@ +/* + * kernel/ccontainer_debug.c - Example container subsystem that + * exposes debug info + * + * Copyright (C) Google Inc, 2007 + * + * Developed by Paul Menage (menage@google.com) + * + */ + +#include <linux/container.h> +#include <linux/fs.h> + +static int debug_create(struct container_subsys *ss, struct container *cont) +{ + struct container_subsys_state *css = kzalloc(sizeof(*css), GFP_KERNEL); + if (!css) + return -ENOMEM; + cont->subsys[debug_subsys_id] = css; + return 0; +} + +stat…
creator: Paul Menage
date: 2007-04-27T10:46:16-00:00

Re: [PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: > - decide whether "Containers" is an acceptable name for the system > given its usage by some other development groups, or whether something > else (ProcessSets? ResourceGroups? TaskGroups?) would be better I place in nomination: tasksets However this would conflict with the taskset utility in Robert Loves schedutils package. The 'taskset' command "is used to set or retrieve the CPU affinity of a running process given its PID or to launch a new COMMAND with a given CPU affinity." The various alternatives you listed all have the advantage of stating both that we have a container/group/set/collection/... of something, and -what- that something might be - process/resource/task/... "tasksets" is the shortest spelling I could think of for such a compond "Something-Collection" form. If the conflict with the schedutils utility is a concern, then the next best alternative would be: taskgroups This is the next shortest way to spell this "Something-Collection" form. -- 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-04-29T01:47:44-00:00

Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: +static ssize_t container_read_uint(struct container *cont, struct cftype *cft, + struct file *file, + char __user *buf, size_t nbytes, + loff_t *ppos) +{ + char tmp[64]; + u64 val = cft->read_uint(cont, cft); + int len = sprintf(tmp, "%llu", val); This sprintf is giving me the following compiler warnings: kernel/container.c: In function 'container_read_uint': kernel/container.c:1025: warning: format '%llu' expects type 'long long unsigned int', but argument 3 has type 'u64' kernel/container.c:1025: warning: format '%llu' expects type 'long long unsigned int', but argument 3 has type 'u64' -- 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-04-29T03:12:29-00:00

Re: [PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: I'm afraid that this patch set doesn't do cpusets very well. It builds and boots and mounts the cpuset file system ok. But trying to write the 'mems' file hangs the system hard. I had to test it against 2.6.21-rc7-mm2, because I can't boot 2.6.21-rc7-mm1, due to the 'bad_page' problem that I noted in an earlier post this evening on lkml. These container patches seemed to apply ok against 2.6.21-rc7-mm2, and built and booted ok. I built with an sn2_defconfig configuration, having the following CONTAINER and CPUSET settings: CONFIG_CONTAINERS=y CONFIG_CONTAINER_DEBUG=y CONFIG_CPUSETS=y CONFIG_CONTAINER_CPUACCT=y CONFIG_PROC_PID_CPUSET=y # CONFIG_ACPI_CONTAINER is not set I could mount the cpuset file system on /dev/cpuset just fine. Then I invoked the following commands: # cd /dev/cpuset # mkdir foo # cd foo # echo 0-3 > cpus # echo 0-1 > mems At that point, the system hangs. Reproduced three times, on two boots. I never get a shell prompt back from that second echo. I have to hit Reset. The three different hangs were done with the following three different values: echo 0-3 > mems echo 0-1 > mems echo 1 > mems On that last one, "echo 1 > mems", I did not do the echo to cpus first. The test system had 8 cpus, numbered 0-7, and 4 mems, numbered 0-3. -- 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-04-29T09:37:21-00:00

Re: [PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: On Sun, Apr 29, 2007 at 02:37:21AM -0700, Paul Jackson wrote: > It builds and boots and mounts the cpuset file system ok. > But trying to write the 'mems' file hangs the system hard. Basically we are attempting a read_lock(&tasklist_lock) in container_task_count() after taking write_lock_irq(&tasklist_lock) in update_nodemask()! This patch seems to fix the prb for me: Fix write_lock() followed by read_lock() bug by introducing a 2nd argument to be passed into container_task_count. Other choice is to introduce a lock and unlocked versions of container_task_count() .. Signed-off-by : Srivatsa Vaddagiri <vatsa@in.ibm.com> --- diff -puN include/linux/container.h~lock_fix include/linux/container.h --- linux-2.6.21-rc7/include/linux/container.h~lock_fix 2007-04-30 21:56:10.000000000 +0530 +++ linux-2.6.21-rc7-vatsa/include/linux/container.h 2007-04-30 21:56:32.000000000 +0530 @@ -164,7 +164,7 @@ int container_is_removed(const struct co int container_path(const struct container *cont, char *buf, int buflen); -int container_task_count(const struct container *cont); +int container_task_count(const struct container *cont, int take_lock); /* Return true if the container is a descendant of the current container */ int container_is_descendant(const struct container *cont); diff -puN kernel/container.c~lock_fix kernel/container.c --- linux-2.6.21-rc7/kernel/container.c~lock_fix 2007-04-30 21:56:10.000000000 +0530 +++ linux-2.6.21-rc7-vatsa/kernel/container.c 2007-04-30 22:06:45.000000000 +0530 @@ -1193,21 +1193,25 @@ int container_add_files(struct container /* Count the number of tasks in a container. Could be made more * time-efficient but less space-efficient with more linked lists * running through each container and the css_group structures - * that referenced it. */ + * that referenced it. 'take_lock' indicates whether caller has locked + * tasklist or not. + */ -int container_task_count(const struct container *cont) { +int container_task_count(const struct container *cont, …
creator: Srivatsa Vaddagiri
date: 2007-04-30T17:04:45-00:00

Re: [PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: On 4/30/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > On Sun, Apr 29, 2007 at 02:37:21AM -0700, Paul Jackson wrote: > > It builds and boots and mounts the cpuset file system ok. > > But trying to write the 'mems' file hangs the system hard. > > Basically we are attempting a read_lock(&tasklist_lock) in > container_task_count() after taking write_lock_irq(&tasklist_lock) in > update_nodemask()! Paul, is there any reason why we need to do a write_lock() on tasklist_lock if we're just trying to block fork, or is it just historical accident? Wouldn't it be fine to do a read_lock()? Paul
creator: Paul Menage
date: 2007-04-30T17:09:38-00:00

Re: [PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: On Mon, Apr 30, 2007 at 10:42:25PM +0530, Srivatsa Vaddagiri wrote: > On Sun, Apr 29, 2007 at 02:37:21AM -0700, Paul Jackson wrote: > > It builds and boots and mounts the cpuset file system ok. > > But trying to write the 'mems' file hangs the system hard. > > Basically we are attempting a read_lock(&tasklist_lock) in > container_task_count() after taking write_lock_irq(&tasklist_lock) in > update_nodemask()! > > This patch seems to fix the prb for me: > > > Fix write_lock() followed by read_lock() bug by introducing a 2nd > argument to be passed into container_task_count. Other choice is to > introduce a lock and unlocked versions of container_task_count() .. > > Signed-off-by : Srivatsa Vaddagiri <vatsa@in.ibm.com> > -int container_task_count(const struct container *cont) { > +int container_task_count(const struct container *cont, int take_lock) { > int count = 0; > struct task_struct *g, *p; > struct container_subsys_state *css; > int subsys_id; > get_first_subsys(cont, &css, &subsys_id); > > - read_lock(&tasklist_lock); > + if (take_lock) > + read_lock(&tasklist_lock); > do_each_thread(g, p) { > if (task_subsys_state(p, subsys_id) == css) > count ++; > } while_each_thread(g, p); > - read_unlock(&tasklist_lock); > + if (take_lock) > + read_unlock(&tasklist_lock); > return count; Umm, no - please naje two versions with and without the lock. Also Please fix up the codingstyle, the { belongs onto a line of it's own.
creator: Christoph Hellwig
date: 2007-04-30T17:23:52-00:00

Re: [PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: On Mon, Apr 30, 2007 at 10:09:38AM -0700, Paul Menage wrote: > Paul, is there any reason why we need to do a write_lock() on > tasklist_lock if we're just trying to block fork, or is it just > historical accident? Wouldn't it be fine to do a read_lock()? Good point ..read_lock() will probably suffice in update_nodemask which means we don't need the patch I sent earlier. Paul (Jackson), This made me see another race in update_nodemask vs fork: Lets say cpuset CS1 has only one task T1 to begin with. update_nodemask(CS1) T1 in do_fork() CPU0 CPU1 ============================================================ = cpuset_fork(); mpol_copy(); ntasks = atomic_read(&cs->count); [ntasks = 2, accounting new born child T2] cs->mems_allowed = something; set_cpuset_being_rebound() write/read_lock(tasklist_lock); do_each_thread { /* Finds only T1 */ mmarray[] = .. } while_each_thread(); write/read_unlock(tasklist_lock); write_lock(tasklist_lock); /* Add T2, child of T1 to tasklist */ write_unlock(tasklist_lock); for (i = 0; i < n; i++) { mpol_rebind_mm(..); } In this for loop, we migrate only T1's ->mm. T2's->mm isn't migrated AFAICS. Is that fine? -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-04-30T17:59:03-00:00

Re: [ckrm-tech] [PATCH 0/9] Containers (V9): Generic Process Containers
type: item
description: On Mon, Apr 30, 2007 at 06:23:52PM +0100, Christoph Hellwig wrote: > Umm, no - please naje two versions with and without the lock. Ok this patch may not be necessary (given Paul's observation that read_lock will suffice in update_nodemask). > Also Please fix up the codingstyle, the { belongs onto a line of it's own. Paul, I suspect that there are a number of Codingstyle related changes you may need to address in your next version (including the one pointed by Christoph) -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-04-30T18:09:06-00:00

Re: [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: menage@google.com wrote: > This patch adds the main containers framework - the container > filesystem, and the basic structures for tracking membership and > associating subsystem state objects to tasks. [snip] > +*** notify_on_release is disabled in the current patch set. It may be > +*** reactivated in a future patch in a less-intrusive manner > + Won't this break user space tools for cpusets? [snip] > +See kernel/container.c for more details. > + > +Subsystems can take/release the container_mutex via the functions > +container_lock()/container_unlock(), and can > +take/release the callback_mutex via the functions > +container_lock()/container_unlock(). > + Hmm.. looks like a documentation error. Both mutex's are obtained through container_lock/container_unlock ? > +Accessing a task's container pointer may be done in the following ways: > +- while holding container_mutex > +- while holding the task's alloc_lock (via task_lock()) > +- inside an rcu_read_lock() section via rcu_dereference() > + container_mutex() and task_lock() can be used for changing the pointer? Could you please explain this a bit further. [snip] > +int populate(struct container_subsys *ss, struct container *cont) > +LL=none > + > +Called after creation of a container to allow a subsystem to populate > +the container directory with file entries. The subsystem should make > +calls to container_add_file() with objects of type cftype (see > +include/linux/container.h for details). Note that although this > +method can return an error code, the error code is currently not > +always handled well. We needed the equivalent of container_remove_file() to be called if container_add_file() failed. [snip] > +struct container { > + unsigned long flags; /* "unsigned long" so bitops work */ > + > + /* count users of this container. >0 means busy, but doesn't > + * necessarily indicate the number of tasks in the > + * container */ > + atomic_t count; > + > + /* > + * We link our 'sibling' struct into our parent'…
creator: Balbir Singh
date: 2007-05-01T17:40:52-00:00

Re: [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: On 5/1/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > menage@google.com wrote: > > This patch adds the main containers framework - the container > > filesystem, and the basic structures for tracking membership and > > associating subsystem state objects to tasks. > > [snip] > > > +*** notify_on_release is disabled in the current patch set. It may be > > +*** reactivated in a future patch in a less-intrusive manner > > + > > Won't this break user space tools for cpusets? Yes, so it's a must-fix before this gets anywhere near a real distribution. > > [snip] > > > +See kernel/container.c for more details. > > + > > +Subsystems can take/release the container_mutex via the functions > > +container_lock()/container_unlock(), and can > > +take/release the callback_mutex via the functions > > +container_lock()/container_unlock(). > > + > > Hmm.. looks like a documentation error. Both mutex's are obtained through > container_lock/container_unlock ? The second half of that sentence is obsolete and should have been deleted. > > > +Accessing a task's container pointer may be done in the following ways: > > +- while holding container_mutex > > +- while holding the task's alloc_lock (via task_lock()) > > +- inside an rcu_read_lock() section via rcu_dereference() > > + > > container_mutex() and task_lock() can be used for changing the pointer? No, these are all for read operations. (Actually, this is a bit of documentation that's bit-rotted - there's no longer a per-task "container" pointer). I'll update this. For write operations, only the container system should be modifying those pointers (under the protection of both container_mutex and alloc_lock). > > We needed the equivalent of container_remove_file() to be called > if container_add_file() failed. > Yes, this is some incomplete behaviour that I inherited from cpusets. Needs tidying up. > > Can't we derive the top_container from containerfs_root? Yes, we could for the cost of an extra dereference. Not sure it's a big …
creator: Paul Menage
date: 2007-05-01T17:46:10-00:00

Re: [PATCH 2/9] Containers (V9): Example CPU accounting subsystem
type: item
description: menage@google.com wrote: > + > +/* Lazily update the load calculation if necessary. Called with ca locked */ > +static void cpuusage_update(struct cpuacct *ca) > +{ > + u64 now = get_jiffies_64(); > + /* If we're not due for an update, return */ > + if (ca->next_interval_check > now) > + return; > + > + if (ca->next_interval_check <= (now - INTERVAL)) { These two conditions seem a little confusing. If ca->next_interval_check > (now - INTERVAL), the else part is executed, but if ca->next_interval_check > (now - INTERVAL) then ca->next_interval_check > now, which implies we return and never enter the else part. It's been quite sometime since I looked at this code, so I might have gotten it wrong. I see a load of 0% on my powerpc box. I think it is because last_interval_time is always 0, I'll debug further -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
creator: Balbir Singh
date: 2007-05-01T17:52:03-00:00

Re: [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: > +static int attach_task_by_pid(struct container *cont, char *pidbuf) > +{ > + pid_t pid; > + struct task_struct *tsk; > + int ret; > + > + if (sscanf(pidbuf, "%d", &pid) != 1) > + return -EIO; > + > + if (pid) { > + read_lock(&tasklist_lock); You could just use rcu_read_lock() and rcu_read_unlock() instead of read_lock(&tasklist_lock) and read_unlock(&tasklist_lock). > + > + tsk = find_task_by_pid(pid); > + if (!tsk || tsk->flags & PF_EXITING) { > + 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); > + } > + > + ret = attach_task(cont, tsk); > + put_task_struct(tsk); > + return ret; > +} > + > /* The various types of files and directories in a container file system */ > > typedef enum { > @@ -684,6 +789,54 @@ typedef enum { > FILE_TASKLIST, > } container_filetype_t; > > +static ssize_t container_common_file_write(struct container *cont, > + struct cftype *cft, > + struct file *file, > + const char __user *userbuf, > + size_t nbytes, loff_t *unused_ppos) > +{ > + container_filetype_t type = cft->private; > + char *buffer; > + int retval = 0; > + > + if (nbytes >= PATH_MAX) > + return -E2BIG; > + > + /* +1 for nul-terminator */ > + if ((buffer = kmalloc(nbytes + 1, GFP_KERNEL)) == 0) > + return -ENOMEM; > + > + if (copy_from_user(buffer, userbuf, nbytes)) { > + retval = -EFAULT; > + goto out1; > + } > + buffer[nbytes] = 0; /* nul-terminate */ > + > + mutex_lock(&container_mutex); > + > + if (container_is_removed(cont)) { > + retval = -ENODEV; > + goto out2; > + } Can't we make this check prior to kmalloc() and copy_from_user()? > +int container_task_count(const struct container *cont) { > + int count = 0; > + struct task_struct *g, *p; > + struct container_subsys_state *css; > + int subsys_id; > …
creator: Balbir Singh
date: 2007-05-01T18:12:33-00:00

Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: I have bcc'd one or more batch scheduler experts on this post. They will know who they are, and should be aware that they are not listed in the public cc list of this message. - pj Balbir Singh, responding to Paul Menage's Container patch set on lkml, wrote: > > > +*** notify_on_release is disabled in the current patch set. It may be > > +*** reactivated in a future patch in a less-intrusive manner > > + > > Won't this break user space tools for cpusets? Yes - disabling notify_on_release would definitely break some important uses of cpusets. This feature must be reactivated somehow before I'll sign up for putting this patch set in the main line. Actually, after I posted a few days ago in another lkml post: http://lkml.org/lkml/2007/4/29/66 that just the simplest cpuset command: mount -t cpuset cpuset /dev/cpuset mkdir /dev/cpuset/foo echo 0 > /dev/cpuset/foo/mems caused an immediate kernel deadlock (Srivatsa has proposed a fix), it is pretty clear that this container patch set is not getting the cpuset testing it will need for acceptance. That's partly my fault. The batch scheduler folks, such as the variants of PBS, LSF and SGE are major user of cpusets on NUMA hardware. This container based replacement for cpusets isn't ready for the main line until at least one of those schedulers can run through one of their test suites. I hesitate to even acknowledge this, as I might be the only person in a position to make this happen, and my time available to contribute to this patch set has been less than I would like. But if it looks like we have all the pieces in place to base cpusets on containers, with no known regressions in cpuset capability, then we must find a way to ensure that one of these batch schedulers, using cpusets on a NUMA box, still works. -- 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-05-01T18:40:30-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: On 5/1/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > + if (container_is_removed(cont)) { > > + retval = -ENODEV; > > + goto out2; > > + } > > Can't we make this check prior to kmalloc() and copy_from_user()? We could but I'm not sure what it would buy us - we'd be optimizing for the case that essentially never occurs. > > > > > +int container_task_count(const struct container *cont) { > > + int count = 0; > > + struct task_struct *g, *p; > > + struct container_subsys_state *css; > > + int subsys_id; > > + get_first_subsys(cont, &css, &subsys_id); > > + > > + read_lock(&tasklist_lock); > > Can be replaced with rcu_read_lock() and rcu_read_unlock() Are you sure about that? I see many users of do_each_thread()/while_each_thread() taking a lock on tasklist_lock, and only one (fs/binfmt_elf.c) that's clearly relying on an RCU critical sections. Documentation? > > Any chance we could get a per-container task list? It will > help subsystem writers as well. It would be possible, yes - but we probably wouldn't want the overhead (additional ref counts and list manipulations on every fork/exit) of it on by default. We could make it a config option that particular subsystems could select. I guess the question is how useful is this really, compared to just doing a do_each_thread() and seeing which tasks are in the container? Certainly that's a non-trivial operation, but in what circumstances is it really necessary to do it? Paul
creator: Paul Menage
date: 2007-05-01T20:37:24-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: On Tue, May 01, 2007 at 01:37:24PM -0700, Paul Menage wrote: > > Any chance we could get a per-container task list? It will > > help subsystem writers as well. > > It would be possible, yes - but we probably wouldn't want the overhead > (additional ref counts and list manipulations on every fork/exit) of > it on by default. We could make it a config option that particular > subsystems could select. > > I guess the question is how useful is this really, compared to just > doing a do_each_thread() and seeing which tasks are in the container? > Certainly that's a non-trivial operation, but in what circumstances is > it really necessary to do it? For the CPU controller I was working on, (a fast access to) such a list would have been valuable. Basically each task has a weight associated with it (p->load_weight) which is made to depend upon its class limit. Whenever the class limit changes, we need to go and change all its member task's ->load_weight value. If you don't maintain the per-container task list, I guess I could still work around it, by either: - Walk the task table and find relevant members, OR better perhaps - Move p->load_weight to a class structure -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-05-02T03:17:56-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: On 5/1/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: > > For the CPU controller I was working on, (a fast access to) such a list would > have been valuable. Basically each task has a weight associated with it > (p->load_weight) which is made to depend upon its class limit. Whenever > the class limit changes, we need to go and change all its member task's > ->load_weight value. > > If you don't maintain the per-container task list, I guess I could still > work around it, by either: > > - Walk the task table and find relevant members That doesn't seem like a terrible solution to me, unless you expect the class limit to be changing incredibly frequently. If we had multiple subsystems that needed to walk the container member list on a fast-path operation (e.g. to make a scheduling decision) that would be a good reason to maintain such a list. > perhaps > - Move p->load_weight to a class structure Sounds like a good idea if you can do it - but if it's per-process, how would it fit in the class structure? Paul
creator: Paul Menage
date: 2007-05-02T03:25:35-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: On Tue, May 01, 2007 at 08:25:35PM -0700, Paul Menage wrote: > > - Walk the task table and find relevant members > > That doesn't seem like a terrible solution to me, unless you expect > the class limit to be changing incredibly frequently. yeah i agree. Group limit(s) should not be changing so frequently. > > perhaps > > - Move p->load_weight to a class structure > > Sounds like a good idea if you can do it - but if it's per-process, > how would it fit in the class structure? p->load_weight essentially depends on two things: - nice value or static priority (which is per process, already present in task_struct) - class limit (which is per class) So in theory we can eliminate the load_weight field in task_struct and compute it at runtime from the above two fields, although it will be slightly inefficient I guess to compute the value every time a task is added to the runqueue. If that is not desirable, then we can stick with option 1 (walk task list and change member task's->load_weight upon class limit change). -- Regards, vatsa
creator: Srivatsa Vaddagiri
date: 2007-05-02T03:38:34-00:00

Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: Paul Jackson wrote: > [[ I have bcc'd one or more batch scheduler experts on this post. > They will know who they are, and should be aware that they are > not listed in the public cc list of this message. - pj ]] > > Balbir Singh, responding to Paul Menage's Container patch set on lkml, wrote: >>> +*** notify_on_release is disabled in the current patch set. It may be >>> +*** reactivated in a future patch in a less-intrusive manner >>> + >> Won't this break user space tools for cpusets? > > > Yes - disabling notify_on_release would definitely break some important > uses of cpusets. This feature must be reactivated somehow before I'll > sign up for putting this patch set in the main line. > > Actually, after I posted a few days ago in another lkml post: > http://lkml.org/lkml/2007/4/29/66 > > that just the simplest cpuset command: > mount -t cpuset cpuset /dev/cpuset > mkdir /dev/cpuset/foo > echo 0 > /dev/cpuset/foo/mems > > caused an immediate kernel deadlock (Srivatsa has proposed a fix), it > is pretty clear that this container patch set is not getting the cpuset > testing it will need for acceptance. That's partly my fault. > > The batch scheduler folks, such as the variants of PBS, LSF and SGE are > major user of cpusets on NUMA hardware. > > This container based replacement for cpusets isn't ready for the main > line until at least one of those schedulers can run through one of > their test suites. I hesitate to even acknowledge this, as I might be > the only person in a position to make this happen, and my time > available to contribute to this patch set has been less than I would > like. > > But if it looks like we have all the pieces in place to base cpusets > on containers, with no known regressions in cpuset capability, then > we must find a way to ensure that one of these batch schedulers, using > cpusets on a NUMA box, still works. > Would it be possible to extract those test cases and integrate them with a testing framework like LTP? Do you have any re…
creator: Balbir Singh
date: 2007-05-02T03:44:01-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: Paul Menage wrote: > On 5/1/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: >> > + if (container_is_removed(cont)) { >> > + retval = -ENODEV; >> > + goto out2; >> > + } >> >> Can't we make this check prior to kmalloc() and copy_from_user()? > > We could but I'm not sure what it would buy us - we'd be optimizing > for the case that essentially never occurs. > I am not sure about the never occurs part of it, because we check for the condition, so it could occur. I agree, it is a premature optimization and could wait a little longer before going in. >> >> >> >> > +int container_task_count(const struct container *cont) { >> > + int count = 0; >> > + struct task_struct *g, *p; >> > + struct container_subsys_state *css; >> > + int subsys_id; >> > + get_first_subsys(cont, &css, &subsys_id); >> > + >> > + read_lock(&tasklist_lock); >> >> Can be replaced with rcu_read_lock() and rcu_read_unlock() > > Are you sure about that? I see many users of > do_each_thread()/while_each_thread() taking a lock on tasklist_lock, > and only one (fs/binfmt_elf.c) that's clearly relying on an RCU > critical sections. Documentation? > I suspect they are all pending conversions to be made. Eric is the expert on this. Meanwhile here's a couple of pointers. Quoting from the second URL "We don't need the tasklist_lock to safely iterate through processes anymore." http://www.linuxjournal.com/article/6993 (please see incremental use of RCU) and http://kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2 .6.17/2.6.17-mm2/broken-out/proc-remove-tasklist_lock-from-p roc_pid_readdir.patch >> >> Any chance we could get a per-container task list? It will >> help subsystem writers as well. > > It would be possible, yes - but we probably wouldn't want the overhead > (additional ref counts and list manipulations on every fork/exit) of > it on by default. We could make it a config option that particular > subsystems could select. > > I guess the question is how useful is this really, compared to just > d…
creator: Balbir Singh
date: 2007-05-02T03:58:01-00:00

Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: Balbir wrote: > Would it be possible to extract those test cases and integrate them > with a testing framework like LTP? Do you have any regression test > suite for cpusets that can be made available publicly so that > any changes to cpusets can be validated? There are essentially two sorts of cpuset regression tests of interest. I have one such test, and the batch scheduler developers have various tests of their batch schedulers. 1) Testing batch schedulers against cpusets: I doubt that the batch scheduler developers would be able to extract a cpuset test from their tests, or be able to share it if they did. Their tests tend to be large tests of batch schedulers, and only incidentally test cpusets -- if we break cpusets, in sometimes even subtle ways that they happen to depend on, we break them. Sometimes there is no way to guess exactly what sorts of changes will break their code; we'll just have to schedule at least one run through one or more of them that rely heavily on cpusets before a change as big as rebasing cpusets on containers is reasonably safe. This test cycle won't be all that easy, so I'd wait until we are pretty close to what we think should be taken into the mainline kernel. I suppose I will have to be the one co-ordinating this test, as I am the only one I know with a presence in both camps. Once this test is done, from then forward, if we break them, we'll just have to deal with it as we do now, when the breakage shows up well down stream from the main kernel tree, at the point that a major batch scheduler release runs into a major distribution release containing the breakage. There is no practical way that I can see, as an ongoing basis, to continue testing for such breakage with every minor change to cpuset related code in the kernel. Any breakage found this way is dealt with by changes in user level code. Once again, I have bcc'd one or more developers of batch schedulers, so they can see what nonsense I am spouting about them now ;). 2) Testi…
creator: Paul Jackson
date: 2007-05-02T06:12:54-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: Paul Menage wrote: > On 5/1/07, Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote: >> For the CPU controller I was working on, (a fast access to) such a list would >> have been valuable. Basically each task has a weight associated with it >> (p->load_weight) which is made to depend upon its class limit. Whenever >> the class limit changes, we need to go and change all its member task's >> ->load_weight value. >> >> If you don't maintain the per-container task list, I guess I could still >> work around it, by either: >> >> - Walk the task table and find relevant members > > That doesn't seem like a terrible solution to me, unless you expect > the class limit to be changing incredibly frequently. > > If we had multiple subsystems that needed to walk the container member > list on a fast-path operation (e.g. to make a scheduling decision) > that would be a good reason to maintain such a list. > I now have a use case for maintaining a per-container task list. I am trying to build a per-container stats similar to taskstats. I intend to support container accounting of 1. Tasks running 2. Tasks stopped 3. Tasks un-interruptible 4. Tasks blocked on IO 5. Tasks sleeping This would provide statistics similar to the patch that Pavel had sent out. I faced the following problems while trying to implement this feature 1. There is no easy way to get a list of all tasks belonging to a container (we need to walk all threads) 2. There is no concept of a container identifier. When a user issues a command to extract statistics, the only unique container identifier is the container path, which means that we need to do a path lookup to determine the dentry for the container (which gets quite ugly with all the string manipulation) Adding a container id, will make it easier to find a container and return statistics belonging to the container. If we get these two features added to the current patches, it will make the infrastructure more "developer" and eventually more user friendly :-) -- Warm R…
creator: Balbir Singh
date: 2007-05-08T14:51:04-00:00

Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: Paul Jackson wrote: > Balbir wrote: > > 1) Testing batch schedulers against cpusets: > > I doubt that the batch scheduler developers would be able to > extract a cpuset test from their tests, or be able to share it if > they did. Their tests tend to be large tests of batch schedulers, > and only incidentally test cpusets -- if we break cpusets, > in sometimes even subtle ways that they happen to depend on, > we break them. > > Sometimes there is no way to guess exactly what sorts of changes > will break their code; we'll just have to schedule at least one > run through one or more of them that rely heavily on cpusets > before a change as big as rebasing cpusets on containers is > reasonably safe. This test cycle won't be all that easy, so I'd > wait until we are pretty close to what we think should be taken > into the mainline kernel. > > I suppose I will have to be the one co-ordinating this test, > as I am the only one I know with a presence in both camps. > > Once this test is done, from then forward, if we break them, > we'll just have to deal with it as we do now, when the breakage > shows up well down stream from the main kernel tree, at the point > that a major batch scheduler release runs into a major distribution > release containing the breakage. There is no practical way that I > can see, as an ongoing basis, to continue testing for such breakage > with every minor change to cpuset related code in the kernel. Any > breakage found this way is dealt with by changes in user level code. > > Once again, I have bcc'd one or more developers of batch schedulers, > so they can see what nonsense I am spouting about them now ;). > That sounds reasonable to me > 2) Testing cpusets with a specific test. > > There I can do better. Attached is the cpuset regression test I > use. It requires at least 4 cpus and 2 memory nodes to do anything > useful. It is copyright by SGI, released under GPL license. > > This regression test is the primary cpuset test upon which I > rel…
creator: Balbir Singh
date: 2007-05-10T04:09:55-00:00

Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: Balbir wrote: > Thanks for the script. Would you like to contribute this script to > LTP for wider availability and testing? Sounds good - though I'm a tad lazy, and don't know how to contribute this to the LTP. You're welcome to contribute it yourself, if that's easier for you, or to point me in the direction of some information on this. Since it's GPL, you can republish it, under that license. ... Most likely however, it will take a little tweaking to make this test work well for LTP. Most automated test environments have some requirements on the behaviour of each test, and it would be surprising if this test happen already to conform to LTP's requirements. -- 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-05-10T04:47:06-00:00

Re: [ckrm-tech] [PATCH 1/9] Containers (V9): Basic container framework
type: item
description: Paul Jackson wrote: > Balbir wrote: >> Thanks for the script. Would you like to contribute this script to >> LTP for wider availability and testing? > > Sounds good - though I'm a tad lazy, and don't know how to contribute > this to the LTP. > > You're welcome to contribute it yourself, if that's easier for you, > or to point me in the direction of some information on this. > > Since it's GPL, you can republish it, under that license. > > ... Most likely however, it will take a little tweaking to make this > test work well for LTP. Most automated test environments have some > requirements on the behaviour of each test, and it would be surprising > if this test happen already to conform to LTP's requirements. > I'll figure out more, once I am comfortable with the script and the setup environment, I'll try and get this into LTP with a GPL license. -- Thanks, Balbir Singh Linux Technology Center IBM, ISTL
creator: Balbir Singh
date: 2007-05-10T04:49:09-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: On 5/8/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > > I now have a use case for maintaining a per-container task list. > I am trying to build a per-container stats similar to taskstats. > I intend to support container accounting of > > 1. Tasks running > 2. Tasks stopped > 3. Tasks un-interruptible > 4. Tasks blocked on IO > 5. Tasks sleeping > > This would provide statistics similar to the patch that Pavel had sent out. > > I faced the following problems while trying to implement this feature > > 1. There is no easy way to get a list of all tasks belonging to a container > (we need to walk all threads) Well, walking the taks list is pretty easy - but yes, it could become inefficient when there are many small containers in use. I've got some ideas for a way of tracking this specifically for containers with subsystems that want this, while avoiding the overhead for subsystems that don't really need it. I'll try to add them to the next patchset. > 2. There is no concept of a container identifier. When a user issues a command > to extract statistics, the only unique container identifier is the container > path, which means that we need to do a path lookup to determine the dentry > for the container (which gets quite ugly with all the string manipulation) We could just cache the container path permanently in the container, and invalidate it if any of its parents gets renamed. (I imagine this happens almost never.) > > Adding a container id, will make it easier to find a container and return > statistics belonging to the container. Not unreasonable, but there are a few questions that would have to be answered: - how is the container id picked? Like a pid, or user-defined? Or some kind of string? - how would it be exposed to userspace? A generic control file provided by the container filesystem in all container directories? - can you give a more concrete example of how this would actually be useful? For your container stats, it seems that just reading a control f…
creator: Paul Menage
date: 2007-05-10T21:21:39-00:00

Re: [ckrm-tech] [PATCH 3/9] Containers (V9): Add tasks file interface
type: item
description: Paul Menage wrote: > On 5/8/07, Balbir Singh <balbir@linux.vnet.ibm.com> wrote: >> >> I now have a use case for maintaining a per-container task list. >> I am trying to build a per-container stats similar to taskstats. >> I intend to support container accounting of >> >> 1. Tasks running >> 2. Tasks stopped >> 3. Tasks un-interruptible >> 4. Tasks blocked on IO >> 5. Tasks sleeping >> >> This would provide statistics similar to the patch that Pavel had sent >> out. >> >> I faced the following problems while trying to implement this feature >> >> 1. There is no easy way to get a list of all tasks belonging to a >> container >> (we need to walk all threads) > > Well, walking the taks list is pretty easy - but yes, it could become > inefficient when there are many small containers in use. > > I've got some ideas for a way of tracking this specifically for > containers with subsystems that want this, while avoiding the overhead > for subsystems that don't really need it. I'll try to add them to the > next patchset. Super! > >> 2. There is no concept of a container identifier. When a user issues a >> command >> to extract statistics, the only unique container identifier is the >> container >> path, which means that we need to do a path lookup to determine the >> dentry >> for the container (which gets quite ugly with all the string >> manipulation) > > We could just cache the container path permanently in the container, > and invalidate it if any of its parents gets renamed. (I imagine this > happens almost never.) > Here's what I have so far, I cache the mount point of the container and add the container path to it. I'm now stuck examining tasks, while walking through a bunch of tasks, there is no easy way of knowing the container path of the task without walking all subsystems and then extracting the containers absolute path. >> >> Adding a container id, will make it easier to find a container and >> return >> statistics belonging to the container. > > Not unreasonable…
creator: Balbir Singh
date: 2007-05-11T02:31:30-00:00