[PATCH] cgroups: skip processes from other namespaces when listing a cgroup

[PATCH] cgroups: skip processes from other namespaces when listing a cgroup

[gowrishankar]

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>
Acked-by: Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

changes for vpid and pid_t
---
 kernel/cgroup.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 35eebd5..588d305 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2012,13 +2012,15 @@ int cgroup_scan_tasks(struct cgroup_scanner *scan)
 static int pid_array_load(pid_t *pidarray, int npids, struct cgroup *cgrp)
 {
     int n = 0;
+    pid_t vpid;
     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 ((vpid = task_pid_vnr(tsk)) > 0)
+            pidarray[n++] = vpid;
     }
     cgroup_iter_end(cgrp, &it);
     return n;
-- 
1.5.5.1

Re: [PATCH] cgroups: skip processes from other namespaces when listing a cgroup

[Balbir Singh]

* gowrishankar <gomuthuk-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> [2008-12-07 20:16:01]:

> 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>
> Acked-by: Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>
> changes for vpid and pid_t
> ---
> kernel/cgroup.c |    4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 35eebd5..588d305 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -2012,13 +2012,15 @@ int cgroup_scan_tasks(struct cgroup_scanner *scan)
> static int pid_array_load(pid_t *pidarray, int npids, struct cgroup *cgrp)
> {
>     int n = 0;
> +    pid_t vpid;

The formatting does not seem right, what is your tab space set to?

>     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 ((vpid = task_pid_vnr(tsk)) > 0)
> +            pidarray[n++] = vpid;
>     }
>     cgroup_iter_end(cgrp, &it);
>     return n;
> -- 
> 1.5.5.1
>
>

-- 
	Balbir

Re: [PATCH] cgroups: skip processes from other namespaces when listing a cgroup

[Dave Hansen]

On Mon, 2008-12-08 at 20:44 +0530, Balbir Singh wrote:
> * gowrishankar <gomuthuk-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> [2008-12-07 20:16:01]:
> >     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 ((vpid = task_pid_vnr(tsk)) > 0)
> > +            pidarray[n++] = vpid;
> >     }
> >     cgroup_iter_end(cgrp, &it);
> >     return n;

One more thing...

Please cc Eric and Suka on patches like this.  If we get many users like
this we'll probably want to add some common function.  Or if someone
missed using one, they can probably suggest one.

-- Dave

[PATCH] cgroups: skip processes from other namespaces when listing a cgroup

[gowrishankar]

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>
Acked-by: Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 kernel/cgroup.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 35eebd5..25fdd2c 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -2011,14 +2011,15 @@ int cgroup_scan_tasks(struct cgroup_scanner *scan)
  */
 static int pid_array_load(pid_t *pidarray, int npids, struct cgroup *cgrp)
 {
-    int n = 0;
+    int n = 0, pid;
     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 ((pid = task_pid_vnr(tsk)) > 0)
+            pidarray[n++] = pid;
     }
     cgroup_iter_end(cgrp, &it);
     return n;
-- 
1.5.5.1

Re: [PATCH] cgroups: skip processes from other namespaces when listing a cgroup

[Andrew Morton]

On Mon, 08 Dec 2008 12:52:16 +0530
gowrishankar <gomuthuk-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:

> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 35eebd5..25fdd2c 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -2011,14 +2011,15 @@ int cgroup_scan_tasks(struct cgroup_scanner *scan)
>   */
>  static int pid_array_load(pid_t *pidarray, int npids, struct cgroup *cgrp)
>  {
> -    int n = 0;
> +    int n = 0, pid;

Your email client is replacing tabs with spaces.

>      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 ((pid = task_pid_vnr(tsk)) > 0)
> +            pidarray[n++] = pid;

Please avoid the assignment-in-an-if shorthand.  We prefer to avoid
tricky C idioms like this keep the code super-simple:


	pid = task_pid_vnr(tsk);
	if (pid > 0)

scripts/checkpatch.pl can/should be used to detect this, and many
other little things.