[PATCH 2/3] Fix the 'current css_set' problem in debug subsystem

[PATCH 2/3] Fix the 'current css_set' problem in debug subsystem

[Liu Aleaxander]

>From 61c93b1a66e43d44820e9b8d22ef0bec80dfc814 Mon Sep 17 00:00:00 2001
From: Liu Aleaxander <Aleaxander@gmail.com>
Date: Mon, 2 Nov 2009 14:30:07 +0800
Subject: [PATCH 2/3] Fix the 'current css_set' problem in debug subsystem

In the current implementation, it always print the same information
no matter which group you are in. And this would be meaningless. So
we should make it print the information of the 'real' current one.

Well, I am not sure it's the current one, but I'm sure it's the first
one of that cgroup.

Signed-off-by: Liu Aleaxander <Aleaxander@gmail.com>
---
 include/linux/cgroup.h |    2 --
 kernel/cgroup.c        |   19 ++++++++++++-------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 4d63c5c..0008dee 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -389,8 +389,6 @@ int cgroup_add_files(struct cgroup *cgrp,
             const struct cftype cft[],
             int count);

-struct css_set* cgroup_first_css_set(const struct cgroup *cgrp);
-
 int cgroup_is_removed(const struct cgroup *cgrp);

 int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen);
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 863089f..ddd4d33 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -340,7 +340,7 @@ static inline void put_css_set_taskexit(struct css_set *cg)
 /*
  * Get the first css_set of the cgroup 'cgrp'.
  */
-struct css_set* cgroup_first_css_set(const struct cgroup *cgrp)
+static struct css_set* cgroup_first_css_set(const struct cgroup *cgrp)
 {
        struct list_head *head = &cgrp->css_sets;
        struct cg_cgroup_link *link;
@@ -4093,17 +4093,22 @@ static u64 debug_taskcount_read(struct cgroup
*cont, struct cftype *cft)

 static u64 current_css_set_read(struct cgroup *cont, struct cftype *cft)
 {
-    return (u64)(unsigned long)current->cgroups;
+    return (u64)(long)cgroup_first_css_set(cont);
 }

 static u64 current_css_set_refcount_read(struct cgroup *cont,
                        struct cftype *cft)
 {
-    u64 count;
-
-    rcu_read_lock();
-    count = atomic_read(&current->cgroups->refcount);
-    rcu_read_unlock();
+    u64 count = 0;
+    struct css_set *cg;
+
+    cg = cgroup_first_css_set(cont);
+    if (cg) {
+        rcu_read_lock();
+        count = atomic_read(&cg->refcount);
+        rcu_read_unlock();
+    }
+
     return count;
 }

-- 
1.6.2.5



-- 
regards
Liu Aleaxander

Re: [PATCH 2/3] Fix the 'current css_set' problem in debug subsystem

[Li Zefan]

Liu Aleaxander wrote:
>>From 61c93b1a66e43d44820e9b8d22ef0bec80dfc814 Mon Sep 17 00:00:00 2001
> From: Liu Aleaxander <Aleaxander@gmail.com>
> Date: Mon, 2 Nov 2009 14:30:07 +0800
> Subject: [PATCH 2/3] Fix the 'current css_set' problem in debug subsystem
> 
> In the current implementation, it always print the same information
> no matter which group you are in. And this would be meaningless. So
> we should make it print the information of the 'real' current one.
> 
> Well, I am not sure it's the current one, but I'm sure it's the first
> one of that cgroup.
> 

I don't think there is a bug with current 'curent_css_set'. The
"current" in all debug.current_xxx interfaces means "current task".

Besides we already print out a cgroup's css links through
debug.cgroup_css_links, without showing the refcount of
each css_set, but we can, if it can be useful:

  # cat debug.cgroup_css_links
  css_set c10d92e0

> Signed-off-by: Liu Aleaxander <Aleaxander@gmail.com>
> ---
>  include/linux/cgroup.h |    2 --
>  kernel/cgroup.c        |   19 ++++++++++++-------
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 4d63c5c..0008dee 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -389,8 +389,6 @@ int cgroup_add_files(struct cgroup *cgrp,
>              const struct cftype cft[],
>              int count);
> 
> -struct css_set* cgroup_first_css_set(const struct cgroup *cgrp);
> -
>  int cgroup_is_removed(const struct cgroup *cgrp);
> 
>  int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen);
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 863089f..ddd4d33 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -340,7 +340,7 @@ static inline void put_css_set_taskexit(struct css_set *cg)
>  /*
>   * Get the first css_set of the cgroup 'cgrp'.
>   */
> -struct css_set* cgroup_first_css_set(const struct cgroup *cgrp)
> +static struct css_set* cgroup_first_css_set(const struct cgroup *cgrp)
>  {
>         struct list_head *head = &cgrp->css_sets;
>         struct cg_cgroup_link *link;
> @@ -4093,17 +4093,22 @@ static u64 debug_taskcount_read(struct cgroup
> *cont, struct cftype *cft)
> 
>  static u64 current_css_set_read(struct cgroup *cont, struct cftype *cft)
>  {
> -    return (u64)(unsigned long)current->cgroups;
> +    return (u64)(long)cgroup_first_css_set(cont);
>  }
> 
>  static u64 current_css_set_refcount_read(struct cgroup *cont,
>                         struct cftype *cft)
>  {
> -    u64 count;
> -
> -    rcu_read_lock();
> -    count = atomic_read(&current->cgroups->refcount);
> -    rcu_read_unlock();
> +    u64 count = 0;
> +    struct css_set *cg;
> +
> +    cg = cgroup_first_css_set(cont);
> +    if (cg) {
> +        rcu_read_lock();
> +        count = atomic_read(&cg->refcount);
> +        rcu_read_unlock();
> +    }
> +
>      return count;
>  }
> 

Re: [PATCH 2/3] Fix the 'current css_set' problem in debug subsystem

[Liu Aleaxander]

On Tue, Nov 3, 2009 at 9:05 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> Liu Aleaxander wrote:
>>>From 61c93b1a66e43d44820e9b8d22ef0bec80dfc814 Mon Sep 17 00:00:00 2001
>> From: Liu Aleaxander <Aleaxander@gmail.com>
>> Date: Mon, 2 Nov 2009 14:30:07 +0800
>> Subject: [PATCH 2/3] Fix the 'current css_set' problem in debug subsystem
>>
>> In the current implementation, it always print the same information
>> no matter which group you are in. And this would be meaningless. So
>> we should make it print the information of the 'real' current one.
>>
>> Well, I am not sure it's the current one, but I'm sure it's the first
>> one of that cgroup.
>>
>
> I don't think there is a bug with current 'curent_css_set'. The
> "current" in all debug.current_xxx interfaces means "current task".
Yes, I know it means 'current task', but is this useful to print the
same things with two more files in all the cgroups?

> Besides we already print out a cgroup's css links through
> debug.cgroup_css_links, without showing the refcount of
> each css_set, but we can, if it can be useful:
>
>  # cat debug.cgroup_css_links
>  css_set c10d92e0
Good link:)

-- 
regards
Liu Aleaxander

Re: [PATCH 2/3] Fix the 'current css_set' problem in debug subsystem

[Paul Menage]

On Mon, Nov 2, 2009 at 5:21 PM, Liu Aleaxander <aleaxander@gmail.com> wrote:
> Yes, I know it means 'current task', but is this useful to print the
> same things with two more files in all the cgroups?
>

We could just export those files in the root cgroup in the debug hierarchy.

Paul