OpenVZ Forum - RDF feed
type: channel
items: [PATCH 0/13] Pid namespaces (OpenVZ view)
items: [PATCH 1/13] Round up the API
items: [PATCH 2/13] Small preparations for namespaces
items: [PATCH 3/13] Introduciton of config option and clone flag
items: [PATCH 4/13] Introduce the vpid fields and helpers for getting them
items: [PATCH 5/13] Expand the pid/task seeking functions set
items: [PATCH 6/13] Pid allocation/freeing procedures
items: [PATCH 7/13] Set virtual pids for a newly cloned task
items: [PATCH 8/13] The namespace cloning
items: [PATCH 9/13] Make proc be able to have multiple super blocks
items: [PATCH 10/13] Make proc draw pids from appropriate namespace
items: [PATCH 11/13] Changes to show virtual ids to user
items: [PATCH 12/13] Show appropriate pids in proc
items: [PATCH 13/13] Make all proc entres accessible in a namespace
items: Instructions of how to make testing easy
items: Re: Instructions of how to make testing easy
items: Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
items: Re: [PATCH 3/13] Introduciton of config option and clone flag
items: Re: [PATCH 11/13] Changes to show virtual ids to user
items: Re: [PATCH 2/13] Small preparations for namespaces
items: Re: [PATCH 1/13] Round up the API
items: Re: [PATCH 5/13] Expand the pid/task seeking functions set
items: Re: Re: [PATCH 0/13] Pid namespaces (OpenVZ view)

[PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: That's how OpenVZ sees the pid namespaces. The main idea is that kernel keeps operating with tasks pid as it did before, but each task obtains one more pid for each pid type - the virtual pid. When putting the pid to user or getting the pid from it kernel operates with the virtual ones. E.g. virtual pid is returned from getpid(), virtual pgid - from getpgid() and so on. Getting virtual pid from user is performed in setpgid(), setsid() and kill() mainly and in some other places. As far as the namespace are concerned I propose the following scheme. The namespace can be created from unshare syscall only. This makes fork() code look easier. Of course task must be prepared to have its pids changed. When task creates a new namespace it becomes its init and sees the tasks from it only. Tasks from init namespace see all the tasks. One relevant thing left behind is shrinking both proc's entries on task death. The reason I didn't do that is the following: this does not guarantee that the pid will be put (and thus still may hold the namespace), but makes the patch more complicated. So if this set will turns out to be interesting I will implement this thing as well. The patches are for 2.6.22-rc1-mm1 tree. Thanks, Pavel
creator: Pavel Emelianov
date: 2007-05-24T12:32:44-00:00

[PATCH 1/13] Round up the API
type: item
description: The set of functions process_session, task_session, process_group and task_pgrp is confusing, as the names can be mixed with each other when looking at the code for a long time. The proposals are to * equip the functions that return the integer with _nr suffix to represent that fact, * and to make all functions work with task (not process) by making the common prefix of the same name. For monotony the routines signal_session() and set_signal_session() are replaced with task_session_nr() and set_task_session(), especially since they are only used with the explicit task->signal dereference. I've sent this before, but Andrew didn't include it, so I resend it as the part of this set. Signed-off-by: Pavel Emelianov <xemul@openvz.org> Acked-by: Serge E. Hallyn <serue@us.ibm.com> --- diff --git a/arch/mips/kernel/irixelf.c b/arch/mips/kernel/irixelf.c index 403d96f..10ba0a5 100644 --- a/arch/mips/kernel/irixelf.c +++ b/arch/mips/kernel/irixelf.c @@ -1170,8 +1170,8 @@ static int irix_core_dump(long signr, st prstatus.pr_sighold = current->blocked.sig[0]; psinfo.pr_pid = prstatus.pr_pid = current->pid; psinfo.pr_ppid = prstatus.pr_ppid = current->parent->pid; - psinfo.pr_pgrp = prstatus.pr_pgrp = process_group(current); - psinfo.pr_sid = prstatus.pr_sid = process_session(current); + psinfo.pr_pgrp = prstatus.pr_pgrp = task_pgrp_nr(current); + psinfo.pr_sid = prstatus.pr_sid = task_session_nr(current); if (current->pid == current->tgid) { /* * This is the record for the group leader. Add in the diff --git a/arch/mips/kernel/irixsig.c b/arch/mips/kernel/irixsig.c index 6980deb..210503e 100644 --- a/arch/mips/kernel/irixsig.c +++ b/arch/mips/kernel/irixsig.c @@ -609,7 +609,7 @@ repeat: p = list_entry(_p,struct task_struct,sibling); if ((type == IRIX_P_PID) && p->pid != pid) continue; - if ((type == IRIX_P_PGID) && process_group(p) != pid) + if ((type == IRIX_P_PGID) && task_pgrp_nr(p) != pid) continue; if ((p->exit_signal != SIGCHLD)) continue; diff --git a/arch/mips/kernel/sys…
creator: Pavel Emelianov
date: 2007-05-24T12:35:20-00:00

[PATCH 2/13] Small preparations for namespaces
type: item
description: This includes #ifdefs in get/put_pid_ns and rewriting the child_reaper() function to the more logical view. This doesn't fit logically into any other patch so I decided to make it separate. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h index 169c6c2..7af7191 100644 --- a/include/linux/pid_namespace.h +++ b/include/linux/pid_namespace.h @@ -26,7 +26,9 @@ extern struct pid_namespace init_pid_ns; static inline void get_pid_ns(struct pid_namespace *ns) { +#ifdef CONFIG_PID_NS kref_get(&ns->kref); +#endif } extern struct pid_namespace *copy_pid_ns(int flags, struct pid_namespace *ns); @@ -34,12 +36,15 @@ extern void free_pid_ns(struct kref *kre static inline void put_pid_ns(struct pid_namespace *ns) { +#ifdef CONFIG_PID_NS kref_put(&ns->kref, free_pid_ns); +#endif } static inline struct task_struct *child_reaper(struct task_struct *tsk) { - return init_pid_ns.child_reaper; + BUG_ON(tsk != current); + return tsk->nsproxy->pid_ns->child_reaper; } #endif /* _LINUX_PID_NS_H */
creator: Pavel Emelianov
date: 2007-05-24T12:37:31-00:00

[PATCH 3/13] Introduciton of config option and clone flag
type: item
description: The config option is CONFIG_PID_NS. The flag is CLONE_NEWPIDS. As I have already said - cloning of pid namespace from fork() is not allowed - use unshare for this. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/init/Kconfig b/init/Kconfig index 2a46e35..59e4625 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -127,6 +127,16 @@ config SWAP_PREFETCH Workstations and multiuser workstation servers will most likely want to say Y. +config PID_NS + bool "Pid namespaces" + default n + help + Enable pid namespaces support. When on task is allowed to unshare + its pid namespace from parent and become its init. After this task + all its children will see only the tasks from this namespace. + However tasks from parent namespace see all the tasks in the system. + Ony one level of nesting is alowed. Tasks cannot leave the namespace. + config SYSVIPC bool "System V IPC" ---help--- diff --git a/include/linux/sched.h b/include/linux/sched.h index d4de6d8..7743a11 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -26,6 +26,7 @@ #define CLONE_STOPPED 0x02000000 /* Start in stopped state */ #define CLONE_NEWUTS 0x04000000 /* New utsname group? */ #define CLONE_NEWIPC 0x08000000 /* New ipcs */ +#define CLONE_NEWPIDS 0x10000000 /* New pids */ /* * Scheduling policies diff --git a/kernel/fork.c b/kernel/fork.c index d7207a1..3ab517c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1606,7 +1612,7 @@ asmlinkage long sys_unshare(unsigned lon err = -EINVAL; if (unshare_flags & ~(CLONE_THREAD|CLONE_FS|CLONE_NEWNS|CLONE_SIGHAND| CLONE_VM|CLONE_FILES|CLONE_SYSVSEM| - CLONE_NEWUTS|CLONE_NEWIPC)) + CLONE_NEWUTS|CLONE_NEWIPC|CLONE_NEWPIDS)) goto bad_unshare_out; if ((err = unshare_thread(unshare_flags))) diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index 1bc4b55..9bcc047 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -110,6 +110,9 @@ int copy_namespaces(int flags, struct ta get_nsproxy(old_ns); + if (flags & CLONE_NEWPIDS) + return -EINVAL; + if (!(…
creator: Pavel Emelianov
date: 2007-05-24T12:42:04-00:00

[PATCH 4/13] Introduce the vpid fields and helpers for getting them
type: item
description: Since we want to export virtual pid to userspace and this is optional (CONFIG_PID_NS) we need helpers for getting the values of vpid/vtgid/etc depending on the config and the appropriate members on structs. A note about the struct pid. As will be seen later pid may now be stored in two hashes - pid_hash and the vpid_hash. The latter is used to find the struct pid by pid get from the userspace. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/include/linux/pid.h b/include/linux/pid.h index 1e0e4e3..3a30f8a 100644 --- a/include/linux/pid.h +++ b/include/linux/pid.h @@ -46,6 +46,11 @@ struct pid /* Try to keep pid_chain in the same cacheline as nr for find_pid */ int nr; struct hlist_node pid_chain; +#ifdef CONFIG_PID_NS + int vnr; + struct hlist_node vpid_chain; + struct pid_namespace *ns; +#endif /* lists of tasks that use this pid */ struct hlist_head tasks[PIDTYPE_MAX]; struct rcu_head rcu; @@ -106,6 +114,20 @@ static inline pid_t pid_nr(struct pid *p return nr; } +#ifdef CONFIG_PID_NS +static inline pid_t pid_vnr(struct pid *pid) +{ + pid_t nr = 0; + if (pid) + nr = pid->vnr; + return nr; +} +#else +#define pid_vnr(pid) pid_nr(pid) +#endif + +#define pid_nr_ns(pid, ns) (ns == &init_pid_ns ? pid_nr(pid) : pid_vnr(pid)) + #define do_each_pid_task(pid, type, task) \ do { \ struct hlist_node *pos___; \ diff --git a/include/linux/sched.h b/include/linux/sched.h index d4de6d8..7743a11 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -456,7 +457,10 @@ struct signal_struct { pid_t session __deprecated; pid_t __session; }; - +#ifdef CONFIG_PID_NS + pid_t vpgrp; + pid_t vsession; +#endif /* boolean value for session group leader */ int leader; @@ -887,6 +891,10 @@ struct task_struct { unsigned did_exec:1; pid_t pid; pid_t tgid; +#ifdef CONFIG_PID_NS + pid_t vpid; + pid_t vtgid; +#endif #ifdef CONFIG_CC_STACKPROTECTOR /* Canary value for the -fstack-protector gcc feature */ @@ -1127,6 +1135,128 @@ static inline struct pid *task_session(s re…
creator: Pavel Emelianov
date: 2007-05-24T12:44:07-00:00

[PATCH 5/13] Expand the pid/task seeking functions set
type: item
description: We need the extended set of functions for searching tasks and pids - search in global namespace, in local namespace (current belongs to) and in arbitrary namespace (used in proc). Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/include/linux/sched.h b/include/linux/sched.h index d4de6d8..7743a11 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1298,8 +1428,16 @@ extern struct task_struct init_task; extern struct mm_struct init_mm; -#define find_task_by_pid(nr) find_task_by_pid_type(PIDTYPE_PID, nr) -extern struct task_struct *find_task_by_pid_type(int type, int pid); +extern struct pid_namespace init_pid_ns; +extern struct task_struct *find_task_by_pid_type_ns(int type, int pid, + struct pid_namespace *ns); + +#define find_task_by_pid_ns(nr, ns) \ + find_task_by_pid_type_ns(PIDTYPE_PID, nr, ns) +#define find_task_by_pid_type(type, nr) \ + find_task_by_pid_type_ns(type, nr, &init_pid_ns) +#define find_task_by_pid(nr) \ + find_task_by_pid_type(PIDTYPE_PID, nr) extern void __set_special_pids(pid_t session, pid_t pgrp); /* per-UID process charging. */ diff --git a/include/linux/pid.h b/include/linux/pid.h index 1e0e4e3..3a30f8a 100644 --- a/include/linux/pid.h +++ b/include/linux/pid.h @@ -83,17 +88,20 @@ extern void FASTCALL(detach_pid(struct t extern void FASTCALL(transfer_pid(struct task_struct *old, struct task_struct *new, enum pid_type)); +struct pid_namespace; /* * look up a PID in the hash table. Must be called with the tasklist_lock * or rcu_read_lock() held. */ extern struct pid *FASTCALL(find_pid(int nr)); +extern struct pid *FASTCALL(__find_vpid(int nr, struct pid_namespace *ns)); +#define find_vpid(pid) __find_vpid(pid, current->nsproxy->pid_ns) /* * Lookup a PID in the hash table, and return with it's count elevated. */ extern struct pid *find_get_pid(int nr); -extern struct pid *find_ge_pid(int nr); +extern struct pid *find_ge_pid(int nr, struct pid_namespace *); extern struct pid *alloc_pid(void); extern void FASTCALL…
creator: Pavel Emelianov
date: 2007-05-24T12:46:49-00:00

[PATCH 6/13] Pid allocation/freeing procedures
type: item
description: This patch make alloc_pid() and free_pid() aware of the namespaces. When a pid is created not in init namespace it gets into two hashes and holds the pointer to the namespace itself. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/kernel/pid.c b/kernel/pid.c index eb66bd2..1815af4 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -54,6 +54,12 @@ static inline int mk_pid(struct pid_name #define find_next_offset(map, off) \ find_next_zero_bit((map)->page, BITS_PER_PAGE, off) +#ifdef CONFIG_PID_NS +static struct hlist_head *vpid_hash; +#define vpid_hashfn(nr, ns) hash_long((unsigned long)nr + (unsigned long)ns, \ + pidhash_shift) +#endif + /* * PID-map pages start out as NULL, they get allocated upon * first use and are never deallocated. This way a low pid_max @@ -197,9 +203,19 @@ fastcall void free_pid(struct pid *pid) spin_lock_irqsave(&pidmap_lock, flags); hlist_del_rcu(&pid->pid_chain); +#ifdef CONFIG_PID_NS + if (pid->ns != &init_pid_ns) + hlist_del_rcu(&pid->vpid_chain); +#endif spin_unlock_irqrestore(&pidmap_lock, flags); free_pidmap(&init_pid_ns, pid->nr); +#ifdef CONFIG_PID_NS + if (pid->ns != &init_pid_ns) { + free_pidmap(pid->ns, pid->vnr); + put_pid_ns(pid->ns); + } +#endif call_rcu(&pid->rcu, delayed_put_pid); } @@ -207,28 +223,52 @@ struct pid *alloc_pid(void) { struct pid *pid; enum pid_type type; - int nr = -1; + int nr, vnr; + struct pid_namespace *ns; pid = kmem_cache_alloc(pid_cachep, GFP_KERNEL); if (!pid) goto out; - nr = alloc_pidmap(current->nsproxy->pid_ns); + vnr = nr = alloc_pidmap(&init_pid_ns); if (nr < 0) goto out_free; + ns = current->nsproxy->pid_ns; +#ifdef CONFIG_PID_NS + if (ns != &init_pid_ns) { + vnr = alloc_pidmap(ns); + if (vnr < 0) + goto out_free_map; + + get_pid_ns(ns); + } +#endif atomic_set(&pid->count, 1); pid->nr = nr; +#ifdef CONFIG_PID_NS + pid->vnr = vnr; + pid->ns = ns; +#endif for (type = 0; type < PIDTYPE_MAX; ++type) INIT_HLIST_HEAD(&pid->tasks[type]); spin_lock_irq(&pidmap_lock); hlist_add_head_rcu(…
creator: Pavel Emelianov
date: 2007-05-24T12:49:47-00:00

[PATCH 7/13] Set virtual pids for a newly cloned task
type: item
description: When new task is created it must have its virtual pids set. When task belongs to init namespace the pids are equal to global ones so it is safe to get vpid from any task. This is the place where we export pids to use space and there will be a patch for these cases, but this case is specal and thus goes separately. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/kernel/fork.c b/kernel/fork.c index d7207a1..3ab517c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1026,6 +1026,7 @@ static struct task_struct *copy_process( delayacct_tsk_init(p); /* Must remain after dup_task_struct() */ copy_flags(clone_flags, p); p->pid = pid_nr(pid); + set_task_vpid(p, pid_vnr(pid)); INIT_LIST_HEAD(&p->children); INIT_LIST_HEAD(&p->sibling); p->vfork_done = NULL; @@ -1101,8 +1102,11 @@ static struct task_struct *copy_process( #endif p->tgid = p->pid; - if (clone_flags & CLONE_THREAD) + set_task_vtgid(p, task_pid_vnr(p)); + if (clone_flags & CLONE_THREAD) { p->tgid = current->tgid; + set_task_vtgid(p, task_pid_vnr(current)); + } if ((retval = security_task_alloc(p))) goto bad_fork_cleanup_policy; @@ -1251,6 +1255,8 @@ static struct task_struct *copy_process( p->signal->tty = current->signal->tty; p->signal->pgrp = task_pgrp_nr(current); set_task_session(p, task_session_nr(current)); + set_task_vpgrp(p, task_pgrp_vnr(current)); + set_task_vsession(p, task_session_vnr(current)); attach_pid(p, PIDTYPE_PGID, task_pgrp(current)); attach_pid(p, PIDTYPE_SID, task_session(current)); @@ -1270,7 +1276,7 @@ static struct task_struct *copy_process( * TID. It's too late to back out if this fails. */ if (clone_flags & CLONE_PARENT_SETTID) - put_user(p->pid, parent_tidptr); + put_user(task_pid_vnr(p), parent_tidptr); proc_fork_connector(p); return p; @@ -1372,7 +1378,7 @@ long do_fork(unsigned long clone_flags, if (!pid) return -EAGAIN; - nr = pid->nr; + nr = pid_vnr(pid); if (unlikely(current->ptrace)) { trace = fork_traceflag (clone_flags); if (trace)
creator: Pavel Emelianov
date: 2007-05-24T12:53:23-00:00

[PATCH 8/13] The namespace cloning
type: item
description: This is the core of the set - the namespace cloning. The cloning consists of two stages - creating of the new namespace and moving a task into it. Create and move is not good as the error path just puts the new namespaces and thus keep the task in it. So after the new namespace is clones task still outside it. It is injected inside explicitly after all the operations that might fail are finished. Another important thing is that task must be alone in its group and session and must not be splitted into threads. This is because all task's pids are moved to the new ns and if they are shared with someone else this someone may happen to be half-inserted into the new space. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/include/linux/pid.h b/include/linux/pid.h index 1e0e4e3..3a30f8a 100644 --- a/include/linux/pid.h +++ b/include/linux/pid.h @@ -106,6 +114,8 @@ static inline pid_t pid_nr(struct pid *p #define pid_nr_ns(pid, ns) (ns == &init_pid_ns ? pid_nr(pid) : pid_vnr(pid)) +void move_init_to_ns(struct task_struct *tsk); + #define do_each_pid_task(pid, type, task) \ do { \ struct hlist_node *pos___; \ diff --git a/kernel/fork.c b/kernel/fork.c index d7207a1..3ab517c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1659,8 +1665,11 @@ asmlinkage long sys_unshare(unsigned lon task_unlock(current); } - if (new_nsproxy) + if (new_nsproxy) { + if (unshare_flags & CLONE_NEWPIDS) + move_init_to_ns(current); put_nsproxy(new_nsproxy); + } bad_unshare_cleanup_semundo: bad_unshare_cleanup_fd: diff --git a/kernel/pid.c b/kernel/pid.c index eb66bd2..1815af4 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -350,11 +412,99 @@ struct pid *find_get_pid(pid_t nr) } EXPORT_SYMBOL_GPL(find_get_pid); +#ifdef CONFIG_PID_NS +static struct pid_namespace *create_pid_namespace(void) +{ + struct pid_namespace *ns; + int i; + + ns = kmalloc(sizeof(struct pid_namespace), GFP_KERNEL); + if (ns == NULL) + goto out; + + ns->pidmap[0].page = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!ns…
creator: Pavel Emelianov
date: 2007-05-24T12:57:58-00:00

[PATCH 9/13] Make proc be able to have multiple super blocks
type: item
description: Each namespace must have its own proc super block where dentries and inodes representing the tasks live. Plus proc super block bust hold the namespace it draws the pids from. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h index 38f7082..d107a33 100644 --- a/include/linux/proc_fs.h +++ b/include/linux/proc_fs.h @@ -126,7 +126,8 @@ extern struct proc_dir_entry *create_pro extern void remove_proc_entry(const char *name, struct proc_dir_entry *parent); extern struct vfsmount *proc_mnt; -extern int proc_fill_super(struct super_block *,void *,int); +struct pid_namespace; +extern int proc_fill_super(struct super_block *, struct pid_namespace *); extern struct inode *proc_get_inode(struct super_block *, unsigned int, struct proc_dir_entry *); /* diff --git a/fs/proc/inode.c b/fs/proc/inode.c index 2ba47e4..5a7b5d5 100644 --- a/fs/proc/inode.c +++ b/fs/proc/inode.c @@ -15,6 +15,7 @@ #include <linux/init.h> #include <linux/module.h> #include <linux/smp_lock.h> +#include <linux/pid_namespace.h> #include <asm/system.h> #include <asm/uaccess.h> @@ -428,9 +429,17 @@ out_mod: return NULL; } -int proc_fill_super(struct super_block *s, void *data, int silent) +int proc_fill_super(struct super_block *s, struct pid_namespace *ns) { struct inode * root_inode; + struct proc_dir_entry * root_dentry; + + root_dentry = &proc_root; + if (ns != &init_pid_ns) { + root_dentry = create_proc_root(); + if (root_dentry == NULL) + goto out_no_de; + } s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC; s->s_blocksize = 1024; @@ -439,8 +448,8 @@ int proc_fill_super(struct super_block * s->s_op = &proc_sops; s->s_time_gran = 1; - de_get(&proc_root); - root_inode = proc_get_inode(s, PROC_ROOT_INO, &proc_root); + de_get(root_dentry); + root_inode = proc_get_inode(s, PROC_ROOT_INO, root_dentry); if (!root_inode) goto out_no_root; root_inode->i_uid = 0; @@ -451,9 +460,10 @@ int proc_fill_super(struct super_block * return 0; out_no_root…
creator: Pavel Emelianov
date: 2007-05-24T13:01:24-00:00

[PATCH 10/13] Make proc draw pids from appropriate namespace
type: item
description: The design is the following. The pids shown in proc tree are the ones get from the namespace the superblock belongs to. As seen from the previous patch, when proc is mounted the current namespace is considered to be the owner of the superbloc. Signed-off-by: Pavel Emelianov <xemul@openvz/.org> --- diff --git a/fs/proc/base.c b/fs/proc/base.c index 8b426e9..e9811ec 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2105,6 +2106,7 @@ struct dentry *proc_pid_lookup(struct in struct dentry *result = ERR_PTR(-ENOENT); struct task_struct *task; unsigned tgid; + struct pid_namespace *ns; result = proc_base_lookup(dir, dentry); if (!IS_ERR(result) || PTR_ERR(result) != -ENOENT) @@ -2114,8 +2116,9 @@ struct dentry *proc_pid_lookup(struct in if (tgid == ~0U) goto out; + ns = (struct pid_namespace *)dentry->d_sb->s_fs_info; rcu_read_lock(); - task = find_task_by_pid(tgid); + task = find_task_by_pid_ns(tgid, ns); if (task) get_task_struct(task); rcu_read_unlock(); @@ -2132,7 +2135,8 @@ out: * Find the first task with tgid >= tgid * */ -static struct task_struct *next_tgid(unsigned int tgid) +static struct task_struct *next_tgid(unsigned int tgid, + struct pid_namespace *ns) { struct task_struct *task; struct pid *pid; @@ -2140,9 +2144,9 @@ static struct task_struct *next_tgid(uns rcu_read_lock(); retry: task = NULL; - pid = find_ge_pid(tgid); + pid = find_ge_pid(tgid, ns); if (pid) { - tgid = pid->nr + 1; + tgid = pid_nr_ns(pid, ns) + 1; task = pid_task(pid, PIDTYPE_PID); /* What we to know is if the pid we have find is the * pid of a thread_group_leader. Testing for task @@ -2182,6 +2186,7 @@ int proc_pid_readdir(struct file * filp, struct task_struct *reaper = get_proc_task(filp->f_path.dentry->d_inode); struct task_struct *task; int tgid; + struct pid_namespace *ns; if (!reaper) goto out_no_task; @@ -2192,11 +2197,12 @@ int proc_pid_readdir(struct file * filp, goto out; } + ns = (struct pid_namespace *)filp->f_dentry->d_sb->s_fs_info; tgid = filp->f_pos - TGID_OFFSET; - fo…
creator: Pavel Emelianov
date: 2007-05-24T13:04:10-00:00

[PATCH 11/13] Changes to show virtual ids to user
type: item
description: This is the largest patch in the set. Make all (I hope) the places where the pid is shown to or get from user operate on the virtual pids. An exception is copy_process - it was in one of the previous patches - and the proc - this will come as a separate patch. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/arch/ia64/kernel/signal.c b/arch/ia64/kernel/signal.c index aeec818..cdb64cc 100644 --- a/arch/ia64/kernel/signal.c +++ b/arch/ia64/kernel/signal.c @@ -227,7 +227,7 @@ ia64_rt_sigreturn (struct sigscratch *sc si.si_signo = SIGSEGV; si.si_errno = 0; si.si_code = SI_KERNEL; - si.si_pid = current->pid; + si.si_pid = task_pid_vnr(current); si.si_uid = current->uid; si.si_addr = sc; force_sig_info(SIGSEGV, &si, current); @@ -332,7 +332,7 @@ force_sigsegv_info (int sig, void __user si.si_signo = SIGSEGV; si.si_errno = 0; si.si_code = SI_KERNEL; - si.si_pid = current->pid; + si.si_pid = task_pid_vnr(current); si.si_uid = current->uid; si.si_addr = addr; force_sig_info(SIGSEGV, &si, current); diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c index fb35ebc..2ce3806 100644 --- a/arch/parisc/kernel/signal.c +++ b/arch/parisc/kernel/signal.c @@ -181,7 +181,7 @@ give_sigsegv: si.si_signo = SIGSEGV; si.si_errno = 0; si.si_code = SI_KERNEL; - si.si_pid = current->pid; + si.si_pid = task_pid_vnr(current); si.si_uid = current->uid; si.si_addr = &frame->uc; force_sig_info(SIGSEGV, &si, current); diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c index 260a1f3..9161107 100644 --- a/drivers/char/tty_io.c +++ b/drivers/char/tty_io.c @@ -103,6 +103,7 @@ #include <linux/selection.h> #include <linux/kmod.h> +#include <linux/nsproxy.h> #undef TTY_DEBUG_HANGUP @@ -3065,7 +3066,7 @@ static int tiocgpgrp(struct tty_struct * */ if (tty == real_tty && current->signal->tty != real_tty) return -ENOTTY; - return put_user(pid_nr(real_tty->pgrp), p); + return put_user(pid_vnr(real_tty->pgrp), p); } /** @@ -3099,7 +3100,7 @@ static int tiocspgrp(struc…
creator: Pavel Emelianov
date: 2007-05-24T13:06:40-00:00

[PATCH 12/13] Show appropriate pids in proc
type: item
description: This is the proc-related part of the previous patch. Since tasks are seen from two proc-s the appropriate (virtual or global) pid must be shown. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/fs/proc/array.c b/fs/proc/array.c index aef7b7b..f65e4c9 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -75,6 +75,7 @@ #include <linux/cpuset.h> #include <linux/rcupdate.h> #include <linux/delayacct.h> +#include <linux/pid_namespace.h> #include <asm/uaccess.h> #include <asm/pgtable.h> @@ -161,7 +162,9 @@ static inline char * task_state(struct t struct group_info *group_info; int g; struct fdtable *fdt = NULL; + struct pid_namespace *ns; + ns = current->nsproxy->pid_ns; rcu_read_lock(); buffer += sprintf(buffer, "State:\t%s\n" @@ -174,9 +177,12 @@ static inline char * task_state(struct t "Gid:\t%d\t%d\t%d\t%d\n", get_task_state(p), (p->sleep_avg/1024)*100/(1020000000/1024), - p->tgid, p->pid, - pid_alive(p) ? rcu_dereference(p->real_parent)->tgid : 0, - pid_alive(p) && p->ptrace ? rcu_dereference(p->parent)->pid : 0, + __task_tgid_nr_ns(p, ns), + __task_pid_nr_ns(p, ns), + pid_alive(p) ? task_ppid_nr_ns(p, ns) : 0, + pid_alive(p) && p->ptrace ? + __task_tgid_nr_ns( + rcu_dereference(p->parent), ns) : 0, p->uid, p->euid, p->suid, p->fsuid, p->gid, p->egid, p->sgid, p->fsgid); @@ -349,6 +355,7 @@ static int do_task_stat(struct task_stru rcu_read_lock(); if (lock_task_sighand(task, &flags)) { struct signal_struct *sig = task->signal; + struct pid_namespace *ns = current->nsproxy->pid_ns; if (sig->tty) { tty_pgrp = pid_nr(sig->tty->pgrp); @@ -381,9 +388,9 @@ static int do_task_stat(struct task_stru stime = cputime_add(stime, sig->stime); } - sid = task_session_nr(task); - pgid = task_pgrp_nr(task); - ppid = rcu_dereference(task->real_parent)->tgid; + sid = __task_session_nr_ns(task, ns); + pgid = __task_pgrp_nr_ns(task, ns); + ppid = task_ppid_nr_ns(task, ns); unlock_task_sighand(task, &flags); } @@ -414,7 +421,7 @@ static int do_task_stat(struct task_stru…
creator: Pavel Emelianov
date: 2007-05-24T13:08:13-00:00

[PATCH 13/13] Make all proc entres accessible in a namespace
type: item
description: This is a hack that must not go to any tree, but this makes patch testing from shell possible. When creating a new pid namespace one need to mount a new proc instance in this space context. But the /proc will contain pids and self symlink only and this breaks some important tools like ps and sshd. They expect /proc/stat, /proc/mounts and some other entries to be present. Without this patch the only possible way to test the namespace is to make all the operations performed from self-made programs. With it one may use general command line tools. This patch makes them visible, but this is ugly. Correct fix must make some kind of proc_entries namespace but this is another interesting task. Signed-off-by: Pavel Emelianov <xemul@openvz.org> --- diff --git a/fs/proc/generic.c b/fs/proc/generic.c index 9401a6f..eec1eb6 100644 --- a/fs/proc/generic.c +++ b/fs/proc/generic.c @@ -393,6 +393,10 @@ struct dentry *proc_lookup(struct inode spin_lock(&proc_subdir_lock); de = PDE(dir); if (de) { +#ifdef CONFIG_PID_NS + if (de->parent == de) + de = &proc_root; +#endif for (de = de->subdir; de ; de = de->next) { if (de->namelen != dentry->d_name.len) continue; @@ -446,6 +450,10 @@ int proc_readdir(struct file * filp, ret = -EINVAL; goto out; } +#ifdef CONFIG_PID_NS + if (de->parent == de) + de = &proc_root; +#endif i = filp->f_pos; switch (i) { case 0:
creator: Pavel Emelianov
date: 2007-05-24T13:13:16-00:00

Instructions of how to make testing easy
type: item
description: That's the program I used for testing. It creates a new session, chroots to new root, clones the namespace, mounts proc and launches the sshd to keep track of the terminals. The new root I prepared was bind-mounted /lib, /bin, /usr etc directories, copied /dev devices with devpts mounted inside and empty /var (for sshd) and /proc (for new mount). After these preparations I launched this enter program and then used ssh to get into the namespace. Hope this will help. The patches introduced was then tested with some mportaint tests from ltp testsuite in 4 ways: 1 kernel w/o the patch 2 kernel with CONFIG_PID_NS=n 3 kernel with namespaces in init namespace 4 kernel with namespaces in subnamespace The results coincided. #include <unistd.h> #include <sys/wait.h> #include <errno.h> #include <linux/unistd.h> #ifndef __NR_unshare #define __NR_unshare 310 #endif _syscall1(int, unshare, int, flags) #define CLONE_NEWPIDS 0x10000000 #define ROOT_DIR "./new_root" int main(void) { int pid; int status; pid = fork(); if (pid < 0) { perror("Can't fork\n"); return 1; } if (pid > 0) { if (waitpid(pid, &status, 0) < 0) { perror("Can't wait kid\n"); return 2; } if (WIFEXITED(status)) printf("%d exited with %d/%d\n", pid, WEXITSTATUS(status) & 0xf, WEXITSTATUS(status) >> 3); else if (WIFSIGNALED(status)) printf("%d signalled with %d\n", pid, WTERMSIG(status)); else printf("Some shit happened with %d\n", pid); return 0; } printf("Set new sid\n"); if (setsid() < 0) return (errno << 4) + 0; printf("Unshare\n"); if (unshare(CLONE_NEWPIDS) < 0) return (errno << 4) + 2; printf("Chroot\n"); if (chroot(ROOT_DIR) < 0) return (errno << 4) + 1; printf("Mount proc\n"); if (mount("none", "/proc", "proc", 0, NULL) < 0) return (errno << 4) + 3; printf("Launching sshd\n"); if (fork() == 0) { execl("/usr/sbin/sshd", "/usr/sbin/sshd", "-p", "2202", NULL); return (errno << 4) + 3; } /* Never exit... Bad init */ while (1) { if (wait(NULL) < 0) sleep(1); } return 0; }
creator: Pavel Emelianov
date: 2007-05-24T13:19:14-00:00

Re: Instructions of how to make testing easy
type: item
description: Hello Pavel ! I'm giving it a try. For those using qemu, you'll need this patch: http://lkml.org/lkml/2007/5/16/360 thanks for the patchset pavel. C.
creator: Cedric Le Goater
date: 2007-05-24T14:55:43-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > That's how OpenVZ sees the pid namespaces. > > The main idea is that kernel keeps operating with tasks pid > as it did before, but each task obtains one more pid for each > pid type - the virtual pid. When putting the pid to user or > getting the pid from it kernel operates with the virtual ones. > > E.g. virtual pid is returned from getpid(), virtual pgid - > from getpgid() and so on. Getting virtual pid from user is > performed in setpgid(), setsid() and kill() mainly and in some > other places. > > As far as the namespace are concerned I propose the following > scheme. The namespace can be created from unshare syscall only. > This makes fork() code look easier. Of course task must be So is your main reason for posting this as a counter to Suka's patchset the concern of overhead at clone? thanks, -serge > prepared to have its pids changed. When task creates a new > namespace it becomes its init and sees the tasks from it only. > Tasks from init namespace see all the tasks. > > One relevant thing left behind is shrinking both proc's entries > on task death. The reason I didn't do that is the following: this > does not guarantee that the pid will be put (and thus still may > hold the namespace), but makes the patch more complicated. So if > this set will turns out to be interesting I will implement this > thing as well. > > The patches are for 2.6.22-rc1-mm1 tree. > > Thanks, > Pavel
creator: serue
date: 2007-05-24T15:09:09-00:00

Re: [PATCH 3/13] Introduciton of config option and clone flag
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > > diff --git a/init/Kconfig b/init/Kconfig > index 2a46e35..59e4625 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -127,6 +127,16 @@ config SWAP_PREFETCH > Workstations and multiuser workstation servers will most likely want > to say Y. > > +config PID_NS > + bool "Pid namespaces" > + default n > + help > + Enable pid namespaces support. When on task is allowed to unshare > + its pid namespace from parent and become its init. After this task > + all its children will see only the tasks from this namespace. > + However tasks from parent namespace see all the tasks in the system. > + Ony one level of nesting is alowed. Tasks cannot leave the namespace. > + Until we kill daemonize and are certain we can't find any other places in the kernel that need to be updated please make PID_NS depend on CONFIG_EXPERIMENTAL. Eric
creator: ebiederm
date: 2007-05-24T15:39:41-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > That's how OpenVZ sees the pid namespaces. > > The main idea is that kernel keeps operating with tasks pid > as it did before, but each task obtains one more pid for each > pid type - the virtual pid. When putting the pid to user or > getting the pid from it kernel operates with the virtual ones. Just a quick reaction. - I would very much like to see a minimum of 3 levels of pids, being supported. Otherwise it is easy to overlook some of the cases that are required to properly support nesting, which long terms seems important. - Semantically fork is easier then unshare. Unshare can mean a lot of things, and it is easy to pick a meaning that has weird side effects. Your implementation has a serious problem in that you change the value of getpid() at runtime. Glibc does not know how to cope with the value of getpid() changing. Eric
creator: ebiederm
date: 2007-05-24T16:00:13-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > This is the largest patch in the set. Make all (I hope) > the places where the pid is shown to or get from user > operate on the virtual pids. > > An exception is copy_process - it was in one of the > previous patches - and the proc - this will come as a > separate patch. This is progress. However you don't currently handle the case of sending a signal from one namespace to another or passing unix credentials from one namespace to another. In particular we need to know the pid of the source task in the destination namespace. Eric
creator: ebiederm
date: 2007-05-24T16:05:59-00:00

Re: [PATCH 2/13] Small preparations for namespaces
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > This includes #ifdefs in get/put_pid_ns and rewriting > the child_reaper() function to the more logical view. > > This doesn't fit logically into any other patch so > I decided to make it separate. > > Signed-off-by: Pavel Emelianov <xemul@openvz.org> > > --- > > diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h > index 169c6c2..7af7191 100644 > --- a/include/linux/pid_namespace.h > +++ b/include/linux/pid_namespace.h > @@ -26,7 +26,9 @@ extern struct pid_namespace init_pid_ns; > > static inline void get_pid_ns(struct pid_namespace *ns) > { > +#ifdef CONFIG_PID_NS > kref_get(&ns->kref); > +#endif > } > > extern struct pid_namespace *copy_pid_ns(int flags, struct pid_namespace *ns); > @@ -34,12 +36,15 @@ extern void free_pid_ns(struct kref *kre > > static inline void put_pid_ns(struct pid_namespace *ns) > { > +#ifdef CONFIG_PID_NS > kref_put(&ns->kref, free_pid_ns); > +#endif > } > > static inline struct task_struct *child_reaper(struct task_struct *tsk) > { > - return init_pid_ns.child_reaper; > + BUG_ON(tsk != current); > + return tsk->nsproxy->pid_ns->child_reaper; > } > > #endif /* _LINUX_PID_NS_H */ This can't be bisect-safe, right? You can't just use tsk->nsproxy->pid_ns, as you've pointed out yourself. -serge
creator: serue
date: 2007-05-24T16:08:40-00:00

Re: [PATCH 1/13] Round up the API
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > The set of functions process_session, task_session, process_group > and task_pgrp is confusing, as the names can be mixed with each other > when looking at the code for a long time. > > The proposals are to > * equip the functions that return the integer with _nr suffix to > represent that fact, > * and to make all functions work with task (not process) by making > the common prefix of the same name. > > For monotony the routines signal_session() and set_signal_session() > are replaced with task_session_nr() and set_task_session(), especially > since they are only used with the explicit task->signal dereference. > > I've sent this before, but Andrew didn't include it, so I resend it > as the part of this set. > > Signed-off-by: Pavel Emelianov <xemul@openvz.org> > Acked-by: Serge E. Hallyn <serue@us.ibm.com> Yup, I still like this patch. thanks, -serge > > --- > > diff --git a/arch/mips/kernel/irixelf.c b/arch/mips/kernel/irixelf.c > index 403d96f..10ba0a5 100644 > --- a/arch/mips/kernel/irixelf.c > +++ b/arch/mips/kernel/irixelf.c > @@ -1170,8 +1170,8 @@ static int irix_core_dump(long signr, st > prstatus.pr_sighold = current->blocked.sig[0]; > psinfo.pr_pid = prstatus.pr_pid = current->pid; > psinfo.pr_ppid = prstatus.pr_ppid = current->parent->pid; > - psinfo.pr_pgrp = prstatus.pr_pgrp = process_group(current); > - psinfo.pr_sid = prstatus.pr_sid = process_session(current); > + psinfo.pr_pgrp = prstatus.pr_pgrp = task_pgrp_nr(current); > + psinfo.pr_sid = prstatus.pr_sid = task_session_nr(current); > if (current->pid == current->tgid) { > /* > * This is the record for the group leader. Add in the > diff --git a/arch/mips/kernel/irixsig.c b/arch/mips/kernel/irixsig.c > index 6980deb..210503e 100644 > --- a/arch/mips/kernel/irixsig.c > +++ b/arch/mips/kernel/irixsig.c > @@ -609,7 +609,7 @@ repeat: > p = list_entry(_p,struct task_struct,sibling); > if ((type == IRIX_P_PID) && p->pid != pid) > continue; > - if ((type == I…
creator: serue
date: 2007-05-24T16:09:46-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Serge E. Hallyn wrote: > Quoting Pavel Emelianov (xemul@openvz.org): >> That's how OpenVZ sees the pid namespaces. >> >> The main idea is that kernel keeps operating with tasks pid >> as it did before, but each task obtains one more pid for each >> pid type - the virtual pid. When putting the pid to user or >> getting the pid from it kernel operates with the virtual ones. >> >> E.g. virtual pid is returned from getpid(), virtual pgid - >> from getpgid() and so on. Getting virtual pid from user is >> performed in setpgid(), setsid() and kill() mainly and in some >> other places. >> >> As far as the namespace are concerned I propose the following >> scheme. The namespace can be created from unshare syscall only. >> This makes fork() code look easier. Of course task must be > > So is your main reason for posting this as a counter to Suka's patchset > the concern of overhead at clone? No, that's just a coincidence that I worked on the same problem. What I propose is another way to make pid namespaces. It has its advantages over Suka's approach. Main are: 1. Lighter exporting of pid to userspace and performance issues on the whole - as you have noticed at least fork() is lighter and many syscalls that return task pids are; 2. Kernel logic of tracking pids is kept - virtual pids are used on kernel-user boundary only; 3. Cleaner logic for namespace migration: with this approach one need to save the virtual pid and let global one change; with Suka's logic this is not clear how to migrate the level 2 namespace (concerning init to be level 0). > thanks, > -serge
creator: xemul
date: 2007-05-24T16:11:30-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Eric W. Biederman wrote: > Pavel Emelianov <xemul@openvz.org> writes: > >> This is the largest patch in the set. Make all (I hope) >> the places where the pid is shown to or get from user >> operate on the virtual pids. >> >> An exception is copy_process - it was in one of the >> previous patches - and the proc - this will come as a >> separate patch. > > > This is progress. However you don't currently handle the > case of sending a signal from one namespace to another or > passing unix credentials from one namespace to another. That's true. Sending of signal from parent ns to children is tricky question. It has many solutions, I wanted to discuss which one is better: 1. Make an "unused" pid in each namespace and use it when signal comes from outside. This resembles the way it is done in OpenVZ. 2. Send the signal like it came from the kernel. > In particular we need to know the pid of the source task > in the destination namespace. But the source task is not always visible in dst. In this case we may use pid, that never exists in the destination, just like it was kill run from bash by user. > Eric >
creator: xemul
date: 2007-05-24T16:15:40-00:00

Re: Instructions of how to make testing easy
type: item
description: Cedric Le Goater wrote: > Hello Pavel ! > > I'm giving it a try. If you want, I can give you complete instructions of how to do it. When I started to test it I faced many tricky places and just want to make sure you won't spent time solving the problems I solved. > For those using qemu, you'll need this patch: > > http://lkml.org/lkml/2007/5/16/360 > > thanks for the patchset pavel. > > C. > >
creator: Pavel Emelianov
date: 2007-05-24T16:16:44-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Quoting Eric W. Biederman (ebiederm@xmission.com): > Pavel Emelianov <xemul@openvz.org> writes: > > > That's how OpenVZ sees the pid namespaces. > > > > The main idea is that kernel keeps operating with tasks pid > > as it did before, but each task obtains one more pid for each > > pid type - the virtual pid. When putting the pid to user or > > getting the pid from it kernel operates with the virtual ones. > > Just a quick reaction. > > - I would very much like to see a minimum of 3 levels of pids, > being supported. Otherwise it is easy to overlook some of the > cases that are required to properly support nesting, which long > terms seems important. Pavel, If I wanted to start a virtual server and in there start some checkpoint restart jobs, so I start a new pid namespace inside the c/r job, what will happen? a. second pidns unshare is refused b. second pidns unshare is allowed, but c/r job is not visible from the virtual server (but is from the global pidns) c. second pidns unshare is allowed, and somehow the c/r job is visible from the virtual server If (a), is this a short-term shortcoming for simplicity of prototype and code review, or do you think it's actually the right thing t do long term? thanks, -serge > - Semantically fork is easier then unshare. Unshare can mean > a lot of things, and it is easy to pick a meaning that has weird > side effects. Your implementation has a serious problem in that you > change the value of getpid() at runtime. Glibc does not know how to > cope with the value of getpid() changing. > > Eric
creator: serue
date: 2007-05-24T16:20:06-00:00

Re: [PATCH 1/13] Round up the API
type: item
description: "Serge E. Hallyn" <serue@us.ibm.com> writes: > Quoting Pavel Emelianov (xemul@openvz.org): >> The set of functions process_session, task_session, process_group >> and task_pgrp is confusing, as the names can be mixed with each other >> when looking at the code for a long time. >> >> The proposals are to >> * equip the functions that return the integer with _nr suffix to >> represent that fact, >> * and to make all functions work with task (not process) by making >> the common prefix of the same name. >> >> For monotony the routines signal_session() and set_signal_session() >> are replaced with task_session_nr() and set_task_session(), especially >> since they are only used with the explicit task->signal dereference. >> >> I've sent this before, but Andrew didn't include it, so I resend it >> as the part of this set. >> >> Signed-off-by: Pavel Emelianov <xemul@openvz.org> >> Acked-by: Serge E. Hallyn <serue@us.ibm.com> > > Yup, I still like this patch. I'm borderline. Less error prone interfaces sound good, less duplication of information sounds good. Changing the names of historical function may be change for the sake of change and thus noise. However if we are going to go this far I think we need to remove the numeric pid cache from the task_struct. Eric
creator: ebiederm
date: 2007-05-24T16:22:45-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Eric W. Biederman wrote: > Pavel Emelianov <xemul@openvz.org> writes: > >> That's how OpenVZ sees the pid namespaces. >> >> The main idea is that kernel keeps operating with tasks pid >> as it did before, but each task obtains one more pid for each >> pid type - the virtual pid. When putting the pid to user or >> getting the pid from it kernel operates with the virtual ones. > > Just a quick reaction. > > - I would very much like to see a minimum of 3 levels of pids, Why not 4? From my part, I would like to know, why such nesting is important. We have plain IPC namespaces and nobody cares. We will have isolated network namespaces, why pids are exception? > being supported. Otherwise it is easy to overlook some of the > cases that are required to properly support nesting, which long > terms seems important. > > - Semantically fork is easier then unshare. Unshare can mean This is not. When you fork, the kid shares the session and the group with its parent, but moving this pids to new ns is bad - the parent will happen to be half-moved. Thus you need to break the session and the group in fork(), but this is extra complexity. > a lot of things, and it is easy to pick a meaning that has weird > side effects. Your implementation has a serious problem in that you > change the value of getpid() at runtime. Glibc does not know how to > cope with the value of getpid() changing. This pid changing happens only once per task lifetime. Though I haven't seen any problems with glibc for many years running OpenVZ and I think, that if glibc will want to cache this getpid() value we can teach it to uncache this value in case someone called unshare() with CLONE_NEWPIDS. > Eric > Thanks, Pavel.
creator: Pavel Emelianov
date: 2007-05-24T16:26:10-00:00

Re: [PATCH 1/13] Round up the API
type: item
description: Eric W. Biederman wrote: > "Serge E. Hallyn" <serue@us.ibm.com> writes: > >> Quoting Pavel Emelianov (xemul@openvz.org): >>> The set of functions process_session, task_session, process_group >>> and task_pgrp is confusing, as the names can be mixed with each other >>> when looking at the code for a long time. >>> >>> The proposals are to >>> * equip the functions that return the integer with _nr suffix to >>> represent that fact, >>> * and to make all functions work with task (not process) by making >>> the common prefix of the same name. >>> >>> For monotony the routines signal_session() and set_signal_session() >>> are replaced with task_session_nr() and set_task_session(), especially >>> since they are only used with the explicit task->signal dereference. >>> >>> I've sent this before, but Andrew didn't include it, so I resend it >>> as the part of this set. >>> >>> Signed-off-by: Pavel Emelianov <xemul@openvz.org> >>> Acked-by: Serge E. Hallyn <serue@us.ibm.com> >> Yup, I still like this patch. > > I'm borderline. Less error prone interfaces sound good, less > duplication of information sounds good. Changing the names of > historical function may be change for the sake of change and > thus noise. They are not historical. These calls appeared soon after new struct pid subsystem. > > However if we are going to go this far I think we need to remove > the numeric pid cache from the task_struct. Object. Numerical pid and tgid on task makes it possible (and this is done in ia64) to export this to user faster. Moreover there can be places in kernel when we still hold the tasks and want to know its pid, but the task is dead already and is going to be delayed_put_task()-ed without pids aboard. I know this can be properly if()-ed but what for? > Eric >
creator: Pavel Emelianov
date: 2007-05-24T16:31:41-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Pavel Emelianov <xemul@sw.ru> writes: > That's true. Sending of signal from parent ns to children > is tricky question. It has many solutions, I wanted to > discuss which one is better: With unix domain sockets and the like it is conceivable we get a pid transfer from one namespace to another and both namespaces are leaf namespaces. I don't remember we can get a leaf to leaf transfer when sending signals. > 1. Make an "unused" pid in each namespace and use it when signal > comes from outside. This resembles the way it is done in OpenVZ. > 2. Send the signal like it came from the kernel. > >> In particular we need to know the pid of the source task >> in the destination namespace. > > But the source task is not always visible in dst. In this case > we may use pid, that never exists in the destination, just like > it was kill run from bash by user. Quite true. So we have the question how do we name a the pid of an unmapped task. The two practical alternatives I see are: - Map the struct pid into the namespace in question. - Use pid == 0 (as if the kernel had generated the signal). - Use pid == -1 (to signal an unknown user space task?) My gut fee is that using pid == 0 is the simplest and most robust way to handle it. That way we don't have information about things outside the pid namespace leaking in. Of course I don't there may be trust issues with reporting a user space process as pid == 0. The worst case I can see with pid == 0. Is that it would be a bug that we can fix later. For other cases it would seem to be a user space API thing that we get stuck with for all time. Eric
creator: ebiederm
date: 2007-05-24T16:33:21-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov wrote: > Eric W. Biederman wrote: > >> Pavel Emelianov <xemul@openvz.org> writes: >> >> >>> That's how OpenVZ sees the pid namespaces. >>> >>> The main idea is that kernel keeps operating with tasks pid >>> as it did before, but each task obtains one more pid for each >>> pid type - the virtual pid. When putting the pid to user or >>> getting the pid from it kernel operates with the virtual ones. >>> >> Just a quick reaction. >> >> - I would very much like to see a minimum of 3 levels of pids, >> > > Why not 4? From my part, I would like to know, why such nesting > is important. We have plain IPC namespaces and nobody cares. > We will have isolated network namespaces, why pids are exception? > Pavel, I am taking advantage to the opportunity to ask you if you plan to send a new network namespace patchset ? -- Daniel _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daniel Lezcano
date: 2007-05-24T16:41:35-00:00

Re: [PATCH 1/13] Round up the API
type: item
description: Quoting Eric W. Biederman (ebiederm@xmission.com): > "Serge E. Hallyn" <serue@us.ibm.com> writes: > > > Quoting Pavel Emelianov (xemul@openvz.org): > >> The set of functions process_session, task_session, process_group > >> and task_pgrp is confusing, as the names can be mixed with each other > >> when looking at the code for a long time. > >> > >> The proposals are to > >> * equip the functions that return the integer with _nr suffix to > >> represent that fact, > >> * and to make all functions work with task (not process) by making > >> the common prefix of the same name. > >> > >> For monotony the routines signal_session() and set_signal_session() > >> are replaced with task_session_nr() and set_task_session(), especially > >> since they are only used with the explicit task->signal dereference. > >> > >> I've sent this before, but Andrew didn't include it, so I resend it > >> as the part of this set. > >> > >> Signed-off-by: Pavel Emelianov <xemul@openvz.org> > >> Acked-by: Serge E. Hallyn <serue@us.ibm.com> > > > > Yup, I still like this patch. > > I'm borderline. Less error prone interfaces sound good, less > duplication of information sounds good. Changing the names of > historical function may be change for the sake of change and > thus noise. > > However if we are going to go this far I think we need to remove > the numeric pid cache from the task_struct. You mean tsk->pid? I agree, especially in Suka's version. Not sure it applies to Pavel's version, though since the "real"/global pid is still stored only in tsk->pid, right? -serge
creator: serue
date: 2007-05-24T16:48:42-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Quoting Pavel Emelianov (xemul@sw.ru): > Serge E. Hallyn wrote: > > Quoting Pavel Emelianov (xemul@openvz.org): > >> That's how OpenVZ sees the pid namespaces. > >> > >> The main idea is that kernel keeps operating with tasks pid > >> as it did before, but each task obtains one more pid for each > >> pid type - the virtual pid. When putting the pid to user or > >> getting the pid from it kernel operates with the virtual ones. > >> > >> E.g. virtual pid is returned from getpid(), virtual pgid - > >> from getpgid() and so on. Getting virtual pid from user is > >> performed in setpgid(), setsid() and kill() mainly and in some > >> other places. > >> > >> As far as the namespace are concerned I propose the following > >> scheme. The namespace can be created from unshare syscall only. > >> This makes fork() code look easier. Of course task must be > > > > So is your main reason for posting this as a counter to Suka's patchset > > the concern of overhead at clone? > > No, that's just a coincidence that I worked on the same problem. > What I propose is another way to make pid namespaces. It has its > advantages over Suka's approach. Main are: > > 1. Lighter exporting of pid to userspace and performance issues > on the whole - as you have noticed at least fork() is > lighter and many syscalls that return task pids are; > 2. Kernel logic of tracking pids is kept - virtual pids are > used on kernel-user boundary only; On the other hand I've really learned to like the consistency of "there is always a single active pid ns for the task from which it sees all other tasks; it is seen in every pid ns for which it has a struct upid." > 3. Cleaner logic for namespace migration: with this approach > one need to save the virtual pid and let global one change; > with Suka's logic this is not clear how to migrate the level > 2 namespace (concerning init to be level 0). This is a very good point. How *would* we migrate the pids at the second level? -serge
creator: serue
date: 2007-05-24T16:59:31-00:00

Re: [PATCH 5/13] Expand the pid/task seeking functions set
type: item
description: On Thu, 2007-05-24 at 16:50 +0400, Pavel Emelianov wrote: > > +struct pid * fastcall __find_vpid(int nr, struct pid_namespace *ns) > +{ > +#ifdef CONFIG_PID_NS > + struct hlist_node *elem; > + struct pid *pid; > +#endif > + > + if (ns == &init_pid_ns) > + return find_pid(nr); > + > +#ifdef CONFIG_PID_NS > + hlist_for_each_entry_rcu(pid, elem, > + &vpid_hash[vpid_hashfn(nr, ns)], vpid_chain) { > + if (pid->vnr == nr && pid->ns == ns) > + return pid; > + } > +#endif > + return NULL; > +} I am a bit worried that there are too many #ifdefs here. Your patch series adds ~20 of them, and they look to me to be mostly in .c files. Section 2 in SubmittingPatches somewhat discourages this. Do you have any plans for cleaning these up? -- Dave _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Dave Hansen
date: 2007-05-24T17:11:04-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: "Serge E. Hallyn" <serue@us.ibm.com> writes: >> 3. Cleaner logic for namespace migration: with this approach >> one need to save the virtual pid and let global one change; >> with Suka's logic this is not clear how to migrate the level >> 2 namespace (concerning init to be level 0). > > This is a very good point. > > How *would* we migrate the pids at the second level? As long as you don't try and restore pids into the initial pid namespace it isn't a problem. You just record the pid hierarchy and the pid for a task in that hierarchy. There really is nothing special going on that should make migration hard. Or did I miss something? Eric
creator: ebiederm
date: 2007-05-24T18:56:53-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Quoting Eric W. Biederman (ebiederm@xmission.com): > "Serge E. Hallyn" <serue@us.ibm.com> writes: > > >> 3. Cleaner logic for namespace migration: with this approach > >> one need to save the virtual pid and let global one change; > >> with Suka's logic this is not clear how to migrate the level > >> 2 namespace (concerning init to be level 0). > > > > This is a very good point. > > > > How *would* we migrate the pids at the second level? > > As long as you don't try and restore pids into the initial pid namespace > it isn't a problem. You just record the pid hierarchy and the pid > for a task in that hierarchy. There really is nothing special going on > that should make migration hard. > > Or did I miss something? Hmm, no, i guess you are right. I was thinking that getting the pid for a process woudl be done purely from userspace, but I guess along with a kernel helper to *set* pids, we could also have a kernel helper to get all pids for all pid namespaces "above" that of the process doing the checkpoint. Makes sense. thanks, -serge
creator: serue
date: 2007-05-24T19:10:57-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > Eric W. Biederman wrote: >> Pavel Emelianov <xemul@openvz.org> writes: >> >>> That's how OpenVZ sees the pid namespaces. >>> >>> The main idea is that kernel keeps operating with tasks pid >>> as it did before, but each task obtains one more pid for each >>> pid type - the virtual pid. When putting the pid to user or >>> getting the pid from it kernel operates with the virtual ones. >> >> Just a quick reaction. >> >> - I would very much like to see a minimum of 3 levels of pids, > > Why not 4? From my part, I would like to know, why such nesting > is important. We have plain IPC namespaces and nobody cares. > We will have isolated network namespaces, why pids are exception? 4+ is fine, and something we will probably care about someday. 3 seems to be the minimum necessary to get people thinking about adding more so we don't have arbitrary special cases, especially in the user interface. At 3 the things are simple enough we don't have to allocate additional data structures etc. If we don't need nesting we don't even need 2 levels, and we can remove the global pid. But we have had that conversation and especially for the current OpenVZ usage we need nesting. Having more then two layers means we are prepared to use pid namespaces more generally. It really isn't that much harder. >> being supported. Otherwise it is easy to overlook some of the >> cases that are required to properly support nesting, which long >> terms seems important. >> >> - Semantically fork is easier then unshare. Unshare can mean > > This is not. When you fork, the kid shares the session and the > group with its parent, but moving this pids to new ns is bad - the > parent will happen to be half-moved. Thus you need to break the > session and the group in fork(), but this is extra complexity. Nope. You will just need to have the child call setsid() if you don't want to share the session and the group. You can perfectly well share the sid and group with the …
creator: ebiederm
date: 2007-05-24T19:18:48-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Serge E. Hallyn wrote: > Quoting Eric W. Biederman (ebiederm@xmission.com): >> Pavel Emelianov <xemul@openvz.org> writes: >> >>> That's how OpenVZ sees the pid namespaces. >>> >>> The main idea is that kernel keeps operating with tasks pid >>> as it did before, but each task obtains one more pid for each >>> pid type - the virtual pid. When putting the pid to user or >>> getting the pid from it kernel operates with the virtual ones. >> Just a quick reaction. >> >> - I would very much like to see a minimum of 3 levels of pids, >> being supported. Otherwise it is easy to overlook some of the >> cases that are required to properly support nesting, which long >> terms seems important. > > Pavel, > > If I wanted to start a virtual server and in there start some checkpoint > restart jobs, so I start a new pid namespace inside the c/r job, what > will happen? What will happen with this namespace on restore? What pids will you assign to it in the parent (but not that init) namespace? a. arbitrary: that means that you don't care that subgroup of tasks in the VS namespace. Thus why don't move them into separate namespace b. try to hold them as they were: this way is likely to fail and can work w/o namespaces at all. So what's your answer? > a. second pidns unshare is refused > b. second pidns unshare is allowed, but c/r job is not visible > from the virtual server (but is from the global pidns) > c. second pidns unshare is allowed, and somehow the c/r job > is visible from the virtual server > > If (a), is this a short-term shortcoming for simplicity of prototype and > code review, or do you think it's actually the right thing t do long > term? > > thanks, > -serge > >> - Semantically fork is easier then unshare. Unshare can mean >> a lot of things, and it is easy to pick a meaning that has weird >> side effects. Your implementation has a serious problem in that you >> change the value of getpid() at runtime. Glibc does not know how to >> cope with the value of getpid() chang…
creator: Pavel Emelianov
date: 2007-05-25T06:29:42-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Eric W. Biederman wrote: > Pavel Emelianov <xemul@sw.ru> writes: > >> That's true. Sending of signal from parent ns to children >> is tricky question. It has many solutions, I wanted to >> discuss which one is better: > > With unix domain sockets and the like it is conceivable we get > a pid transfer from one namespace to another and both namespaces > are leaf namespaces. I don't remember we can get a leaf to leaf > transfer when sending signals. We should not allow any transfer from leaf NS to leaf NS. Should I explain why? >> 1. Make an "unused" pid in each namespace and use it when signal >> comes from outside. This resembles the way it is done in OpenVZ. >> 2. Send the signal like it came from the kernel. >> >>> In particular we need to know the pid of the source task >>> in the destination namespace. >> But the source task is not always visible in dst. In this case >> we may use pid, that never exists in the destination, just like >> it was kill run from bash by user. > > Quite true. So we have the question how do we name a the pid of > an unmapped task. > > The two practical alternatives I see are: > - Map the struct pid into the namespace in question. Bad solution. We will poison the dst namespace > - Use pid == 0 (as if the kernel had generated the signal). Not just pid 0, but SI_KERNEL in si_code. > - Use pid == -1 (to signal an unknown user space task?) Hm... Strange solution. > My gut fee is that using pid == 0 is the simplest and most robust > way to handle it. That way we don't have information about things > outside the pid namespace leaking in. Of course I don't there may > be trust issues with reporting a user space process as pid == 0. > > The worst case I can see with pid == 0. Is that it would be a bug > that we can fix later. For other cases it would seem to be a user > space API thing that we get stuck with for all time. We cannot trust userspace application to expect some pid other than positive. All that we can is either use some always-absent…
creator: Pavel Emelianov
date: 2007-05-25T06:33:22-00:00

Re: [PATCH 2/13] Small preparations for namespaces
type: item
description: Serge E. Hallyn wrote: > Quoting Pavel Emelianov (xemul@openvz.org): >> This includes #ifdefs in get/put_pid_ns and rewriting >> the child_reaper() function to the more logical view. >> >> This doesn't fit logically into any other patch so >> I decided to make it separate. >> >> Signed-off-by: Pavel Emelianov <xemul@openvz.org> >> >> --- >> >> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h >> index 169c6c2..7af7191 100644 >> --- a/include/linux/pid_namespace.h >> +++ b/include/linux/pid_namespace.h >> @@ -26,7 +26,9 @@ extern struct pid_namespace init_pid_ns; >> >> static inline void get_pid_ns(struct pid_namespace *ns) >> { >> +#ifdef CONFIG_PID_NS >> kref_get(&ns->kref); >> +#endif >> } >> >> extern struct pid_namespace *copy_pid_ns(int flags, struct pid_namespace *ns); >> @@ -34,12 +36,15 @@ extern void free_pid_ns(struct kref *kre >> >> static inline void put_pid_ns(struct pid_namespace *ns) >> { >> +#ifdef CONFIG_PID_NS >> kref_put(&ns->kref, free_pid_ns); >> +#endif >> } >> >> static inline struct task_struct *child_reaper(struct task_struct *tsk) >> { >> - return init_pid_ns.child_reaper; >> + BUG_ON(tsk != current); >> + return tsk->nsproxy->pid_ns->child_reaper; >> } >> >> #endif /* _LINUX_PID_NS_H */ > > This can't be bisect-safe, right? You can't just use > tsk->nsproxy->pid_ns, as you've pointed out yourself. I can :) See - I have a proving BUG_ON() here. > > -serge >
creator: Pavel Emelianov
date: 2007-05-25T06:58:20-00:00

Re: [PATCH 1/13] Round up the API
type: item
description: Serge E. Hallyn wrote: > Quoting Eric W. Biederman (ebiederm@xmission.com): >> "Serge E. Hallyn" <serue@us.ibm.com> writes: >> >>> Quoting Pavel Emelianov (xemul@openvz.org): >>>> The set of functions process_session, task_session, process_group >>>> and task_pgrp is confusing, as the names can be mixed with each other >>>> when looking at the code for a long time. >>>> >>>> The proposals are to >>>> * equip the functions that return the integer with _nr suffix to >>>> represent that fact, >>>> * and to make all functions work with task (not process) by making >>>> the common prefix of the same name. >>>> >>>> For monotony the routines signal_session() and set_signal_session() >>>> are replaced with task_session_nr() and set_task_session(), especially >>>> since they are only used with the explicit task->signal dereference. >>>> >>>> I've sent this before, but Andrew didn't include it, so I resend it >>>> as the part of this set. >>>> >>>> Signed-off-by: Pavel Emelianov <xemul@openvz.org> >>>> Acked-by: Serge E. Hallyn <serue@us.ibm.com> >>> Yup, I still like this patch. >> I'm borderline. Less error prone interfaces sound good, less >> duplication of information sounds good. Changing the names of >> historical function may be change for the sake of change and >> thus noise. >> >> However if we are going to go this far I think we need to remove >> the numeric pid cache from the task_struct. > > You mean tsk->pid? > > I agree, especially in Suka's version. Not sure it applies to Pavel's > version, though since the "real"/global pid is still stored only in > tsk->pid, right? No. All objects that have pid (task_struct, signal_struct and pid (struct)) have two ids after this patch - virtual one and global one. > -serge >
creator: Pavel Emelianov
date: 2007-05-25T07:00:21-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Serge E. Hallyn wrote: > Quoting Pavel Emelianov (xemul@sw.ru): >> Serge E. Hallyn wrote: >>> Quoting Pavel Emelianov (xemul@openvz.org): >>>> That's how OpenVZ sees the pid namespaces. >>>> >>>> The main idea is that kernel keeps operating with tasks pid >>>> as it did before, but each task obtains one more pid for each >>>> pid type - the virtual pid. When putting the pid to user or >>>> getting the pid from it kernel operates with the virtual ones. >>>> >>>> E.g. virtual pid is returned from getpid(), virtual pgid - >>>> from getpgid() and so on. Getting virtual pid from user is >>>> performed in setpgid(), setsid() and kill() mainly and in some >>>> other places. >>>> >>>> As far as the namespace are concerned I propose the following >>>> scheme. The namespace can be created from unshare syscall only. >>>> This makes fork() code look easier. Of course task must be >>> So is your main reason for posting this as a counter to Suka's patchset >>> the concern of overhead at clone? >> No, that's just a coincidence that I worked on the same problem. >> What I propose is another way to make pid namespaces. It has its >> advantages over Suka's approach. Main are: >> >> 1. Lighter exporting of pid to userspace and performance issues >> on the whole - as you have noticed at least fork() is >> lighter and many syscalls that return task pids are; >> 2. Kernel logic of tracking pids is kept - virtual pids are >> used on kernel-user boundary only; > > On the other hand I've really learned to like the consistency of "there > is always a single active pid ns for the task from which it sees all > other tasks; it is seen in every pid ns for which it has a struct > upid." > >> 3. Cleaner logic for namespace migration: with this approach >> one need to save the virtual pid and let global one change; >> with Suka's logic this is not clear how to migrate the level >> 2 namespace (concerning init to be level 0). > > This is a very good point. > > How *would* we migrate the pids at the …
creator: Pavel Emelianov
date: 2007-05-25T07:06:49-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Serge E. Hallyn wrote: > Quoting Eric W. Biederman (ebiederm@xmission.com): >> "Serge E. Hallyn" <serue@us.ibm.com> writes: >> >>>> 3. Cleaner logic for namespace migration: with this approach >>>> one need to save the virtual pid and let global one change; >>>> with Suka's logic this is not clear how to migrate the level >>>> 2 namespace (concerning init to be level 0). >>> This is a very good point. >>> >>> How *would* we migrate the pids at the second level? >> As long as you don't try and restore pids into the initial pid namespace >> it isn't a problem. You just record the pid hierarchy and the pid >> for a task in that hierarchy. There really is nothing special going on >> that should make migration hard. >> >> Or did I miss something? > > Hmm, no, i guess you are right. I was thinking that getting the pid for > a process woudl be done purely from userspace, but I guess along with a > kernel helper to *set* pids, we could also have a kernel helper to get > all pids for all pid namespaces "above" that of the process doing the > checkpoint. So do you agree that if we migrate a VS we need to migrate the whole VS? > Makes sense. > > thanks, > -serge >
creator: Pavel Emelianov
date: 2007-05-25T07:08:31-00:00

Re: [PATCH 5/13] Expand the pid/task seeking functions set
type: item
description: Dave Hansen wrote: > On Thu, 2007-05-24 at 16:50 +0400, Pavel Emelianov wrote: >> +struct pid * fastcall __find_vpid(int nr, struct pid_namespace *ns) >> +{ >> +#ifdef CONFIG_PID_NS >> + struct hlist_node *elem; >> + struct pid *pid; >> +#endif >> + >> + if (ns == &init_pid_ns) >> + return find_pid(nr); >> + >> +#ifdef CONFIG_PID_NS >> + hlist_for_each_entry_rcu(pid, elem, >> + &vpid_hash[vpid_hashfn(nr, ns)], vpid_chain) { >> + if (pid->vnr == nr && pid->ns == ns) >> + return pid; >> + } >> +#endif >> + return NULL; >> +} > > I am a bit worried that there are too many #ifdefs here. Your patch > series adds ~20 of them, and they look to me to be mostly in .c files. > Section 2 in SubmittingPatches somewhat discourages this. > > Do you have any plans for cleaning these up? Sure I have. But this approach makes review simpler - everyone explicitly see what exact actions are taken in each place. In the second iteration this will be make in a more elegant way like making static inline stubs etc. This set is a kind of RFC and proof-of-concept. I didn't intent this to be merged to any tree as is. That's why a attached the lats patch with strut in proc to observe the whole tree. BTW, question to Sukadev - how did you test your patches? I do know that ps utility doesn't work without full /proc tree and I don's see similar hacks in your patchset. > -- Dave > > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2007-05-25T07:08:48-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Eric W. Biederman wrote: > Pavel Emelianov <xemul@openvz.org> writes: > >> Eric W. Biederman wrote: >>> Pavel Emelianov <xemul@openvz.org> writes: >>> >>>> That's how OpenVZ sees the pid namespaces. >>>> >>>> The main idea is that kernel keeps operating with tasks pid >>>> as it did before, but each task obtains one more pid for each >>>> pid type - the virtual pid. When putting the pid to user or >>>> getting the pid from it kernel operates with the virtual ones. >>> Just a quick reaction. >>> >>> - I would very much like to see a minimum of 3 levels of pids, >> Why not 4? From my part, I would like to know, why such nesting >> is important. We have plain IPC namespaces and nobody cares. >> We will have isolated network namespaces, why pids are exception? > > 4+ is fine, and something we will probably care about someday. > 3 seems to be the minimum necessary to get people thinking about > adding more so we don't have arbitrary special cases, especially > in the user interface. At 3 the things are simple enough we don't > have to allocate additional data structures etc. > > If we don't need nesting we don't even need 2 levels, and we > can remove the global pid. But we have had that conversation > and especially for the current OpenVZ usage we need nesting. We need nesting but 2 levels is more than enough. Yet again - we have 2 level IPC namespace, 2 level network namespace etc. Generic structures are not always needed. Say, why don't we have N-level page tables in kernel? Why not make them generic? What if some ia128 architecture will require 7-level tables!? > Having more then two layers means we are prepared to use pid namespaces more > generally. It really isn't that much harder. It is not, but do we need to spend so much time on solving not relevant problems? >>> being supported. Otherwise it is easy to overlook some of the >>> cases that are required to properly support nesting, which long >>> terms seems important. >>> >>> - Semantically fork is easier then u…
creator: Pavel Emelianov
date: 2007-05-25T07:15:51-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Daniel Lezcano wrote: > Pavel Emelianov wrote: >> Eric W. Biederman wrote: >> >>> Pavel Emelianov <xemul@openvz.org> writes: >>> >>> >>>> That's how OpenVZ sees the pid namespaces. >>>> >>>> The main idea is that kernel keeps operating with tasks pid >>>> as it did before, but each task obtains one more pid for each >>>> pid type - the virtual pid. When putting the pid to user or >>>> getting the pid from it kernel operates with the virtual ones. >>>> >>> Just a quick reaction. >>> - I would very much like to see a minimum of 3 levels of pids, >>> >> >> Why not 4? From my part, I would like to know, why such nesting >> is important. We have plain IPC namespaces and nobody cares. >> We will have isolated network namespaces, why pids are exception? >> > Pavel, > > I am taking advantage to the opportunity to ask you if you plan to send > a new network namespace patchset ? Unfortunately no :( Right now we're focusing on pids and resource management. > -- Daniel > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Pavel Emelianov
date: 2007-05-25T07:26:17-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov wrote: > Daniel Lezcano wrote: > >> Pavel Emelianov wrote: >> >>> Eric W. Biederman wrote: >>> >>> >>>> Pavel Emelianov <xemul@openvz.org> writes: >>>> >>>> >>>> >>>>> That's how OpenVZ sees the pid namespaces. >>>>> >>>>> The main idea is that kernel keeps operating with tasks pid >>>>> as it did before, but each task obtains one more pid for each >>>>> pid type - the virtual pid. When putting the pid to user or >>>>> getting the pid from it kernel operates with the virtual ones. >>>>> >>>>> >>>> Just a quick reaction. >>>> - I would very much like to see a minimum of 3 levels of pids, >>>> >>>> >>> Why not 4? From my part, I would like to know, why such nesting >>> is important. We have plain IPC namespaces and nobody cares. >>> We will have isolated network namespaces, why pids are exception? >>> >>> >> Pavel, >> >> I am taking advantage to the opportunity to ask you if you plan to send >> a new network namespace patchset ? >> > > Unfortunately no :( Right now we're focusing on pids and > resource management. > Yep, a big part :) Did you, OpenVZ guys, had time to look at Eric's patchset ? _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daniel Lezcano
date: 2007-05-25T08:30:43-00:00

Re: [PATCH 2/13] Small preparations for namespaces
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > Serge E. Hallyn wrote: > > Quoting Pavel Emelianov (xemul@openvz.org): > >> This includes #ifdefs in get/put_pid_ns and rewriting > >> the child_reaper() function to the more logical view. > >> > >> This doesn't fit logically into any other patch so > >> I decided to make it separate. > >> > >> Signed-off-by: Pavel Emelianov <xemul@openvz.org> > >> > >> --- > >> > >> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h > >> index 169c6c2..7af7191 100644 > >> --- a/include/linux/pid_namespace.h > >> +++ b/include/linux/pid_namespace.h > >> @@ -26,7 +26,9 @@ extern struct pid_namespace init_pid_ns; > >> > >> static inline void get_pid_ns(struct pid_namespace *ns) > >> { > >> +#ifdef CONFIG_PID_NS > >> kref_get(&ns->kref); > >> +#endif > >> } > >> > >> extern struct pid_namespace *copy_pid_ns(int flags, struct pid_namespace *ns); > >> @@ -34,12 +36,15 @@ extern void free_pid_ns(struct kref *kre > >> > >> static inline void put_pid_ns(struct pid_namespace *ns) > >> { > >> +#ifdef CONFIG_PID_NS > >> kref_put(&ns->kref, free_pid_ns); > >> +#endif > >> } > >> > >> static inline struct task_struct *child_reaper(struct task_struct *tsk) > >> { > >> - return init_pid_ns.child_reaper; > >> + BUG_ON(tsk != current); > >> + return tsk->nsproxy->pid_ns->child_reaper; > >> } > >> > >> #endif /* _LINUX_PID_NS_H */ > > > > This can't be bisect-safe, right? You can't just use > > tsk->nsproxy->pid_ns, as you've pointed out yourself. > > I can :) See - I have a proving BUG_ON() here. I didn't know BUG_ON()'s actually warded off bugs :) You've tested this with the infamous NFS testcase? I don't see *why* it would work for you, but if you claim it does, I guess you'd know better than I :) -serge
creator: serue
date: 2007-05-25T13:01:18-00:00

Re: [PATCH 1/13] Round up the API
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > Serge E. Hallyn wrote: > > Quoting Eric W. Biederman (ebiederm@xmission.com): > >> "Serge E. Hallyn" <serue@us.ibm.com> writes: > >> > >>> Quoting Pavel Emelianov (xemul@openvz.org): > >>>> The set of functions process_session, task_session, process_group > >>>> and task_pgrp is confusing, as the names can be mixed with each other > >>>> when looking at the code for a long time. > >>>> > >>>> The proposals are to > >>>> * equip the functions that return the integer with _nr suffix to > >>>> represent that fact, > >>>> * and to make all functions work with task (not process) by making > >>>> the common prefix of the same name. > >>>> > >>>> For monotony the routines signal_session() and set_signal_session() > >>>> are replaced with task_session_nr() and set_task_session(), especially > >>>> since they are only used with the explicit task->signal dereference. > >>>> > >>>> I've sent this before, but Andrew didn't include it, so I resend it > >>>> as the part of this set. > >>>> > >>>> Signed-off-by: Pavel Emelianov <xemul@openvz.org> > >>>> Acked-by: Serge E. Hallyn <serue@us.ibm.com> > >>> Yup, I still like this patch. > >> I'm borderline. Less error prone interfaces sound good, less > >> duplication of information sounds good. Changing the names of > >> historical function may be change for the sake of change and > >> thus noise. > >> > >> However if we are going to go this far I think we need to remove > >> the numeric pid cache from the task_struct. > > > > You mean tsk->pid? > > > > I agree, especially in Suka's version. Not sure it applies to Pavel's > > version, though since the "real"/global pid is still stored only in > > tsk->pid, right? > > No. All objects that have pid (task_struct, signal_struct and pid (struct)) > have two ids after this patch - virtual one and global one. (Yes, so wouldn't removing task->pid be pretty detrimental?) Could you outline how you would extend this to 3 levels? Would you just add a…
creator: serue
date: 2007-05-25T13:02:30-00:00

Re: [PATCH 2/13] Small preparations for namespaces
type: item
description: Serge E. Hallyn wrote: > Quoting Pavel Emelianov (xemul@openvz.org): >> Serge E. Hallyn wrote: >>> Quoting Pavel Emelianov (xemul@openvz.org): >>>> This includes #ifdefs in get/put_pid_ns and rewriting >>>> the child_reaper() function to the more logical view. >>>> >>>> This doesn't fit logically into any other patch so >>>> I decided to make it separate. >>>> >>>> Signed-off-by: Pavel Emelianov <xemul@openvz.org> >>>> >>>> --- >>>> >>>> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h >>>> index 169c6c2..7af7191 100644 >>>> --- a/include/linux/pid_namespace.h >>>> +++ b/include/linux/pid_namespace.h >>>> @@ -26,7 +26,9 @@ extern struct pid_namespace init_pid_ns; >>>> >>>> static inline void get_pid_ns(struct pid_namespace *ns) >>>> { >>>> +#ifdef CONFIG_PID_NS >>>> kref_get(&ns->kref); >>>> +#endif >>>> } >>>> >>>> extern struct pid_namespace *copy_pid_ns(int flags, struct pid_namespace *ns); >>>> @@ -34,12 +36,15 @@ extern void free_pid_ns(struct kref *kre >>>> >>>> static inline void put_pid_ns(struct pid_namespace *ns) >>>> { >>>> +#ifdef CONFIG_PID_NS >>>> kref_put(&ns->kref, free_pid_ns); >>>> +#endif >>>> } >>>> >>>> static inline struct task_struct *child_reaper(struct task_struct *tsk) >>>> { >>>> - return init_pid_ns.child_reaper; >>>> + BUG_ON(tsk != current); >>>> + return tsk->nsproxy->pid_ns->child_reaper; >>>> } >>>> >>>> #endif /* _LINUX_PID_NS_H */ >>> This can't be bisect-safe, right? You can't just use >>> tsk->nsproxy->pid_ns, as you've pointed out yourself. >> I can :) See - I have a proving BUG_ON() here. > > I didn't know BUG_ON()'s actually warded off bugs :) It does not, but it says to code reader that this call expects something special. In this case - tsk is expected to be current always. And it is. > You've tested this with the infamous NFS testcase? What testcase do you mean? > I don't see *why* it would work for you, but if you claim it does, I > guess you'd know better than I :) I don't get you here. I've chec…
creator: Pavel Emelianov
date: 2007-05-25T13:21:12-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > Serge E. Hallyn wrote: > > Quoting Eric W. Biederman (ebiederm@xmission.com): > >> "Serge E. Hallyn" <serue@us.ibm.com> writes: > >> > >>>> 3. Cleaner logic for namespace migration: with this approach > >>>> one need to save the virtual pid and let global one change; > >>>> with Suka's logic this is not clear how to migrate the level > >>>> 2 namespace (concerning init to be level 0). > >>> This is a very good point. > >>> > >>> How *would* we migrate the pids at the second level? > >> As long as you don't try and restore pids into the initial pid namespace > >> it isn't a problem. You just record the pid hierarchy and the pid > >> for a task in that hierarchy. There really is nothing special going on > >> that should make migration hard. > >> > >> Or did I miss something? > > > > Hmm, no, i guess you are right. I was thinking that getting the pid for > > a process woudl be done purely from userspace, but I guess along with a > > kernel helper to *set* pids, we could also have a kernel helper to get > > all pids for all pid namespaces "above" that of the process doing the > > checkpoint. > > So do you agree that if we migrate a VS we need to migrate the whole VS? I started to respond, then realized you were probably asking something different than I thought. My original response is below, but here is I think the answer to your question, which is important because I think your question might highlight a misunderstanding about the design of Suka's code. Let's say a vserver is started, and in there a pidns is started for a checkpoint/restart job. So let's say we have PID 13 in the root namespace starting PID 14 in a new namespace. So using (pid, pid_ns) as the terminology, we havd (13,1) as the parent process, and (14,1)=(1,2) as the init of the vserver. Let's ignore other tasks inthe vserver, and just talk about (1402,2) as the init of the checkpoint restart job, so it is (1402,2)=(1,3). And oh, yeah, (1402,2)=(1,3)=(2309,…
creator: serue
date: 2007-05-25T13:25:18-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > Serge E. Hallyn wrote: > > Quoting Eric W. Biederman (ebiederm@xmission.com): > >> Pavel Emelianov <xemul@openvz.org> writes: > >> > >>> That's how OpenVZ sees the pid namespaces. > >>> > >>> The main idea is that kernel keeps operating with tasks pid > >>> as it did before, but each task obtains one more pid for each > >>> pid type - the virtual pid. When putting the pid to user or > >>> getting the pid from it kernel operates with the virtual ones. > >> Just a quick reaction. > >> > >> - I would very much like to see a minimum of 3 levels of pids, > >> being supported. Otherwise it is easy to overlook some of the > >> cases that are required to properly support nesting, which long > >> terms seems important. > > > > Pavel, > > > > If I wanted to start a virtual server and in there start some checkpoint > > restart jobs, so I start a new pid namespace inside the c/r job, what > > will happen? > > What will happen with this namespace on restore? What pids will > you assign to it in the parent (but not that init) namespace? No, no, my question is earlier. Maybe my use of the term "checkpoint/restart job" is confusing, so let me call it a "batch job" instead, with the understanding that it is started with the intent of being safely checkpoint/restartable later on. So in the original batch job, started in a vserver, what will the pids look like in the checkpoint/restart job? But I think I know the answer - you'll leave vpid == pid for these tasks, and only set vpid differently when restarting a job, since that's when you really care? So the only situation where there might be a shortcoming is when restarting a job in a vserver? -serge > a. arbitrary: that means that you don't care that subgroup > of tasks in the VS namespace. Thus why don't move them > into separate namespace > b. try to hold them as they were: this way is likely to fail > and can work w/o namespaces at all. > > So what's your answer? > > > a. second pidns …
creator: serue
date: 2007-05-25T13:29:29-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Serge E. Hallyn wrote: > Quoting Pavel Emelianov (xemul@openvz.org): >> Serge E. Hallyn wrote: >>> Quoting Eric W. Biederman (ebiederm@xmission.com): >>>> "Serge E. Hallyn" <serue@us.ibm.com> writes: >>>> >>>>>> 3. Cleaner logic for namespace migration: with this approach >>>>>> one need to save the virtual pid and let global one change; >>>>>> with Suka's logic this is not clear how to migrate the level >>>>>> 2 namespace (concerning init to be level 0). >>>>> This is a very good point. >>>>> >>>>> How *would* we migrate the pids at the second level? >>>> As long as you don't try and restore pids into the initial pid namespace >>>> it isn't a problem. You just record the pid hierarchy and the pid >>>> for a task in that hierarchy. There really is nothing special going on >>>> that should make migration hard. >>>> >>>> Or did I miss something? >>> Hmm, no, i guess you are right. I was thinking that getting the pid for >>> a process woudl be done purely from userspace, but I guess along with a >>> kernel helper to *set* pids, we could also have a kernel helper to get >>> all pids for all pid namespaces "above" that of the process doing the >>> checkpoint. >> So do you agree that if we migrate a VS we need to migrate the whole VS? > > I started to respond, then realized you were probably asking something > different than I thought. My original response is below, but here is I > think the answer to your question, which is important because I think > your question might highlight a misunderstanding about the design of > Suka's code. > > Let's say a vserver is started, and in there a pidns is started for a > checkpoint/restart job. So let's say we have PID 13 in the root > namespace starting PID 14 in a new namespace. So using (pid, pid_ns) as > the terminology, we havd (13,1) as the parent process, and (14,1)=(1,2) > as the init of the vserver. Let's ignore other tasks inthe vserver, and > just talk about (1402,2) as the init of the checkpoint restart job, so > it is (1…
creator: Pavel Emelianov
date: 2007-05-25T13:53:06-00:00

Re: [PATCH 2/13] Small preparations for namespaces
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > Serge E. Hallyn wrote: > > Quoting Pavel Emelianov (xemul@openvz.org): > >> Serge E. Hallyn wrote: > >>> Quoting Pavel Emelianov (xemul@openvz.org): > >>>> This includes #ifdefs in get/put_pid_ns and rewriting > >>>> the child_reaper() function to the more logical view. > >>>> > >>>> This doesn't fit logically into any other patch so > >>>> I decided to make it separate. > >>>> > >>>> Signed-off-by: Pavel Emelianov <xemul@openvz.org> > >>>> > >>>> --- > >>>> > >>>> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h > >>>> index 169c6c2..7af7191 100644 > >>>> --- a/include/linux/pid_namespace.h > >>>> +++ b/include/linux/pid_namespace.h > >>>> @@ -26,7 +26,9 @@ extern struct pid_namespace init_pid_ns; > >>>> > >>>> static inline void get_pid_ns(struct pid_namespace *ns) > >>>> { > >>>> +#ifdef CONFIG_PID_NS > >>>> kref_get(&ns->kref); > >>>> +#endif > >>>> } > >>>> > >>>> extern struct pid_namespace *copy_pid_ns(int flags, struct pid_namespace *ns); > >>>> @@ -34,12 +36,15 @@ extern void free_pid_ns(struct kref *kre > >>>> > >>>> static inline void put_pid_ns(struct pid_namespace *ns) > >>>> { > >>>> +#ifdef CONFIG_PID_NS > >>>> kref_put(&ns->kref, free_pid_ns); > >>>> +#endif > >>>> } > >>>> > >>>> static inline struct task_struct *child_reaper(struct task_struct *tsk) > >>>> { > >>>> - return init_pid_ns.child_reaper; > >>>> + BUG_ON(tsk != current); > >>>> + return tsk->nsproxy->pid_ns->child_reaper; > >>>> } > >>>> > >>>> #endif /* _LINUX_PID_NS_H */ > >>> This can't be bisect-safe, right? You can't just use > >>> tsk->nsproxy->pid_ns, as you've pointed out yourself. > >> I can :) See - I have a proving BUG_ON() here. > > > > I didn't know BUG_ON()'s actually warded off bugs :) > > It does not, but it says to code reader that this call > expects something special. In this case - tsk is expected > to be current always. And it is. I don't think that's sufficient. It's been awhile so I'm fuzzy on …
creator: serue
date: 2007-05-25T13:55:59-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Quoting Pavel Emelianov (xemul@openvz.org): > Serge E. Hallyn wrote: > > Quoting Pavel Emelianov (xemul@openvz.org): > >> Serge E. Hallyn wrote: > >>> Quoting Eric W. Biederman (ebiederm@xmission.com): > >>>> "Serge E. Hallyn" <serue@us.ibm.com> writes: > >>>> > >>>>>> 3. Cleaner logic for namespace migration: with this approach > >>>>>> one need to save the virtual pid and let global one change; > >>>>>> with Suka's logic this is not clear how to migrate the level > >>>>>> 2 namespace (concerning init to be level 0). > >>>>> This is a very good point. > >>>>> > >>>>> How *would* we migrate the pids at the second level? > >>>> As long as you don't try and restore pids into the initial pid namespace > >>>> it isn't a problem. You just record the pid hierarchy and the pid > >>>> for a task in that hierarchy. There really is nothing special going on > >>>> that should make migration hard. > >>>> > >>>> Or did I miss something? > >>> Hmm, no, i guess you are right. I was thinking that getting the pid for > >>> a process woudl be done purely from userspace, but I guess along with a > >>> kernel helper to *set* pids, we could also have a kernel helper to get > >>> all pids for all pid namespaces "above" that of the process doing the > >>> checkpoint. > >> So do you agree that if we migrate a VS we need to migrate the whole VS? > > > > I started to respond, then realized you were probably asking something > > different than I thought. My original response is below, but here is I > > think the answer to your question, which is important because I think > > your question might highlight a misunderstanding about the design of > > Suka's code. > > > > Let's say a vserver is started, and in there a pidns is started for a > > checkpoint/restart job. So let's say we have PID 13 in the root > > namespace starting PID 14 in a new namespace. So using (pid, pid_ns) as > > the terminology, we havd (13,1) as the parent process, and (14,1)=(1,2) > > as the init of the vserver. Let's ign…
creator: serue
date: 2007-05-25T14:25:50-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > > *I* would like to know how you migrate a *part* of a virtual > server? What happens with pids, IPC ids, network connections? > > There are many entities in VS that are not bound to task, but to > VS and if you migrate only half of them you're risking in loosing > the integrity of the VS. If you don't care it - why do you need > namespaces at all? Well there are other uses for namespaces like providing a context for private mounts. That said the concept for migration is not a partial VS. But a nested VS. Eric
creator: ebiederm
date: 2007-05-25T14:55:27-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > Eric W. Biederman wrote: >> Pavel Emelianov <xemul@sw.ru> writes: >> >>> That's true. Sending of signal from parent ns to children >>> is tricky question. It has many solutions, I wanted to >>> discuss which one is better: >> >> With unix domain sockets and the like it is conceivable we get >> a pid transfer from one namespace to another and both namespaces >> are leaf namespaces. I don't remember we can get a leaf to leaf >> transfer when sending signals. > > We should not allow any transfer from leaf NS to leaf NS. > Should I explain why? In a checkpointable context it is a bad thing, and we can prevent it by carefully setting up all of the namespaces. However it is a fundamental possibility that exists, and because we can avoid it with careful setup. I don't see a reason to deny it if something was either inadvertantly or explicitly causes it to happen. Do you have another reason for denying the transfer that I'm not thinking of? >> >> The worst case I can see with pid == 0. Is that it would be a bug >> that we can fix later. For other cases it would seem to be a user >> space API thing that we get stuck with for all time. > > We cannot trust userspace application to expect some pid other than > positive. All that we can is either use some always-absent pid or > send the signal as SI_KERNEL. > > Our experience show that making decisions like above causes random > applications failures that are hard (or even impossible) to debug. Ok. So I guess I see what you are proposing is picking an arbitrary pid, say pid == 2, and reserving that in all pid namespaces and using it when we have a pid that does not map to a specific namespace. I'm fine with that. All I care about is that we have a solution, preferably simple, to the non-mapped pid problem. Eric
creator: ebiederm
date: 2007-05-25T15:48:29-00:00

Re: [PATCH 5/13] Expand the pid/task seeking functions set
type: item
description: Pavel Emelianov [xemul@openvz.org] wrote: | Dave Hansen wrote: | > On Thu, 2007-05-24 at 16:50 +0400, Pavel Emelianov wrote: | >> +struct pid * fastcall __find_vpid(int nr, struct pid_namespace *ns) | >> +{ | >> +#ifdef CONFIG_PID_NS | >> + struct hlist_node *elem; | >> + struct pid *pid; | >> +#endif | >> + | >> + if (ns == &init_pid_ns) | >> + return find_pid(nr); | >> + | >> +#ifdef CONFIG_PID_NS | >> + hlist_for_each_entry_rcu(pid, elem, | >> + &vpid_hash[vpid_hashfn(nr, ns)], vpid_chain) { | >> + if (pid->vnr == nr && pid->ns == ns) | >> + return pid; | >> + } | >> +#endif | >> + return NULL; | >> +} | > | > I am a bit worried that there are too many #ifdefs here. Your patch | > series adds ~20 of them, and they look to me to be mostly in .c files. | > Section 2 in SubmittingPatches somewhat discourages this. | > | > Do you have any plans for cleaning these up? | | Sure I have. But this approach makes review simpler - everyone | explicitly see what exact actions are taken in each place. In | the second iteration this will be make in a more elegant way | like making static inline stubs etc. | | This set is a kind of RFC and proof-of-concept. I didn't intent | this to be merged to any tree as is. That's why a attached the | lats patch with strut in proc to observe the whole tree. | | BTW, question to Sukadev - how did you test your patches? I do | know that ps utility doesn't work without full /proc tree and | I don's see similar hacks in your patchset. Patches (#13 and #14) in my patchset allow remounting /proc in a child namespace. So the script (lxc-wrap.sh Patch-0) remounts /proc when it enters the new namespace. "ps -e" in the namespace only shows processes from that namespace. "ps -e" in init pid ns shows processes in child namespaces with numeric pids (pid_t) from init pid ns. | | > -- Dave | > | > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinf…
creator: Sukadev Bhattiprolu
date: 2007-05-25T23:36:58-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Serge E. Hallyn wrote: > Quoting Pavel Emelianov (xemul@openvz.org): >> Serge E. Hallyn wrote: >>> Quoting Pavel Emelianov (xemul@openvz.org): >>>> Serge E. Hallyn wrote: >>>>> Quoting Eric W. Biederman (ebiederm@xmission.com): >>>>>> "Serge E. Hallyn" <serue@us.ibm.com> writes: >>>>>> >>>>>>>> 3. Cleaner logic for namespace migration: with this approach >>>>>>>> one need to save the virtual pid and let global one change; >>>>>>>> with Suka's logic this is not clear how to migrate the level >>>>>>>> 2 namespace (concerning init to be level 0). >>>>>>> This is a very good point. >>>>>>> >>>>>>> How *would* we migrate the pids at the second level? >>>>>> As long as you don't try and restore pids into the initial pid namespace >>>>>> it isn't a problem. You just record the pid hierarchy and the pid >>>>>> for a task in that hierarchy. There really is nothing special going on >>>>>> that should make migration hard. >>>>>> >>>>>> Or did I miss something? >>>>> Hmm, no, i guess you are right. I was thinking that getting the pid for >>>>> a process woudl be done purely from userspace, but I guess along with a >>>>> kernel helper to *set* pids, we could also have a kernel helper to get >>>>> all pids for all pid namespaces "above" that of the process doing the >>>>> checkpoint. >>>> So do you agree that if we migrate a VS we need to migrate the whole VS? >>> I started to respond, then realized you were probably asking something >>> different than I thought. My original response is below, but here is I >>> think the answer to your question, which is important because I think >>> your question might highlight a misunderstanding about the design of >>> Suka's code. >>> >>> Let's say a vserver is started, and in there a pidns is started for a >>> checkpoint/restart job. So let's say we have PID 13 in the root >>> namespace starting PID 14 in a new namespace. So using (pid, pid_ns) as >>> the terminology, we havd (13,1) as the parent process, and (14,1)=(1,2) >>> as the init of …
creator: Pavel Emelianov
date: 2007-05-28T07:48:20-00:00

Re: Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov wrote: >>> Though I haven't >>>seen any problems with glibc for many years running OpenVZ and I think, >>>that if glibc will want to cache this getpid() value we can teach it to >>>uncache this value in case someone called unshare() with CLONE_NEWPIDS. >> >>glibc very much caches the results of getpid(). > > > Can you prove it? We have run OpenVZ for many years and with many > userspace configurations and we haven't seen the problems with > glibc ever. Pavel, but we always do full namespace entering with fork()'s and such actions. So we simply couldn't trigger getpid() caching. Thanks, Kirill
creator: dev
date: 2007-05-28T11:50:02-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > Hmm. I see. So you don't care that the pids in the namespace #2 are still > the same. I can understand that politics for namespace #1, but for #2... I'm confused, I think the statement above is wrong. If we just checkpoint/restart a leaf pid namespace we don't care about the other pids, in other namespace. If we checkpoint/restart a pid namespace with another pid namespace nested inside it we need to preserve the pids in the pid namespace we are checkpointing and in a nested pid namespaces. Pids in namespaces that none of the process we are migrating cannot see we do not care about. (i.e. the init pid namespace, and possibly some of it's children) > OK, if you need this let us go on with such model, but I'd like to see > the CONFIG_PID_NS_MULTILEVEL for this. Or at least CONFIG_PID_NS_FLAT for > my model as we do not need to sacrifice the performance to such generic > behavior. Where is the world would a performance sacrafice come in? If you happen to be using a deeply nested pid namespace I can see a small performance hit, there is fundamentally more to do. However if you don't use a nested pid namespace there should not be more work todo and it should be impossible to measure the over head. Further 3 levels should be as simple to implement and as cheap as two levels. Because we can continue to use static allocation. Eric
creator: ebiederm
date: 2007-05-29T04:30:45-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Eric W. Biederman wrote: > Pavel Emelianov <xemul@openvz.org> writes: > >> Hmm. I see. So you don't care that the pids in the namespace #2 are still >> the same. I can understand that politics for namespace #1, but for #2... > > I'm confused, I think the statement above is wrong. > > If we just checkpoint/restart a leaf pid namespace we don't care about > the other pids, in other namespace. > > If we checkpoint/restart a pid namespace with another pid namespace > nested inside it we need to preserve the pids in the pid namespace we > are checkpointing and in a nested pid namespaces. > > Pids in namespaces that none of the process we are migrating cannot > see we do not care about. (i.e. the init pid namespace, and possibly > some of it's children) > >> OK, if you need this let us go on with such model, but I'd like to see >> the CONFIG_PID_NS_MULTILEVEL for this. Or at least CONFIG_PID_NS_FLAT for >> my model as we do not need to sacrifice the performance to such generic >> behavior. > > Where is the world would a performance sacrafice come in? If you Easy! Consider the problem of getting a list of pids for proc. In case of flat layout we just take a number from a known structure. In case of nested pids we have to scan through the list of pid_elem-s or lookup the hash or something similar. The same stays true for wait() when we have to compare pids in the eligible_child(), for setpgid(), terminal ioctls and so on and so forth. Not to be unfounded I will measure booth cases with unixbench's spawn, execl and shell tests and with "ps -xaf" and report the results. All will be run in init namespace and in "level one" namespace. If the flat layout wins (with noticeable difference) I would insist having two of them. Agree? > happen to be using a deeply nested pid namespace I can see a small > performance hit, there is fundamentally more to do. However if you > don't use a nested pid namespace there should not be more work todo > and it should be impossible to measure the o…
creator: Pavel Emelianov
date: 2007-05-29T07:47:35-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Hello ! >>> The worst case I can see with pid == 0. Is that it would be a bug >>> that we can fix later. For other cases it would seem to be a user >>> space API thing that we get stuck with for all time. >> We cannot trust userspace application to expect some pid other than >> positive. All that we can is either use some always-absent pid or >> send the signal as SI_KERNEL. >> >> Our experience show that making decisions like above causes random <>> applications failures that are hard (or even impossible) to debug. > Ok. So I guess I see what you are proposing is picking an arbitrary > pid, say pid == 2, and reserving that in all pid namespaces and using > it when we have a pid that does not map to a specific namespace. I'm > fine with that. > > All I care about is that we have a solution, preferably simple, > to the non-mapped pid problem. Pavel, are you against using pid == 0 and setting si_code to SI_KERNEL ? C.
creator: Cedric Le Goater
date: 2007-05-29T12:32:15-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > Eric W. Biederman wrote: >> Pavel Emelianov <xemul@openvz.org> writes: >> Where is the world would a performance sacrafice come in? If you > > Easy! Consider the problem of getting a list of pids for proc. In case > of flat layout we just take a number from a known structure. In case of > nested pids we have to scan through the list of pid_elem-s or lookup > the hash or something similar. We walk through the pidmap. That should not change either way. I'm actually not horribly fond of walking through the pidmap but it was needed for correctness so we could have a stable token we could return to user space for restarting readdir in /proc. > The same stays true for wait() when we have to compare pids in the > eligible_child(), for setpgid(), terminal ioctls and so on and so forth. We should be comparing struct pid pointers not user space pid_t values. With that being the case we should convert at the edge of user space and all should be good. >> happen to be using a deeply nested pid namespace I can see a small >> performance hit, there is fundamentally more to do. However if you >> don't use a nested pid namespace there should not be more work todo >> and it should be impossible to measure the over head. >> >> Further 3 levels should be as simple to implement and as cheap as two >> levels. Because we can continue to use static allocation. > > Wait a bit. Do you mean that there's enough to have only 3 levels of > namespaces? I.e. to have a struct pid look like > struct pid { > int pid; > int pid1; /* for first level */ > int pid2; /* for 2nd level */ > ... > } > ? Initially yes. 3 levels should be enough. Ultimately we may want more but that should be a small tweak at the implementation level. Nothing outside of the pid functions should care. Eric
creator: ebiederm
date: 2007-05-29T12:36:06-00:00

Re: [PATCH 0/13] Pid namespaces (OpenVZ view)
type: item
description: Hmm. I seem to have forgotten to send this one. Pavel Emelianov <xemul@openvz.org> writes: > Eric W. Biederman wrote: > Generic structures are not always needed. Say, why don't we > have N-level page tables in kernel? Why not make them generic? > What if some ia128 architecture will require 7-level tables!? PID namespaces unlike the other namespaces are fundamentally nested. Which is an unfortunate pain. But if you want to allow nesting of containers of different types such as system containers and application containers you need nested PID namespaces. >> Having more then two layers means we are prepared to use pid namespaces more >> generally. It really isn't that much harder. > > It is not, but do we need to spend so much time on solving > not relevant problems? It is relevant to some of us. Therefore it is a relevant problem. >>>> - Semantically fork is easier then unshare. Unshare can mean >>> This is not. When you fork, the kid shares the session and the >>> group with its parent, but moving this pids to new ns is bad - the >>> parent will happen to be half-moved. Thus you need to break the >>> session and the group in fork(), but this is extra complexity. >> >> Nope. You will just need to have the child call setsid() if >> you don't want to share the session and the group. > > Of course, but setsid() must be done *before* creating a new > namespace, Otherwise you will have a half-inserted into new > namespace task. This sounds awful. We can experience weird interactions, but not really worse then the sending a signal from outside the namespace. So we may want to map the pids of the session and the pgrp into the new namespace but functionally it's not really a big deal, and we can call setsid after the fork. >>>> a lot of things, and it is easy to pick a meaning that has weird >>>> side effects. Your implementation has a serious problem in that you >>>> change the value of getpid() at runtime. Glibc does not know how to >>>> cope with the value of getpid() chan…
creator: ebiederm
date: 2007-05-29T13:07:13-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Cedric Le Goater wrote: > Hello ! > >>>> The worst case I can see with pid == 0. Is that it would be a bug >>>> that we can fix later. For other cases it would seem to be a user >>>> space API thing that we get stuck with for all time. >>> We cannot trust userspace application to expect some pid other than >>> positive. All that we can is either use some always-absent pid or >>> send the signal as SI_KERNEL. >>> >>> Our experience show that making decisions like above causes random > <>> applications failures that are hard (or even impossible) to debug. > >> Ok. So I guess I see what you are proposing is picking an arbitrary >> pid, say pid == 2, and reserving that in all pid namespaces and using >> it when we have a pid that does not map to a specific namespace. I'm >> fine with that. >> >> All I care about is that we have a solution, preferably simple, >> to the non-mapped pid problem. > > Pavel, are you against using pid == 0 and setting si_code to SI_KERNEL ? I think I am. A quick grep through the code revealed one place where this can happen, so I believe application are (have to be) somehow prepared to this. > C. >
creator: Pavel Emelianov
date: 2007-05-31T07:57:37-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Pavel Emelianov wrote: > Cedric Le Goater wrote: >> Hello ! >> >>>>> The worst case I can see with pid == 0. Is that it would be a bug >>>>> that we can fix later. For other cases it would seem to be a user >>>>> space API thing that we get stuck with for all time. >>>> We cannot trust userspace application to expect some pid other than >>>> positive. All that we can is either use some always-absent pid or >>>> send the signal as SI_KERNEL. >>>> >>>> Our experience show that making decisions like above causes random >> <>> applications failures that are hard (or even impossible) to debug. >> >>> Ok. So I guess I see what you are proposing is picking an arbitrary >>> pid, say pid == 2, and reserving that in all pid namespaces and using >>> it when we have a pid that does not map to a specific namespace. I'm >>> fine with that. >>> >>> All I care about is that we have a solution, preferably simple, >>> to the non-mapped pid problem. >> Pavel, are you against using pid == 0 and setting si_code to SI_KERNEL ? > > I think I am. A quick grep through the code revealed one place where Sorry. I have misprinted. I meant "I think I am *NOT*". My bad :( > this can happen, so I believe application are (have to be) somehow > prepared to this. > >> C. >> > >
creator: Pavel Emelianov
date: 2007-05-31T08:00:17-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > Pavel Emelianov wrote: >> Cedric Le Goater wrote: >>> Hello ! >>> >>>>>> The worst case I can see with pid == 0. Is that it would be a bug >>>>>> that we can fix later. For other cases it would seem to be a user >>>>>> space API thing that we get stuck with for all time. >>>>> We cannot trust userspace application to expect some pid other than >>>>> positive. All that we can is either use some always-absent pid or >>>>> send the signal as SI_KERNEL. >>>>> >>>>> Our experience show that making decisions like above causes random >>> <>> applications failures that are hard (or even impossible) to debug. >>> >>>> Ok. So I guess I see what you are proposing is picking an arbitrary >>>> pid, say pid == 2, and reserving that in all pid namespaces and using >>>> it when we have a pid that does not map to a specific namespace. I'm >>>> fine with that. >>>> >>>> All I care about is that we have a solution, preferably simple, >>>> to the non-mapped pid problem. >>> Pavel, are you against using pid == 0 and setting si_code to SI_KERNEL ? >> >> I think I am. A quick grep through the code revealed one place where > > Sorry. I have misprinted. I meant "I think I am *NOT*". My bad :( > >> this can happen, so I believe application are (have to be) somehow >> prepared to this. Where was this. I'd like to follow your complete line of thinking. Eric
creator: ebiederm
date: 2007-05-31T11:26:12-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Eric W. Biederman wrote: > Pavel Emelianov <xemul@openvz.org> writes: > >> Pavel Emelianov wrote: >>> Cedric Le Goater wrote: >>>> Hello ! >>>> >>>>>>> The worst case I can see with pid == 0. Is that it would be a bug >>>>>>> that we can fix later. For other cases it would seem to be a user >>>>>>> space API thing that we get stuck with for all time. >>>>>> We cannot trust userspace application to expect some pid other than >>>>>> positive. All that we can is either use some always-absent pid or >>>>>> send the signal as SI_KERNEL. >>>>>> >>>>>> Our experience show that making decisions like above causes random >>>> <>> applications failures that are hard (or even impossible) to debug. >>>> >>>>> Ok. So I guess I see what you are proposing is picking an arbitrary >>>>> pid, say pid == 2, and reserving that in all pid namespaces and using >>>>> it when we have a pid that does not map to a specific namespace. I'm >>>>> fine with that. >>>>> >>>>> All I care about is that we have a solution, preferably simple, >>>>> to the non-mapped pid problem. >>>> Pavel, are you against using pid == 0 and setting si_code to SI_KERNEL ? >>> I think I am. A quick grep through the code revealed one place where >> Sorry. I have misprinted. I meant "I think I am *NOT*". My bad :( >> >>> this can happen, so I believe application are (have to be) somehow >>> prepared to this. > > Where was this. I'd like to follow your complete line of thinking. The line concerning why I think that sending a signal from SI_KERNEL is good solution? > Eric >
creator: Pavel Emelianov
date: 2007-05-31T11:46:01-00:00

Re: [PATCH 11/13] Changes to show virtual ids to user
type: item
description: Pavel Emelianov <xemul@openvz.org> writes: > Eric W. Biederman wrote: >> Pavel Emelianov <xemul@openvz.org> writes: >> >>> Pavel Emelianov wrote: >>>> Cedric Le Goater wrote: >>>>> Hello ! >>>>> >>>>>>>> The worst case I can see with pid == 0. Is that it would be a bug >>>>>>>> that we can fix later. For other cases it would seem to be a user >>>>>>>> space API thing that we get stuck with for all time. >>>>>>> We cannot trust userspace application to expect some pid other than >>>>>>> positive. All that we can is either use some always-absent pid or >>>>>>> send the signal as SI_KERNEL. >>>>>>> >>>>>>> Our experience show that making decisions like above causes random >>>>> <>> applications failures that are hard (or even impossible) to debug. >>>>> >>>>>> Ok. So I guess I see what you are proposing is picking an arbitrary >>>>>> pid, say pid == 2, and reserving that in all pid namespaces and using >>>>>> it when we have a pid that does not map to a specific namespace. I'm >>>>>> fine with that. >>>>>> >>>>>> All I care about is that we have a solution, preferably simple, >>>>>> to the non-mapped pid problem. >>>>> Pavel, are you against using pid == 0 and setting si_code to SI_KERNEL ? >>>> I think I am. A quick grep through the code revealed one place where >>> Sorry. I have misprinted. I meant "I think I am *NOT*". My bad :( >>> >>>> this can happen, so I believe application are (have to be) somehow >>>> prepared to this. >> >> Where was this. I'd like to follow your complete line of thinking. > > The line concerning why I think that sending a signal from > SI_KERNEL is good solution? Let me just restate everything to be certain we are not getting confused. The problem was what to do with signals from unmmaped pids. You have just said pid == 0 with SI_KERNEL seems to work. The kernel occasionally sends signal that way already. The primary argument against this in my memory was that we a user space application might treat the kernel case special (more trust), so …
creator: ebiederm
date: 2007-05-31T13:41:22-00:00