Subject: [PATCH 08/17] Pid-NS(V3) Define/use pid->upid_list list.

From: Sukadev Bhattiprolu <sukadev@us.ibm.com>


With multiple pid namespaces, a process would be known by several pid_t
values, one in each pid namespace. To represent this, we introduce a
'struct upid' which associates a single pid_t value with a single pid
namespace.

We then replace the pid->nr field in 'struct pid' with a list of struct upid'
entries (referred to as 'pid->upid_list'). This list represents the multiple
pid_t values of the process, one in each namespace. The current patch adds
just one element to this list, corresponding to 'init_pid_ns'. Subsequent
patches implement multiple pid namespaces and add more elements to the list.

The 'struct upid' also replaces 'struct pid' in the pid_hash table to enable us
to find processes given a pid_t from any namespace (i.e we find 'struct upid'
for a given pid_t and from the 'struct upid', we find the 'struct pid' of the
process)

We finally reimplement find_pid() and pid_to_nr() to use pid->upid_list
and remove unused fields from 'struct pid'.

Changelog:
	2.6.21-mm2-pidns3:

	- 'struct upid' used to be called 'struct pid_nr' and a list of these
	   were hanging off of 'struct pid'. So, we renamed 'struct pid_nr'
	   and now hold them in a statically sized array in 'struct pid' since
	   the number of 'struct upid's for a process is known at process-
	   creation time.
	  
	2.6.21-rc3-mm2:

	- [Eric Biederman] Combine all logical changes into one patch
	- [Eric Biederman] Implement __pid_nr(pid_ns, pid) for use in procfs.
	  (now called pid_to_nr_in_ns()).
	- [Serge Hallyn]: Remove (!pid_nr) check in free_pid_nr()

Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
---
 fs/proc/array.c               |   30 ++++++--
 fs/proc/base.c                |    9 ++
 include/linux/init_task.h     |   14 +++-
 include/linux/pid.h           |   62 ++++++++++++++---
 include/linux/pid_namespace.h |   15 ++++
 kernel/fork.c                 |    2 
 kernel/pid.c                  |  145 +++++++++++++++++++++++++++++++++---------
 7 files changed, 220 insertions(+), 57 deletions(-)

Index: lx26-22-rc4-mm2/include/linux/pid.h
===================================================================
--- lx26-22-rc4-mm2.orig/include/linux/pid.h	2007-06-15 18:44:50.000000000 -0700
+++ lx26-22-rc4-mm2/include/linux/pid.h	2007-06-15 19:47:58.000000000 -0700
@@ -16,6 +16,25 @@ enum pid_type
 	PIDTYPE_MAX
 };
 
+struct pid_namespace;
+
+/*
+ * A struct upid holds a process identifier (or pid->nr) for a given
+ * pid namespace.
+ *
+ * A list of 'struct upid' entries is stored in the struct pid. This list
+ * is used to get the process identifier associated with the pid
+ * namespace it is being seen from.
+ */
+struct upid
+{
+	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
+	struct hlist_node pid_chain; 	/* link hash collisions on pid_hash */
+	int nr;	 			/* user space pid number */
+	struct pid_namespace *pid_ns; 	/* pid namespace in which nr is valid */
+	struct pid *pid; 		/* back to task's unique kernel pid */
+};
+
 /*
  * What is struct pid?
  *
@@ -48,12 +67,11 @@ enum pid_type
 struct pid
 {
 	atomic_t count;
-	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
-	int nr;
-	struct hlist_node pid_chain;
 	/* lists of tasks that use this pid */
 	struct hlist_head tasks[PIDTYPE_MAX];
 	struct rcu_head rcu;
+	int num_upids;
+	struct upid upid_list[1];
 };
 
 extern struct pid init_struct_pid;
@@ -100,16 +118,10 @@ extern struct pid *FASTCALL(find_pid(int
 extern struct pid *find_get_pid(int nr);
 extern struct pid *find_ge_pid(int nr);
 
-extern struct pid *alloc_pid(enum copy_process_type);
+extern struct pid *dup_struct_pid(enum copy_process_type);
 extern void FASTCALL(free_pid(struct pid *pid));
 
-static inline pid_t pid_to_nr(struct pid *pid)
-{
-	pid_t nr = 0;
-	if (pid)
-		nr = pid->nr;
-	return nr;
-}
+extern pid_t pid_to_nr(struct pid *pid);
 
 #define do_each_pid_task(pid, type, task)				\
 	do {								\
@@ -122,4 +134,32 @@ static inline pid_t pid_to_nr(struct pid
 			}						\
 	} while (0)
 
+/*
+ * Return the pid_t by which the process @pid is known in the pid
+ * namespace @ns.
+ *
+ * Return 0 if:
+ * 	- @pid is NULL (eg: procfs calls this for task_pgrp(init_task)
+ * 	  which is NULL).
+ *
+ * 	- process does not have pid_t in the namespace @ns (eg: parent
+ * 	  process of a child reaper does not exist in the child namespace.
+ * 	  A getppid() call by the child reaper results in 0).
+ */
+static inline pid_t pid_to_nr_in_ns(struct pid *pid, struct pid_namespace *ns)
+{
+	int i;
+	struct upid *upid;
+
+	if (!pid)
+		return 0;
+
+	for (i = 0; i < pid->num_upids; i++) {
+		upid = &pid->upid_list[i];
+		if (upid->pid_ns == ns)
+			return upid->nr;
+	}
+	return 0;
+}
+
 #endif /* _LINUX_PID_H */
Index: lx26-22-rc4-mm2/include/linux/init_task.h
===================================================================
--- lx26-22-rc4-mm2.orig/include/linux/init_task.h	2007-06-15 18:44:07.000000000 -0700
+++ lx26-22-rc4-mm2/include/linux/init_task.h	2007-06-15 19:47:58.000000000 -0700
@@ -89,19 +89,27 @@ extern struct nsproxy init_nsproxy;
 
 extern struct group_info init_groups;
 
-#define INIT_STRUCT_PID {						\
-	.count 		= ATOMIC_INIT(1),				\
-	.nr		= 0, 						\
+#define INIT_STRUCT_UPID {						\
+	.nr		= 0,						\
 	/* Don't put this struct pid in pid_hash */			\
 	.pid_chain	= { .next = NULL, .pprev = NULL },		\
+	.pid_ns		= &init_pid_ns,					\
+	.pid		= &init_struct_pid,				\
+}
+
+#define INIT_STRUCT_PID {						\
+	.count 		= ATOMIC_INIT(1),				\
 	.tasks		= {						\
 		{ .first = &init_task.pids[PIDTYPE_PID].node },		\
 		{ .first = &init_task.pids[PIDTYPE_PGID].node },	\
 		{ .first = &init_task.pids[PIDTYPE_SID].node },		\
 	},								\
 	.rcu		= RCU_HEAD_INIT,				\
+	.num_upids	= 1,						\
+	.upid_list 	= { INIT_STRUCT_UPID },				\
 }
 
+
 #define INIT_PID_LINK(type) 					\
 {								\
 	.node = {						\
Index: lx26-22-rc4-mm2/kernel/pid.c
===================================================================
--- lx26-22-rc4-mm2.orig/kernel/pid.c	2007-06-15 18:44:50.000000000 -0700
+++ lx26-22-rc4-mm2/kernel/pid.c	2007-06-15 19:48:11.000000000 -0700
@@ -30,6 +30,7 @@
 #include <linux/init_task.h>
 
 #define pid_hashfn(nr) hash_long((unsigned long)nr, pidhash_shift)
+
 static struct hlist_head *pid_hash;
 static int pidhash_shift;
 static struct kmem_cache *pid_cachep;
@@ -179,10 +180,67 @@ static int next_pidmap(struct pid_namesp
 	return -1;
 }
 
+static void clear_upid(struct upid *upid)
+{
+	/* We can be called with write_lock_irq(&tasklist_lock) held */
+	unsigned long flags;
+
+	free_pidmap(upid->pid_ns, upid->nr);
+
+	spin_lock_irqsave(&pidmap_lock, flags);
+	hlist_del_rcu(&upid->pid_chain);
+	spin_unlock_irqrestore(&pidmap_lock, flags);
+
+	put_pid_ns(upid->pid_ns);
+}
+
+static int init_upid(struct upid *upid, struct pid *pid,
+		struct pid_namespace *pid_ns)
+{
+	int nr;
+
+	nr = alloc_pidmap(pid_ns);
+	if (nr < 0)
+		return nr;
+
+	upid->pid_ns = get_pid_ns(pid_ns);
+	upid->nr = nr;
+
+	/*
+	 * The pid and upid are created and destroyed at the same time,
+	 * as a single unit.  We don't need to refcount the pid use
+	 * here.
+	 */
+	upid->pid = pid;
+
+	INIT_HLIST_NODE(&upid->pid_chain);
+	spin_lock_irq(&pidmap_lock);
+	hlist_add_head_rcu(&upid->pid_chain, &pid_hash[pid_hashfn(nr)]);
+	spin_unlock_irq(&pidmap_lock);
+
+	return 0;
+}
+
+/*
+ * Return the pid_t by which the process @pid is known in the active
+ * pid namespace of the caller.
+ *
+ * TODO: pid_to_nr() cannot be easily inlined due to dependencies between
+ *      'task_struct', 'nsproxy' and 'pid_namespace'. As a part of
+ *      implementing multiple pid namespaces, we remove 'pid_namespace'
+ *      from 'nsproxy' and inline this function.
+ */
+pid_t pid_to_nr(struct pid *pid)
+{
+	return pid_to_nr_in_ns(pid, task_active_pid_ns(current));
+}
+EXPORT_SYMBOL_GPL(pid_to_nr);
+
 fastcall void put_pid(struct pid *pid)
 {
 	if (!pid)
 		return;
+
 	if ((atomic_read(&pid->count) == 1) ||
 	     atomic_dec_and_test(&pid->count))
 		kmem_cache_free(pid_cachep, pid);
@@ -197,66 +255,91 @@ static void delayed_put_pid(struct rcu_h
 
 fastcall void free_pid(struct pid *pid)
 {
-	/* We can be called with write_lock_irq(&tasklist_lock) held */
-	unsigned long flags;
+	int i;
+	struct upid *upid;
 
 	/* check this here to keep copy_process() cleaner */
 	if (unlikely(pid == &init_struct_pid))
 		return;
 
-	spin_lock_irqsave(&pidmap_lock, flags);
-	hlist_del_rcu(&pid->pid_chain);
-	spin_unlock_irqrestore(&pidmap_lock, flags);
+	/* clear any upids that we actually initialized */
+	for (i = 0; i < pid->num_upids; i++) {
+		upid = &pid->upid_list[i];
+		if (!upid->pid_ns)
+			break;
+		clear_upid(upid);
+	}
 
-	free_pidmap(&init_pid_ns, pid->nr);
 	call_rcu(&pid->rcu, delayed_put_pid);
 }
 
-struct pid *alloc_pid(enum copy_process_type copy_src)
+static struct pid *alloc_struct_pid(int num_upids)
 {
 	struct pid *pid;
 	enum pid_type type;
-	int nr = -1;
-
-	/* check this here to keep copy_process() cleaner */
-	if (unlikely(copy_src == COPY_IDLE_PROCESS))
-		return &init_struct_pid;
 
-	pid = kmem_cache_alloc(pid_cachep, GFP_KERNEL);
+	/* for now we only support one pid namespace */
+	BUG_ON(num_upids != 1);
+	pid = kmem_cache_zalloc(pid_cachep, GFP_KERNEL);
 	if (!pid)
-		goto out;
-
-	nr = alloc_pidmap(task_active_pid_ns(current));
-	if (nr < 0)
-		goto out_free;
+		return NULL;
 
 	atomic_set(&pid->count, 1);
-	pid->nr = nr;
+	pid->num_upids = num_upids;
+
 	for (type = 0; type < PIDTYPE_MAX; ++type)
 		INIT_HLIST_HEAD(&pid->tasks[type]);
 
-	spin_lock_irq(&pidmap_lock);
-	hlist_add_head_rcu(&p

Pavel Emelianov [xemul@openvz.org] wrote:
| sukadev@us.ibm.com wrote:
| > Subject: [PATCH 08/17] Pid-NS(V3) Define/use pid->upid_list list.
| > 
| > From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
| > 
| > 
| > With multiple pid namespaces, a process would be known by several pid_t
| > values, one in each pid namespace. To represent this, we introduce a
| > 'struct upid' which associates a single pid_t value with a single pid
| > namespace.
| > 
| > We then replace the pid->nr field in 'struct pid' with a list of struct upid'
| > entries (referred to as 'pid->upid_list'). This list represents the multiple
| > pid_t values of the process, one in each namespace. The current patch adds
| > just one element to this list, corresponding to 'init_pid_ns'. Subsequent
| > patches implement multiple pid namespaces and add more elements to the list.
| > 
| > The 'struct upid' also replaces 'struct pid' in the pid_hash table to enable us
| > to find processes given a pid_t from any namespace (i.e we find 'struct upid'
| > for a given pid_t and from the 'struct upid', we find the 'struct pid' of the
| > process)
| > 
| > We finally reimplement find_pid() and pid_to_nr() to use pid->upid_list
| > and remove unused fields from 'struct pid'.
| > 
| > Changelog:
| > 	2.6.21-mm2-pidns3:
| > 
| > 	- 'struct upid' used to be called 'struct pid_nr' and a list of these
| > 	   were hanging off of 'struct pid'. So, we renamed 'struct pid_nr'
| > 	   and now hold them in a statically sized array in 'struct pid' since
| > 	   the number of 'struct upid's for a process is known at process-
| > 	   creation time.
| > 	  
| > 	2.6.21-rc3-mm2:
| > 
| > 	- [Eric Biederman] Combine all logical changes into one patch
| > 	- [Eric Biederman] Implement __pid_nr(pid_ns, pid) for use in procfs.
| > 	  (now called pid_to_nr_in_ns()).
| > 	- [Serge Hallyn]: Remove (!pid_nr) check in free_pid_nr()
| > 
| > Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
| > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
| > ---
| >  fs/proc/array.c               |   30 ++++++--
| >  fs/proc/base.c                |    9 ++
| >  include/linux/init_task.h     |   14 +++-
| >  include/linux/pid.h           |   62 ++++++++++++++---
| >  include/linux/pid_namespace.h |   15 ++++
| >  kernel/fork.c                 |    2 
| >  kernel/pid.c                  |  145 +++++++++++++++++++++++++++++++++---------
| >  7 files changed, 220 insertions(+), 57 deletions(-)
| > 
| > Index: lx26-22-rc4-mm2/include/linux/pid.h
| > ===================================================================
| > --- lx26-22-rc4-mm2.orig/include/linux/pid.h	2007-06-15 18:44:50.000000000 -0700
| > +++ lx26-22-rc4-mm2/include/linux/pid.h	2007-06-15 19:47:58.000000000 -0700
| > @@ -16,6 +16,25 @@ enum pid_type
| >  	PIDTYPE_MAX
| >  };
| >  
| > +struct pid_namespace;
| > +
| > +/*
| > + * A struct upid holds a process identifier (or pid->nr) for a given
| > + * pid namespace.
| > + *
| > + * A list of 'struct upid' entries is stored in the struct pid. This list
| > + * is used to get the process identifier associated with the pid
| > + * namespace it is being seen from.
| > + */
| > +struct upid
| > +{
| > +	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
| > +	struct hlist_node pid_chain; 	/* link hash collisions on pid_hash */
| > +	int nr;	 			/* user space pid number */
| > +	struct pid_namespace *pid_ns; 	/* pid namespace in which nr is valid */
| > +	struct pid *pid; 		/* back to task's unique kernel pid */
| > +};
| > +
| >  /*
| >   * What is struct pid?
| >   *
| > @@ -48,12 +67,11 @@ enum pid_type
| >  struct pid
| >  {
| >  	atomic_t count;
| > -	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
| > -	int nr;
| > -	struct hlist_node pid_chain;
| >  	/* lists of tasks that use this pid */
| >  	struct hlist_head tasks[PIDTYPE_MAX];
| >  	struct rcu_head rcu;
| > +	int num_upids;
| > +	struct upid upid_list[1];
| 
| Further in your patches you define MAX_NESTED_PID_NS. What for, you
| use the linked list here!?

Hmm. I don't understand. upid_list[] is an array (and not a linked
list). Are you saying the '_list' in 'upid_list' is misleading ?

Placing a limit like MAX_NESTED_PID_NS simplifies allocation of
'struct pid'.


| 
| >  };
| >  
| >  extern struct pid init_struct_pid;
| 
| [snip]
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

sukadev@us.ibm.com wrote:
> Subject: [PATCH 08/17] Pid-NS(V3) Define/use pid->upid_list list.
> 
> From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> 
> 
> With multiple pid namespaces, a process would be known by several pid_t
> values, one in each pid namespace. To represent this, we introduce a
> 'struct upid' which associates a single pid_t value with a single pid
> namespace.
> 
> We then replace the pid->nr field in 'struct pid' with a list of struct upid'
> entries (referred to as 'pid->upid_list'). This list represents the multiple
> pid_t values of the process, one in each namespace. The current patch adds
> just one element to this list, corresponding to 'init_pid_ns'. Subsequent
> patches implement multiple pid namespaces and add more elements to the list.
> 
> The 'struct upid' also replaces 'struct pid' in the pid_hash table to enable us
> to find processes given a pid_t from any namespace (i.e we find 'struct upid'
> for a given pid_t and from the 'struct upid', we find the 'struct pid' of the
> process)
> 
> We finally reimplement find_pid() and pid_to_nr() to use pid->upid_list
> and remove unused fields from 'struct pid'.
> 
> Changelog:
> 	2.6.21-mm2-pidns3:
> 
> 	- 'struct upid' used to be called 'struct pid_nr' and a list of these
> 	   were hanging off of 'struct pid'. So, we renamed 'struct pid_nr'
> 	   and now hold them in a statically sized array in 'struct pid' since
> 	   the number of 'struct upid's for a process is known at process-
> 	   creation time.
> 	  
> 	2.6.21-rc3-mm2:
> 
> 	- [Eric Biederman] Combine all logical changes into one patch
> 	- [Eric Biederman] Implement __pid_nr(pid_ns, pid) for use in procfs.
> 	  (now called pid_to_nr_in_ns()).
> 	- [Serge Hallyn]: Remove (!pid_nr) check in free_pid_nr()
> 
> Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> ---
>  fs/proc/array.c               |   30 ++++++--
>  fs/proc/base.c                |    9 ++
>  include/linux/init_task.h     |   14 +++-
>  include/linux/pid.h           |   62 ++++++++++++++---
>  include/linux/pid_namespace.h |   15 ++++
>  kernel/fork.c                 |    2 
>  kernel/pid.c                  |  145 +++++++++++++++++++++++++++++++++---------
>  7 files changed, 220 insertions(+), 57 deletions(-)
> 
> Index: lx26-22-rc4-mm2/include/linux/pid.h
> ===================================================================
> --- lx26-22-rc4-mm2.orig/include/linux/pid.h	2007-06-15 18:44:50.000000000 -0700
> +++ lx26-22-rc4-mm2/include/linux/pid.h	2007-06-15 19:47:58.000000000 -0700
> @@ -16,6 +16,25 @@ enum pid_type
>  	PIDTYPE_MAX
>  };
>  
> +struct pid_namespace;
> +
> +/*
> + * A struct upid holds a process identifier (or pid->nr) for a given
> + * pid namespace.
> + *
> + * A list of 'struct upid' entries is stored in the struct pid. This list
> + * is used to get the process identifier associated with the pid
> + * namespace it is being seen from.
> + */
> +struct upid
> +{
> +	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
> +	struct hlist_node pid_chain; 	/* link hash collisions on pid_hash */
> +	int nr;	 			/* user space pid number */
> +	struct pid_namespace *pid_ns; 	/* pid namespace in which nr is valid */
> +	struct pid *pid; 		/* back to task's unique kernel pid */
> +};
> +
>  /*
>   * What is struct pid?
>   *
> @@ -48,12 +67,11 @@ enum pid_type
>  struct pid
>  {
>  	atomic_t count;
> -	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
> -	int nr;
> -	struct hlist_node pid_chain;
>  	/* lists of tasks that use this pid */
>  	struct hlist_head tasks[PIDTYPE_MAX];
>  	struct rcu_head rcu;
> +	int num_upids;
> +	struct upid upid_list[1];

Further in your patches you define MAX_NESTED_PID_NS. What for, you
use the linked list here!?

>  };
>  
>  extern struct pid init_struct_pid;

[snip]
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Pavel Emelianov [xemul@openvz.org] wrote:
| sukadev@us.ibm.com wrote:
| > Pavel Emelianov [xemul@openvz.org] wrote:
| > | sukadev@us.ibm.com wrote:
| > | > Subject: [PATCH 08/17] Pid-NS(V3) Define/use pid->upid_list list.
| > | > 
| > | > From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
| > | > 
| > | > 
| > | > With multiple pid namespaces, a process would be known by several pid_t
| > | > values, one in each pid namespace. To represent this, we introduce a
| > | > 'struct upid' which associates a single pid_t value with a single pid
| > | > namespace.
| > | > 
| > | > We then replace the pid->nr field in 'struct pid' with a list of struct upid'
| > | > entries (referred to as 'pid->upid_list'). This list represents the multiple
| > | > pid_t values of the process, one in each namespace. The current patch adds
| > | > just one element to this list, corresponding to 'init_pid_ns'. Subsequent
| > | > patches implement multiple pid namespaces and add more elements to the list.
| > | > 
| > | > The 'struct upid' also replaces 'struct pid' in the pid_hash table to enable us
| > | > to find processes given a pid_t from any namespace (i.e we find 'struct upid'
| > | > for a given pid_t and from the 'struct upid', we find the 'struct pid' of the
| > | > process)
| > | > 
| > | > We finally reimplement find_pid() and pid_to_nr() to use pid->upid_list
| > | > and remove unused fields from 'struct pid'.
| > | > 
| > | > Changelog:
| > | > 	2.6.21-mm2-pidns3:
| > | > 
| > | > 	- 'struct upid' used to be called 'struct pid_nr' and a list of these
| > | > 	   were hanging off of 'struct pid'. So, we renamed 'struct pid_nr'
| > | > 	   and now hold them in a statically sized array in 'struct pid' since
| > | > 	   the number of 'struct upid's for a process is known at process-
| > | > 	   creation time.
| > | > 	  
| > | > 	2.6.21-rc3-mm2:
| > | > 
| > | > 	- [Eric Biederman] Combine all logical changes into one patch
| > | > 	- [Eric Biederman] Implement __pid_nr(pid_ns, pid) for use in procfs.
| > | > 	  (now called pid_to_nr_in_ns()).
| > | > 	- [Serge Hallyn]: Remove (!pid_nr) check in free_pid_nr()
| > | > 
| > | > Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
| > | > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
| > | > ---
| > | >  fs/proc/array.c               |   30 ++++++--
| > | >  fs/proc/base.c                |    9 ++
| > | >  include/linux/init_task.h     |   14 +++-
| > | >  include/linux/pid.h           |   62 ++++++++++++++---
| > | >  include/linux/pid_namespace.h |   15 ++++
| > | >  kernel/fork.c                 |    2 
| > | >  kernel/pid.c                  |  145 +++++++++++++++++++++++++++++++++---------
| > | >  7 files changed, 220 insertions(+), 57 deletions(-)
| > | > 
| > | > Index: lx26-22-rc4-mm2/include/linux/pid.h
| > | > ===================================================================
| > | > --- lx26-22-rc4-mm2.orig/include/linux/pid.h	2007-06-15 18:44:50.000000000 -0700
| > | > +++ lx26-22-rc4-mm2/include/linux/pid.h	2007-06-15 19:47:58.000000000 -0700
| > | > @@ -16,6 +16,25 @@ enum pid_type
| > | >  	PIDTYPE_MAX
| > | >  };
| > | >  
| > | > +struct pid_namespace;
| > | > +
| > | > +/*
| > | > + * A struct upid holds a process identifier (or pid->nr) for a given
| > | > + * pid namespace.
| > | > + *
| > | > + * A list of 'struct upid' entries is stored in the struct pid. This list
| > | > + * is used to get the process identifier associated with the pid
| > | > + * namespace it is being seen from.
| > | > + */
| > | > +struct upid
| > | > +{
| > | > +	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
| > | > +	struct hlist_node pid_chain; 	/* link hash collisions on pid_hash */
| > | > +	int nr;	 			/* user space pid number */
| > | > +	struct pid_namespace *pid_ns; 	/* pid namespace in which nr is valid */
| > | > +	struct pid *pid; 		/* back to task's unique kernel pid */
| > | > +};
| > | > +
| > | >  /*
| > | >   * What is struct pid?
| > | >   *
| > | > @@ -48,12 +67,11 @@ enum pid_type
| > | >  struct pid
| > | >  {
| > | >  	atomic_t count;
| > | > -	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
| > | > -	int nr;
| > | > -	struct hlist_node pid_chain;
| > | >  	/* lists of tasks that use this pid */
| > | >  	struct hlist_head tasks[PIDTYPE_MAX];
| > | >  	struct rcu_head rcu;
| > | > +	int num_upids;
| > | > +	struct upid upid_list[1];
| > | 
| > | Further in your patches you define MAX_NESTED_PID_NS. What for, you
| > | use the linked list here!?
| > 
| > Hmm. I don't understand. upid_list[] is an array (and not a linked
| > list). Are you saying the '_list' in 'upid_list' is misleading ?
| 
| Oh, I see! You allocate all the upids in one chunk. I have missed
| that, sorry :)
| 
| > Placing a limit like MAX_NESTED_PID_NS simplifies allocation of
| > 'struct pid'.
| 
| How? If we have, say, 100-level namespace than we have to create
| the sizeof(struct pid) + 100 * sizeof(struct upid) bytes.

I should have been a little more clear.

I was comparing this with my previous version which did not have the
MAX_NESTED_PID_NS limit and allowed for arbitrary levels of nesting
(100 or even 1000 :-). Allocating that kind of 'struct pid' is more
complex and looks like an overkill at this time.

With a limit like MAX_NESTED_PID_NS, we could in theory create that
many pid caches, one for each level of nesting and use the appropriate
cache in clone().

| 
| > 
| > | 
| > | >  };
| > | >  
| > | >  extern struct pid init_struct_pid;
| > | 
| > | [snip]
| > _______________________________________________
| > Containers mailing list
| > Containers@lists.linux-foundation.org
| > https://lists.linux-foundation.org/mailman/listinfo/containers
| > 
| > _______________________________________________
| > Devel mailing list
| > Devel@openvz.org
| > https://openvz.org/mailman/listinfo/devel
| > 
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

sukadev@us.ibm.com wrote:
> Pavel Emelianov [xemul@openvz.org] wrote:
> | sukadev@us.ibm.com wrote:
> | > Subject: [PATCH 08/17] Pid-NS(V3) Define/use pid->upid_list list.
> | > 
> | > From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> | > 
> | > 
> | > With multiple pid namespaces, a process would be known by several pid_t
> | > values, one in each pid namespace. To represent this, we introduce a
> | > 'struct upid' which associates a single pid_t value with a single pid
> | > namespace.
> | > 
> | > We then replace the pid->nr field in 'struct pid' with a list of struct upid'
> | > entries (referred to as 'pid->upid_list'). This list represents the multiple
> | > pid_t values of the process, one in each namespace. The current patch adds
> | > just one element to this list, corresponding to 'init_pid_ns'. Subsequent
> | > patches implement multiple pid namespaces and add more elements to the list.
> | > 
> | > The 'struct upid' also replaces 'struct pid' in the pid_hash table to enable us
> | > to find processes given a pid_t from any namespace (i.e we find 'struct upid'
> | > for a given pid_t and from the 'struct upid', we find the 'struct pid' of the
> | > process)
> | > 
> | > We finally reimplement find_pid() and pid_to_nr() to use pid->upid_list
> | > and remove unused fields from 'struct pid'.
> | > 
> | > Changelog:
> | > 	2.6.21-mm2-pidns3:
> | > 
> | > 	- 'struct upid' used to be called 'struct pid_nr' and a list of these
> | > 	   were hanging off of 'struct pid'. So, we renamed 'struct pid_nr'
> | > 	   and now hold them in a statically sized array in 'struct pid' since
> | > 	   the number of 'struct upid's for a process is known at process-
> | > 	   creation time.
> | > 	  
> | > 	2.6.21-rc3-mm2:
> | > 
> | > 	- [Eric Biederman] Combine all logical changes into one patch
> | > 	- [Eric Biederman] Implement __pid_nr(pid_ns, pid) for use in procfs.
> | > 	  (now called pid_to_nr_in_ns()).
> | > 	- [Serge Hallyn]: Remove (!pid_nr) check in free_pid_nr()
> | > 
> | > Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
> | > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> | > ---
> | >  fs/proc/array.c               |   30 ++++++--
> | >  fs/proc/base.c                |    9 ++
> | >  include/linux/init_task.h     |   14 +++-
> | >  include/linux/pid.h           |   62 ++++++++++++++---
> | >  include/linux/pid_namespace.h |   15 ++++
> | >  kernel/fork.c                 |    2 
> | >  kernel/pid.c                  |  145 +++++++++++++++++++++++++++++++++---------
> | >  7 files changed, 220 insertions(+), 57 deletions(-)
> | > 
> | > Index: lx26-22-rc4-mm2/include/linux/pid.h
> | > ===================================================================
> | > --- lx26-22-rc4-mm2.orig/include/linux/pid.h	2007-06-15 18:44:50.000000000 -0700
> | > +++ lx26-22-rc4-mm2/include/linux/pid.h	2007-06-15 19:47:58.000000000 -0700
> | > @@ -16,6 +16,25 @@ enum pid_type
> | >  	PIDTYPE_MAX
> | >  };
> | >  
> | > +struct pid_namespace;
> | > +
> | > +/*
> | > + * A struct upid holds a process identifier (or pid->nr) for a given
> | > + * pid namespace.
> | > + *
> | > + * A list of 'struct upid' entries is stored in the struct pid. This list
> | > + * is used to get the process identifier associated with the pid
> | > + * namespace it is being seen from.
> | > + */
> | > +struct upid
> | > +{
> | > +	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
> | > +	struct hlist_node pid_chain; 	/* link hash collisions on pid_hash */
> | > +	int nr;	 			/* user space pid number */
> | > +	struct pid_namespace *pid_ns; 	/* pid namespace in which nr is valid */
> | > +	struct pid *pid; 		/* back to task's unique kernel pid */
> | > +};
> | > +
> | >  /*
> | >   * What is struct pid?
> | >   *
> | > @@ -48,12 +67,11 @@ enum pid_type
> | >  struct pid
> | >  {
> | >  	atomic_t count;
> | > -	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
> | > -	int nr;
> | > -	struct hlist_node pid_chain;
> | >  	/* lists of tasks that use this pid */
> | >  	struct hlist_head tasks[PIDTYPE_MAX];
> | >  	struct rcu_head rcu;
> | > +	int num_upids;
> | > +	struct upid upid_list[1];
> | 
> | Further in your patches you define MAX_NESTED_PID_NS. What for, you
> | use the linked list here!?
> 
> Hmm. I don't understand. upid_list[] is an array (and not a linked
> list). Are you saying the '_list' in 'upid_list' is misleading ?

Oh, I see! You allocate all the upids in one chunk. I have missed
that, sorry :)

> Placing a limit like MAX_NESTED_PID_NS simplifies allocation of
> 'struct pid'.

How? If we have, say, 100-level namespace than we have to create
the sizeof(struct pid) + 100 * sizeof(struct upid) bytes.

> 
> | 
> | >  };
> | >  
> | >  extern struct pid init_struct_pid;
> | 
> | [snip]
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
> 
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers

sukadev@us.ibm.com wrote:
> Pavel Emelianov [xemul@openvz.org] wrote:
> | sukadev@us.ibm.com wrote:
> | > Pavel Emelianov [xemul@openvz.org] wrote:
> | > | sukadev@us.ibm.com wrote:
> | > | > Subject: [PATCH 08/17] Pid-NS(V3) Define/use pid->upid_list list.
> | > | > 
> | > | > From: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> | > | > 
> | > | > 
> | > | > With multiple pid namespaces, a process would be known by several pid_t
> | > | > values, one in each pid namespace. To represent this, we introduce a
> | > | > 'struct upid' which associates a single pid_t value with a single pid
> | > | > namespace.
> | > | > 
> | > | > We then replace the pid->nr field in 'struct pid' with a list of struct upid'
> | > | > entries (referred to as 'pid->upid_list'). This list represents the multiple
> | > | > pid_t values of the process, one in each namespace. The current patch adds
> | > | > just one element to this list, corresponding to 'init_pid_ns'. Subsequent
> | > | > patches implement multiple pid namespaces and add more elements to the list.
> | > | > 
> | > | > The 'struct upid' also replaces 'struct pid' in the pid_hash table to enable us
> | > | > to find processes given a pid_t from any namespace (i.e we find 'struct upid'
> | > | > for a given pid_t and from the 'struct upid', we find the 'struct pid' of the
> | > | > process)
> | > | > 
> | > | > We finally reimplement find_pid() and pid_to_nr() to use pid->upid_list
> | > | > and remove unused fields from 'struct pid'.
> | > | > 
> | > | > Changelog:
> | > | > 	2.6.21-mm2-pidns3:
> | > | > 
> | > | > 	- 'struct upid' used to be called 'struct pid_nr' and a list of these
> | > | > 	   were hanging off of 'struct pid'. So, we renamed 'struct pid_nr'
> | > | > 	   and now hold them in a statically sized array in 'struct pid' since
> | > | > 	   the number of 'struct upid's for a process is known at process-
> | > | > 	   creation time.
> | > | > 	  
> | > | > 	2.6.21-rc3-mm2:
> | > | > 
> | > | > 	- [Eric Biederman] Combine all logical changes into one patch
> | > | > 	- [Eric Biederman] Implement __pid_nr(pid_ns, pid) for use in procfs.
> | > | > 	  (now called pid_to_nr_in_ns()).
> | > | > 	- [Serge Hallyn]: Remove (!pid_nr) check in free_pid_nr()
> | > | > 
> | > | > Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
> | > | > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com>
> | > | > ---
> | > | >  fs/proc/array.c               |   30 ++++++--
> | > | >  fs/proc/base.c                |    9 ++
> | > | >  include/linux/init_task.h     |   14 +++-
> | > | >  include/linux/pid.h           |   62 ++++++++++++++---
> | > | >  include/linux/pid_namespace.h |   15 ++++
> | > | >  kernel/fork.c                 |    2 
> | > | >  kernel/pid.c                  |  145 +++++++++++++++++++++++++++++++++---------
> | > | >  7 files changed, 220 insertions(+), 57 deletions(-)
> | > | > 
> | > | > Index: lx26-22-rc4-mm2/include/linux/pid.h
> | > | > ===================================================================
> | > | > --- lx26-22-rc4-mm2.orig/include/linux/pid.h	2007-06-15 18:44:50.000000000 -0700
> | > | > +++ lx26-22-rc4-mm2/include/linux/pid.h	2007-06-15 19:47:58.000000000 -0700
> | > | > @@ -16,6 +16,25 @@ enum pid_type
> | > | >  	PIDTYPE_MAX
> | > | >  };
> | > | >  
> | > | > +struct pid_namespace;
> | > | > +
> | > | > +/*
> | > | > + * A struct upid holds a process identifier (or pid->nr) for a given
> | > | > + * pid namespace.
> | > | > + *
> | > | > + * A list of 'struct upid' entries is stored in the struct pid. This list
> | > | > + * is used to get the process identifier associated with the pid
> | > | > + * namespace it is being seen from.
> | > | > + */
> | > | > +struct upid
> | > | > +{
> | > | > +	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
> | > | > +	struct hlist_node pid_chain; 	/* link hash collisions on pid_hash */
> | > | > +	int nr;	 			/* user space pid number */
> | > | > +	struct pid_namespace *pid_ns; 	/* pid namespace in which nr is valid */
> | > | > +	struct pid *pid; 		/* back to task's unique kernel pid */
> | > | > +};
> | > | > +
> | > | >  /*
> | > | >   * What is struct pid?
> | > | >   *
> | > | > @@ -48,12 +67,11 @@ enum pid_type
> | > | >  struct pid
> | > | >  {
> | > | >  	atomic_t count;
> | > | > -	/* Try to keep pid_chain in the same cacheline as nr for find_pid */
> | > | > -	int nr;
> | > | > -	struct hlist_node pid_chain;
> | > | >  	/* lists of tasks that use this pid */
> | > | >  	struct hlist_head tasks[PIDTYPE_MAX];
> | > | >  	struct rcu_head rcu;
> | > | > +	int num_upids;
> | > | > +	struct upid upid_list[1];
> | > | 
> | > | Further in your patches you define MAX_NESTED_PID_NS. What for, you
> | > | use the linked list here!?
> | > 
> | > Hmm. I don't understand. upid_list[] is an array (and not a linked
> | > list). Are you saying the '_list' in 'upid_list' is misleading ?
> | 
> | Oh, I see! You allocate all the upids in one chunk. I have missed
> | that, sorry :)
> | 
> | > Placing a limit like MAX_NESTED_PID_NS simplifies allocation of
> | > 'struct pid'.
> | 
> | How? If we have, say, 100-level namespace than we have to create
> | the sizeof(struct pid) + 100 * sizeof(struct upid) bytes.
> 
> I should have been a little more clear.
> 
> I was comparing this with my previous version which did not have the
> MAX_NESTED_PID_NS limit and allowed for arbitrary levels of nesting
> (100 or even 1000 :-). Allocating that kind of 'struct pid' is more
> complex and looks like an overkill at this time.
> 
> With a limit like MAX_NESTED_PID_NS, we could in theory create that
> many pid caches, one for each level of nesting and use the appropriate
> cache in clone().

Oh! I see. Thanks. Although this looks a bit ... weird.

> | 
> | > 
> | > | 
> | > | >  };
> | > | >  
> | > | >  extern struct pid init_struct_pid;
> | > | 
> | > | [snip]
> | > _______________________________________________
> | > Containers mailing list
> | > Containers@lists.linux-foundation.org
> | > https://lists.linux-foundation.org/mailman/listinfo/containers
> | > 
> | > _______________________________________________
> | > Devel mailing list
> | > Devel@openvz.org
> | > https://openvz.org/mailman/listinfo/devel
> | > 
> 

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers