OpenVZ Forum - RDF feed
type: channel
items: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list
items: Re: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list

[RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list
type: item
description: From: Sukadev Bhattiprolu <sukadev@us.ibm.com> Subject: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list Stop using task->nsproxy->pid_ns. Use pid_namespace from pid->pid_nrs list instead. To simplify error handling, this patch moves processing of CLONE_NEWPID flag, currently in copy_namespaces()/copy_process(), to alloc_pid() which is where the process association with a pid namespace is established. i.e when cloning a new pid namespace, alloc_pid() allocates a new pid_nr for both the parent and child namespaces. Changelog: - Move definition of set_pid_ns_child_reaper() into the previous helper function patch. - Minor changes to accomodate changes in underlying patches. - Fix compile warnings about parent_pid_ns (Cedric Le Goater) - [Badari Pulavarty's comments]: No need to allocate nsproxy when only CLONE_NEWPID is set. And attach_pid_nr() only needs one parameter. - Add privilege check for clone(CLONE_NEWPID). Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> Cc: Cedric Le Goater <clg@fr.ibm.com> Cc: Dave Hansen <haveblue@us.ibm.com> Cc: Serge Hallyn <serue@us.ibm.com> Cc: containers@lists.osdl.org --- include/linux/pid.h | 2 - include/linux/pid_namespace.h | 11 ++------- kernel/fork.c | 12 ++++++--- kernel/nsproxy.c | 11 --------- kernel/pid.c | 51 ++++++++++++++++++++++++++++++------------ 5 files changed, 50 insertions(+), 37 deletions(-) Index: lx26-20-mm2b/include/linux/pid.h =================================================================== --- lx26-20-mm2b.orig/include/linux/pid.h 2007-03-09 19:00:11.000000000 -0800 +++ lx26-20-mm2b/include/linux/pid.h 2007-03-09 19:01:09.000000000 -0800 @@ -119,7 +119,7 @@ extern struct pid *find_ge_pid(int nr); extern int attach_pid_nr(struct pid *pid, struct pid_nr *pid_nr); extern void free_pid_nr(struct pid_nr *pid_nr); extern struct pid_nr *alloc_pid_nr(struct pid_namespace *pid_ns); -extern struct pid *alloc_pid(void); +extern struct pid *alloc_pid(int clone_flags); extern void FASTCALL(free_pid(st…
creator: Sukadev Bhattiprolu
date: 2007-03-10T04:02:29-00:00

Re: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list
type: item
description: sukadev@us.ibm.com writes: > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> > Subject: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list > > Stop using task->nsproxy->pid_ns. Use pid_namespace from pid->pid_nrs > list instead. > > To simplify error handling, this patch moves processing of CLONE_NEWPID > flag, currently in copy_namespaces()/copy_process(), to alloc_pid() which > is where the process association with a pid namespace is established. > > i.e when cloning a new pid namespace, alloc_pid() allocates a new pid_nr > for both the parent and child namespaces. This patch seems to do a bit much, it is hard to follow what changes you are making. It probably makes sense to modify things so alloc_pid can do everything it needs to. It looks like we can safely move alloc_pid into copy_process and just dig out the pid number and place it in nr if copy_process succeeds. Which should allow the special case for setting the child reaper to go away, because we can allocate the task_struct before allocating the struct pid. > Changelog: > - Move definition of set_pid_ns_child_reaper() into the previous > helper function patch. > - Minor changes to accomodate changes in underlying patches. > - Fix compile warnings about parent_pid_ns (Cedric Le Goater) > - [Badari Pulavarty's comments]: No need to allocate nsproxy when > only CLONE_NEWPID is set. And attach_pid_nr() only needs one > parameter. > - Add privilege check for clone(CLONE_NEWPID). > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > Cc: Cedric Le Goater <clg@fr.ibm.com> > Cc: Dave Hansen <haveblue@us.ibm.com> > Cc: Serge Hallyn <serue@us.ibm.com> > Cc: containers@lists.osdl.org > > --- > include/linux/pid.h | 2 - > include/linux/pid_namespace.h | 11 ++------- > kernel/fork.c | 12 ++++++--- > kernel/nsproxy.c | 11 --------- > kernel/pid.c | 51 ++++++++++++++++++++++++++++++------------ > 5 files changed, 50 insertions(+), 37 deletions(-) > > Index: lx26-20-mm2b/include/linux/pid.h > ==============…
creator: ebiederm
date: 2007-03-11T12:48:04-00:00

Re: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list
type: item
description: Quoting Eric W. Biederman (ebiederm@xmission.com): > sukadev@us.ibm.com writes: > > > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> > > Subject: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list > > > > Stop using task->nsproxy->pid_ns. Use pid_namespace from pid->pid_nrs > > list instead. > > > > To simplify error handling, this patch moves processing of CLONE_NEWPID > > flag, currently in copy_namespaces()/copy_process(), to alloc_pid() which > > is where the process association with a pid namespace is established. > > > > i.e when cloning a new pid namespace, alloc_pid() allocates a new pid_nr > > for both the parent and child namespaces. > > > This patch seems to do a bit much, it is hard to follow what changes you > are making. Is this a design comment, or do you mean you'd like to see it broken into two or more patches? > It probably makes sense to modify things so alloc_pid can do everything > it needs to. > > It looks like we can safely move alloc_pid into copy_process and > just dig out the pid number and place it in nr if copy_process succeeds. > > Which should allow the special case for setting the child reaper to go > away, because we can allocate the task_struct before allocating the struct > pid. That would be nice. That little reaper setting helper bugs me. > > Changelog: > > - Move definition of set_pid_ns_child_reaper() into the previous > > helper function patch. > > - Minor changes to accomodate changes in underlying patches. > > - Fix compile warnings about parent_pid_ns (Cedric Le Goater) > > - [Badari Pulavarty's comments]: No need to allocate nsproxy when > > only CLONE_NEWPID is set. And attach_pid_nr() only needs one > > parameter. > > - Add privilege check for clone(CLONE_NEWPID). > > > > Signed-off-by: Sukadev Bhattiprolu <sukadev@us.ibm.com> > > Cc: Cedric Le Goater <clg@fr.ibm.com> > > Cc: Dave Hansen <haveblue@us.ibm.com> > > Cc: Serge Hallyn <serue@us.ibm.com> > > Cc: containers@lists.osdl.org > > > > --- > > include/linux/p…
creator: serue
date: 2007-03-11T13:15:37-00:00

Re: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list
type: item
description: "Serge E. Hallyn" <serue@us.ibm.com> writes: > Quoting Eric W. Biederman (ebiederm@xmission.com): >> sukadev@us.ibm.com writes: >> >> > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> >> > Subject: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list >> > >> > Stop using task->nsproxy->pid_ns. Use pid_namespace from pid->pid_nrs >> > list instead. >> > >> > To simplify error handling, this patch moves processing of CLONE_NEWPID >> > flag, currently in copy_namespaces()/copy_process(), to alloc_pid() which >> > is where the process association with a pid namespace is established. >> > >> > i.e when cloning a new pid namespace, alloc_pid() allocates a new pid_nr >> > for both the parent and child namespaces. >> >> >> This patch seems to do a bit much, it is hard to follow what changes you >> are making. > > Is this a design comment, or do you mean you'd like to see it broken > into two or more patches? The latter. There is no reason for the changes to remove the use of nsproxy->pid_ns need to be all in one patch. >> It probably makes sense to modify things so alloc_pid can do everything >> it needs to. >> >> It looks like we can safely move alloc_pid into copy_process and >> just dig out the pid number and place it in nr if copy_process succeeds. >> >> Which should allow the special case for setting the child reaper to go >> away, because we can allocate the task_struct before allocating the struct >> pid. > > That would be nice. That little reaper setting helper bugs me. Which is why I suggested the reorganization.... >> > Index: lx26-20-mm2b/kernel/pid.c >> > =================================================================== >> > --- lx26-20-mm2b.orig/kernel/pid.c 2007-03-09 19:00:42.000000000 -0800 >> > +++ lx26-20-mm2b/kernel/pid.c 2007-03-09 19:01:09.000000000 -0800 >> > @@ -221,8 +221,13 @@ fastcall void free_pid(struct pid *pid) >> > hlist_del_rcu(&pid->pid_chain); >> > spin_unlock_irqrestore(&pidmap_lock, flags); >> > >> > - hlist_for_each_entry(pid_nr,…
creator: ebiederm
date: 2007-03-11T17:52:52-00:00

Re: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list
type: item
description: Eric W. Biederman [ebiederm@xmission.com] wrote: | "Serge E. Hallyn" <serue@us.ibm.com> writes: | | > Quoting Eric W. Biederman (ebiederm@xmission.com): | >> sukadev@us.ibm.com writes: | >> | >> > From: Sukadev Bhattiprolu <sukadev@us.ibm.com> | >> > Subject: [RFC][PATCH 3/5] Use pid namespace from struct pid_nrs list | >> > | >> > Stop using task->nsproxy->pid_ns. Use pid_namespace from pid->pid_nrs | >> > list instead. | >> > | >> > To simplify error handling, this patch moves processing of CLONE_NEWPID | >> > flag, currently in copy_namespaces()/copy_process(), to alloc_pid() which | >> > is where the process association with a pid namespace is established. | >> > | >> > i.e when cloning a new pid namespace, alloc_pid() allocates a new pid_nr | >> > for both the parent and child namespaces. | >> | >> | >> This patch seems to do a bit much, it is hard to follow what changes you | >> are making. | > | > Is this a design comment, or do you mean you'd like to see it broken | > into two or more patches? | | The latter. There is no reason for the changes to remove the use | of nsproxy->pid_ns need to be all in one patch. I will try to break this up into smaller patches. | | >> It probably makes sense to modify things so alloc_pid can do everything | >> it needs to. | >> | >> It looks like we can safely move alloc_pid into copy_process and | >> just dig out the pid number and place it in nr if copy_process succeeds. | >> | >> Which should allow the special case for setting the child reaper to go | >> away, because we can allocate the task_struct before allocating the struct | >> pid. | > | > That would be nice. That little reaper setting helper bugs me. Yes. I like that idea too. | | Which is why I suggested the reorganization.... | | | >> > Index: lx26-20-mm2b/kernel/pid.c | >> > =================================================================== | >> > --- lx26-20-mm2b.orig/kernel/pid.c 2007-03-09 19:00:42.000000000 -0800 | >> > +++ lx26-20-mm2b/kernel/pid.c 2007-03-0…
creator: Sukadev Bhattiprolu
date: 2007-03-12T21:35:57-00:00