[Devel] Re: [PATCH 5/13] Expand the pid/task seeking functions set

sukadev at us.ibm.com sukadev at us.ibm.com
Fri May 25 16:36:58 PDT 2007


Pavel Emelianov [xemul at 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 at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers




More information about the Devel mailing list