Re: [Devel] [PATCH] cgroups: not to iterate other namespace process inside container

[Balbir Singh <balbir-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>]

* Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> [2008-12-07 22:31:03]:

> On Sun, Dec 7, 2008 at 10:52 PM, gowrishankar
> <gomuthuk-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> > Once tasks are populated from system namespace inside cgroup, container
> > replaces
> > other namespace task with 0 while listing tasks, inside container.
> > Though this is expected behaviour
> > from container end, there is no use of showing unwanted 0s.
> > In below patch, we check if a process is in same namespace before
> > loading into pid array.
> >
> > Signed-off-by: Gowrishankar M <gowrishankar.m-xthvdsQ13ZrQT0dZR+AlfA@public.gmane.org>
> 
> Looks sensible, thanks.
> 
> Acked-by: Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> 
> Can you send it on to akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org?
> 
> A couple of small style issues:
> 
> - title would be better as "skip processes from other namespaces when
> listing a cgroup"
> 
> - use the local variable name "pid" rather than "ret"
> 

I would recommend using pid_t as type, since task_pid_vnr returns pid_t and vpid as the
variable name. Although this fixes pid_array_load(), I am worried that
cg_list would still have tasks from other namespaces that can affect
controllers. We'll need other changes to get the controllers working
correctly (a review of the code maybe?).

> > Index: linux-2.6.28-rc3/kernel/cgroup.c
> > ===================================================================
> > --- linux-2.6.28-rc3.orig/kernel/cgroup.c    2008-12-07
> > 13:23:19.000000000 -0500
> > +++ linux-2.6.28-rc3/kernel/cgroup.c    2008-12-07 13:24:28.000000000 -0500
> > @@ -2011,14 +2011,15 @@
> >  */
> >  static int pid_array_load(pid_t *pidarray, int npids, struct cgroup *cgrp)
> >  {
> > -    int n = 0;
> > +    int n = 0, ret;
> >     struct cgroup_iter it;
> >     struct task_struct *tsk;
> >     cgroup_iter_start(cgrp, &it);
> >     while ((tsk = cgroup_iter_next(cgrp, &it))) {
> >         if (unlikely(n == npids))
> >             break;
> > -        pidarray[n++] = task_pid_vnr(tsk);
> > +        if ((ret = task_pid_vnr(tsk)) > 0)
> > +            pidarray[n++] = ret;
> >     }
> >     cgroup_iter_end(cgrp, &it);
> >     return n;

-- 
	Balbir