[PATCH -tip 0/2] sched: Get group names back in /proc/sched_debug - v1

[PATCH -tip 0/2] sched: Get group names back in /proc/sched_debug - v1

[Bharata B Rao]

Hi,

This patchset adds group names back to /proc/sched_debug. This v1 post
addresses the comments received during initial post (https://lkml.org/lkml/2011/1/10/2)

Mike, you may want to add your sign off since I took pieces of your code
from your review comments.

Regards,
Bharata.

[PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Bharata B Rao]

sched: Reinstate group names in /proc/sched_debug.

Displaying of group names in /proc/sched_debug was dropped in autogroup
patches. Add group names while displaying cfs_rq and tasks information.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -16,6 +16,8 @@
 #include <linux/kallsyms.h>
 #include <linux/utsname.h>
 
+static DEFINE_SPINLOCK(sched_debug_lock);
+
 /*
  * This allows printing both to /proc/sched_debug and
  * to the console
@@ -86,6 +88,23 @@ static void print_cfs_group_stats(struct
 }
 #endif
 
+#ifdef CONFIG_CGROUP_SCHED
+static char group_path[PATH_MAX];
+
+static char * task_group_path(struct task_group *tg)
+{
+	/*
+	 * May be NULL if the underlying cgroup isn't fully-created yet
+	 */
+	if (!tg->css.cgroup) {
+		group_path[0] = '\0';
+		return group_path;
+	}
+	cgroup_path(tg->css.cgroup, group_path, PATH_MAX);
+	return group_path;
+}
+#endif
+
 static void
 print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
 {
@@ -108,6 +127,9 @@ print_task(struct seq_file *m, struct rq
 	SEQ_printf(m, "%15Ld %15Ld %15Ld.%06ld %15Ld.%06ld %15Ld.%06ld",
 		0LL, 0LL, 0LL, 0L, 0LL, 0L, 0LL, 0L);
 #endif
+#ifdef CONFIG_CGROUP_SCHED
+	SEQ_printf(m, " %s", task_group_path(task_group(p)));
+#endif
 
 	SEQ_printf(m, "\n");
 }
@@ -144,7 +166,11 @@ void print_cfs_rq(struct seq_file *m, in
 	struct sched_entity *last;
 	unsigned long flags;
 
+#ifdef CONFIG_FAIR_GROUP_SCHED
+	SEQ_printf(m, "\ncfs_rq[%d]:%s\n", cpu, task_group_path(cfs_rq->tg));
+#else
 	SEQ_printf(m, "\ncfs_rq[%d]:\n", cpu);
+#endif
 	SEQ_printf(m, "  .%-30s: %Ld.%06ld\n", "exec_clock",
 			SPLIT_NS(cfs_rq->exec_clock));
 
@@ -191,7 +217,11 @@ void print_cfs_rq(struct seq_file *m, in
 
 void print_rt_rq(struct seq_file *m, int cpu, struct rt_rq *rt_rq)
 {
+#ifdef CONFIG_RT_GROUP_SCHED
+	SEQ_printf(m, "\nrt_rq[%d]:%s\n", cpu, task_group_path(rt_rq->tg));
+#else
 	SEQ_printf(m, "\nrt_rq[%d]:\n", cpu);
+#endif
 
 #define P(x) \
 	SEQ_printf(m, "  .%-30s: %Ld\n", #x, (long long)(rt_rq->x))
@@ -212,6 +242,7 @@ extern __read_mostly int sched_clock_run
 static void print_cpu(struct seq_file *m, int cpu)
 {
 	struct rq *rq = cpu_rq(cpu);
+	unsigned long flags;
 
 #ifdef CONFIG_X86
 	{
@@ -266,10 +297,14 @@ static void print_cpu(struct seq_file *m
 
 #undef P
 #endif
+	spin_lock_irqsave(&sched_debug_lock, flags);
 	print_cfs_stats(m, cpu);
 	print_rt_stats(m, cpu);
 
+	rcu_read_lock();
 	print_rq(m, rq, cpu);
+	rcu_read_unlock();
+	spin_unlock_irqrestore(&sched_debug_lock, flags);
 }
 
 static const char *sched_tunable_scaling_names[] = {

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Yong Zhang]

On Tue, Jan 11, 2011 at 03:41:54PM +0530, Bharata B Rao wrote:
> sched: Reinstate group names in /proc/sched_debug.
> 
> Displaying of group names in /proc/sched_debug was dropped in autogroup
> patches. Add group names while displaying cfs_rq and tasks information.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> --- a/kernel/sched_debug.c
> +++ b/kernel/sched_debug.c
> @@ -16,6 +16,8 @@
>  #include <linux/kallsyms.h>
>  #include <linux/utsname.h>
>  
> +static DEFINE_SPINLOCK(sched_debug_lock);

I don't get your point on introducing this lock.
Just to avoid concurrent access?
Or could you explain please?

Thanks,
Yong

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Bharata B Rao]

On Tue, Jan 11, 2011 at 08:46:58PM +0800, Yong Zhang wrote:
> On Tue, Jan 11, 2011 at 03:41:54PM +0530, Bharata B Rao wrote:
> > sched: Reinstate group names in /proc/sched_debug.
> > 
> > Displaying of group names in /proc/sched_debug was dropped in autogroup
> > patches. Add group names while displaying cfs_rq and tasks information.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> >  kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > --- a/kernel/sched_debug.c
> > +++ b/kernel/sched_debug.c
> > @@ -16,6 +16,8 @@
> >  #include <linux/kallsyms.h>
> >  #include <linux/utsname.h>
> >  
> > +static DEFINE_SPINLOCK(sched_debug_lock);
> 
> I don't get your point on introducing this lock.
> Just to avoid concurrent access?

Yes.

> Or could you explain please?

The group name is generated in a global buffer which is protected by
this lock. Earlier small sized local buffer (on stack) was used for
this purpose which wasn't ideal. Also since we can be here from
interrupt context, I wanted to avoid allocation too. Hence went for
a global buffer protected by a lock.

Regards,
Bharata.

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Yong Zhang]

On Tue, Jan 11, 2011 at 07:12:44PM +0530, Bharata B Rao wrote:
> The group name is generated in a global buffer which is protected by
> this lock. Earlier small sized local buffer (on stack) was used for
> this purpose which wasn't ideal. Also since we can be here from
> interrupt context, I wanted to avoid allocation too. Hence went for
> a global buffer protected by a lock.

Sounds good ;)

BTW, I guess we can also remove rcu_read_lock/unlock() in print_rt_stats()
and print_cfs_stats() with sched_debug_lock hold.

Thanks,
Yong

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Bharata B Rao]

On Tue, Jan 11, 2011 at 10:00:05PM +0800, Yong Zhang wrote:
> On Tue, Jan 11, 2011 at 07:12:44PM +0530, Bharata B Rao wrote:
> > The group name is generated in a global buffer which is protected by
> > this lock. Earlier small sized local buffer (on stack) was used for
> > this purpose which wasn't ideal. Also since we can be here from
> > interrupt context, I wanted to avoid allocation too. Hence went for
> > a global buffer protected by a lock.
> 
> Sounds good ;)
> 
> BTW, I guess we can also remove rcu_read_lock/unlock() in print_rt_stats()
> and print_cfs_stats() with sched_debug_lock hold.

We could, but I guess its not a recommended practice anymore to depend on
spinlocks to protect rcu readside.

Regards,
Bharata.

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Yong Zhang]

On Wed, Jan 12, 2011 at 3:49 PM, Bharata B Rao
<bharata@linux.vnet.ibm.com> wrote:
>> BTW, I guess we can also remove rcu_read_lock/unlock() in print_rt_stats()
>> and print_cfs_stats() with sched_debug_lock hold.
>
> We could, but I guess its not a recommended practice anymore to depend on
> spinlocks to protect rcu readside.

OK. Then let's keep it.

Thanks,
Yong

-- 
Only stand for myself

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Peter Zijlstra]

On Wed, 2011-01-12 at 13:19 +0530, Bharata B Rao wrote:
> 
> We could, but I guess its not a recommended practice anymore to depend on
> spinlocks to protect rcu readside.

Not only is it not recommended, its a flat out bug for PREEMPT_RCU.

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Paul E. McKenney]

On Wed, Jan 12, 2011 at 12:18:32PM +0100, Peter Zijlstra wrote:
> On Wed, 2011-01-12 at 13:19 +0530, Bharata B Rao wrote:
> > 
> > We could, but I guess its not a recommended practice anymore to depend on
> > spinlocks to protect rcu readside.
> 
> Not only is it not recommended, its a flat out bug for PREEMPT_RCU.

And lockdep-RCU will yell at you under PREEMPT_RCU, which will result
in Miles Lane yelling at me.  ;-)

							Thanx, Paul

Re: [PATCH -tip 1/2] [PATCH -tip 1/2] sched: Reinstate group names in /proc/sched_debug.

[Yong Zhang]

On Thu, Jan 13, 2011 at 7:16 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Jan 12, 2011 at 12:18:32PM +0100, Peter Zijlstra wrote:
>> Not only is it not recommended, its a flat out bug for PREEMPT_RCU.
>
> And lockdep-RCU will yell at you under PREEMPT_RCU, which will result
> in Miles Lane yelling at me.  ;-)


Thanks Peter and Paul for pointing this.
I will take a look at PREEMPT_RCU ;)

Thanks,
Yong

-- 
Only stand for myself

[tip:sched/urgent] sched: Reinstate group names in /proc/sched_debug

[tip-bot for Bharata B Rao]

Commit-ID:  efe25c2c7b3a5d17b0c70987a758d8fe7af8e3d1
Gitweb:     http://git.kernel.org/tip/efe25c2c7b3a5d17b0c70987a758d8fe7af8e3d1
Author:     Bharata B Rao <bharata@linux.vnet.ibm.com>
AuthorDate: Tue, 11 Jan 2011 15:41:54 +0530
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 18 Jan 2011 15:09:39 +0100

sched: Reinstate group names in /proc/sched_debug

Displaying of group names in /proc/sched_debug was dropped in autogroup
patches. Add group names while displaying cfs_rq and tasks information.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110111101153.GE4772@in.ibm.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
index 1dfae3d..4d36f37 100644
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -16,6 +16,8 @@
 #include <linux/kallsyms.h>
 #include <linux/utsname.h>
 
+static DEFINE_SPINLOCK(sched_debug_lock);
+
 /*
  * This allows printing both to /proc/sched_debug and
  * to the console
@@ -86,6 +88,23 @@ static void print_cfs_group_stats(struct seq_file *m, int cpu, struct task_group
 }
 #endif
 
+#ifdef CONFIG_CGROUP_SCHED
+static char group_path[PATH_MAX];
+
+static char *task_group_path(struct task_group *tg)
+{
+	/*
+	 * May be NULL if the underlying cgroup isn't fully-created yet
+	 */
+	if (!tg->css.cgroup) {
+		group_path[0] = '\0';
+		return group_path;
+	}
+	cgroup_path(tg->css.cgroup, group_path, PATH_MAX);
+	return group_path;
+}
+#endif
+
 static void
 print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
 {
@@ -108,6 +127,9 @@ print_task(struct seq_file *m, struct rq *rq, struct task_struct *p)
 	SEQ_printf(m, "%15Ld %15Ld %15Ld.%06ld %15Ld.%06ld %15Ld.%06ld",
 		0LL, 0LL, 0LL, 0L, 0LL, 0L, 0LL, 0L);
 #endif
+#ifdef CONFIG_CGROUP_SCHED
+	SEQ_printf(m, " %s", task_group_path(task_group(p)));
+#endif
 
 	SEQ_printf(m, "\n");
 }
@@ -144,7 +166,11 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
 	struct sched_entity *last;
 	unsigned long flags;
 
+#ifdef CONFIG_FAIR_GROUP_SCHED
+	SEQ_printf(m, "\ncfs_rq[%d]:%s\n", cpu, task_group_path(cfs_rq->tg));
+#else
 	SEQ_printf(m, "\ncfs_rq[%d]:\n", cpu);
+#endif
 	SEQ_printf(m, "  .%-30s: %Ld.%06ld\n", "exec_clock",
 			SPLIT_NS(cfs_rq->exec_clock));
 
@@ -191,7 +217,11 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct cfs_rq *cfs_rq)
 
 void print_rt_rq(struct seq_file *m, int cpu, struct rt_rq *rt_rq)
 {
+#ifdef CONFIG_RT_GROUP_SCHED
+	SEQ_printf(m, "\nrt_rq[%d]:%s\n", cpu, task_group_path(rt_rq->tg));
+#else
 	SEQ_printf(m, "\nrt_rq[%d]:\n", cpu);
+#endif
 
 #define P(x) \
 	SEQ_printf(m, "  .%-30s: %Ld\n", #x, (long long)(rt_rq->x))
@@ -212,6 +242,7 @@ extern __read_mostly int sched_clock_running;
 static void print_cpu(struct seq_file *m, int cpu)
 {
 	struct rq *rq = cpu_rq(cpu);
+	unsigned long flags;
 
 #ifdef CONFIG_X86
 	{
@@ -266,10 +297,14 @@ static void print_cpu(struct seq_file *m, int cpu)
 
 #undef P
 #endif
+	spin_lock_irqsave(&sched_debug_lock, flags);
 	print_cfs_stats(m, cpu);
 	print_rt_stats(m, cpu);
 
+	rcu_read_lock();
 	print_rq(m, rq, cpu);
+	rcu_read_unlock();
+	spin_unlock_irqrestore(&sched_debug_lock, flags);
 }
 
 static const char *sched_tunable_scaling_names[] = {

[PATCH -tip 2/2] sched: Display autogroup names in /proc/sched_debug.

[Bharata B Rao]

sched: Display autogroup names in /proc/sched_debug.

Add autogroup name to cfs_rq and tasks information in /proc/sched_debug.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 kernel/sched_autogroup.c |    5 +++++
 kernel/sched_debug.c     |    3 +++
 2 files changed, 8 insertions(+)

--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -231,6 +231,11 @@ void proc_sched_autogroup_show_task(stru
 #ifdef CONFIG_SCHED_DEBUG
 static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
 {
+	int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
+
+	if (!enabled || !tg->autogroup)
+	       return 0;
+
 	return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
 }
 #endif /* CONFIG_SCHED_DEBUG */
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -93,6 +93,9 @@ static char group_path[PATH_MAX];
 
 static char * task_group_path(struct task_group *tg)
 {
+	if (autogroup_path(tg, group_path, PATH_MAX))
+		return group_path;
+
 	/*
 	 * May be NULL if the underlying cgroup isn't fully-created yet
 	 */

[tip:sched/urgent] sched: Display autogroup names in /proc/sched_debug

[tip-bot for Bharata B Rao]

Commit-ID:  8ecedd7a06d27a31dbb36fab88e2ba6e6edd43ca
Gitweb:     http://git.kernel.org/tip/8ecedd7a06d27a31dbb36fab88e2ba6e6edd43ca
Author:     Bharata B Rao <bharata@linux.vnet.ibm.com>
AuthorDate: Tue, 11 Jan 2011 15:42:57 +0530
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 18 Jan 2011 15:09:40 +0100

sched: Display autogroup names in /proc/sched_debug

Add autogroup name to cfs_rq and tasks information to /proc/sched_debug.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20110111101257.GF4772@in.ibm.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_autogroup.c |    5 +++++
 kernel/sched_debug.c     |    3 +++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/kernel/sched_autogroup.c b/kernel/sched_autogroup.c
index 32a723b..938d52f 100644
--- a/kernel/sched_autogroup.c
+++ b/kernel/sched_autogroup.c
@@ -231,6 +231,11 @@ void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m)
 #ifdef CONFIG_SCHED_DEBUG
 static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
 {
+	int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
+
+	if (!enabled || !tg->autogroup)
+		return 0;
+
 	return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
 }
 #endif /* CONFIG_SCHED_DEBUG */
diff --git a/kernel/sched_debug.c b/kernel/sched_debug.c
index 4d36f37..e4d3725 100644
--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -93,6 +93,9 @@ static char group_path[PATH_MAX];
 
 static char *task_group_path(struct task_group *tg)
 {
+	if (autogroup_path(tg, group_path, PATH_MAX))
+		return group_path;
+
 	/*
 	 * May be NULL if the underlying cgroup isn't fully-created yet
 	 */

Re: [PATCH -tip 0/2] sched: Get group names back in /proc/sched_debug - v1

[Mike Galbraith]

On Tue, 2011-01-11 at 15:40 +0530, Bharata B Rao wrote:
> Hi,
> 
> This patchset adds group names back to /proc/sched_debug. This v1 post
> addresses the comments received during initial post (https://lkml.org/lkml/2011/1/10/2)
> 
> Mike, you may want to add your sign off since I took pieces of your code
> from your review comments.

It's fine by me with or without.

	-Mike

[PATCH -tip 0/2] sched: Get group names back in /proc/sched_debug

[Bharata B Rao]

Hi,

With autogroup patches, the group names got dropped off from
/proc/sched_debug. Without group name info, it is difficult to figure out
which cfs_rq or task belongs to which group in /proc/sched_debug output.

This patchset brings back the group name and also adds autogroup names
to /proc/sched_debug output.

Before this patchset
====================
# grep cfs_rq /proc/sched_debug 
cfs_rq[0]:
cfs_rq[0]:
cfs_rq[1]:
cfs_rq[1]:
cfs_rq[2]:
cfs_rq[2]:
cfs_rq[3]:
cfs_rq[3]:
cfs_rq[4]:
cfs_rq[5]:
cfs_rq[6]:
cfs_rq[7]:
cfs_rq[8]:
cfs_rq[9]:
cfs_rq[10]:
cfs_rq[11]:
cfs_rq[12]:
cfs_rq[13]:
cfs_rq[13]:
cfs_rq[14]:
cfs_rq[14]:
cfs_rq[15]:
cfs_rq[15]:

# grep while1 /proc/sched_debug 
R         while1  2203     48420.920338      5007   120     48420.920338     48951.419166         7.485277
R         while1  2226     23823.872648      2865   120     23823.872648     28347.561387         3.988024
R         while1  2204     43006.490234      4851   120     43006.490234     48459.814725         0.000000

With this patchset
==================
# grep cfs_rq /proc/sched_debug 
cfs_rq[0]:/autogroup-52
cfs_rq[0]:/autogroup-0
cfs_rq[1]:/autogroup-54
cfs_rq[1]:/autogroup-0
cfs_rq[2]:/autogroup-51
cfs_rq[2]:/autogroup-0
cfs_rq[3]:/autogroup-0
cfs_rq[4]:/autogroup-50
cfs_rq[4]:/autogroup-0
cfs_rq[5]:/autogroup-11
cfs_rq[5]:/autogroup-0
cfs_rq[6]:/autogroup-0
cfs_rq[7]:/autogroup-0
cfs_rq[8]:/autogroup-13
cfs_rq[8]:/autogroup-0
cfs_rq[9]:/autogroup-0
cfs_rq[10]:/autogroup-52
cfs_rq[10]:/autogroup-0
cfs_rq[11]:/autogroup-52
cfs_rq[11]:/autogroup-0
cfs_rq[12]:/autogroup-0
cfs_rq[13]:/autogroup-0
cfs_rq[14]:/1
cfs_rq[14]:/autogroup-0
cfs_rq[15]:/autogroup-0

# grep while1 /proc/sched_debug 
R         while1  2208     38199.296761      3826   120     38199.296761     38200.395907         4.341835 /autogroup-54
R         while1  2205     31251.518310      5629   120     31251.518310     54431.493631         6.882145 /autogroup-50
R         while1  2206     43920.889967      5402   120     43920.889967     53954.315334         0.000000 /1

Regards,
Bharata.

[PATCH -tip 2/2] sched: Display autogroup names in /proc/sched_debug.

[Bharata B Rao]

sched: Display autogroup names in /proc/sched_debug.

Add autogroup name to cfs_rq and tasks information in /proc/sched_debug.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 kernel/sched_debug.c |    8 ++++++++
 1 file changed, 8 insertions(+)

--- a/kernel/sched_debug.c
+++ b/kernel/sched_debug.c
@@ -92,6 +92,14 @@ static void print_cfs_group_stats(struct
 #ifdef CONFIG_CGROUP_SCHED
 static char * task_group_path(struct task_group *tg)
 {
+#ifdef CONFIG_SCHED_AUTOGROUP
+	int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
+
+	if (enabled && tg->autogroup) {
+		autogroup_path(tg, group_path, PATH_MAX);
+		return group_path;
+	}
+#endif
 	/*
 	 * May be NULL if the underlying cgroup isn't fully-created yet
 	 */

Re: [PATCH -tip 2/2] sched: Display autogroup names in /proc/sched_debug.

[Mike Galbraith]

On Mon, 2011-01-10 at 10:46 +0530, Bharata B Rao wrote:
> sched: Display autogroup names in /proc/sched_debug.
> 
> Add autogroup name to cfs_rq and tasks information in /proc/sched_debug.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>  kernel/sched_debug.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> --- a/kernel/sched_debug.c
> +++ b/kernel/sched_debug.c
> @@ -92,6 +92,14 @@ static void print_cfs_group_stats(struct
>  #ifdef CONFIG_CGROUP_SCHED
>  static char * task_group_path(struct task_group *tg)
>  {
> +#ifdef CONFIG_SCHED_AUTOGROUP
> +	int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
> +
> +	if (enabled && tg->autogroup) {
> +		autogroup_path(tg, group_path, PATH_MAX);
> +		return group_path;
> +	}
> +#endif
>  	/*
>  	 * May be NULL if the underlying cgroup isn't fully-created yet
>  	 */

What prevents the task being moved, and the task group being freed under
you?  Looks to me like task_group_path() needs rcu_read_lock().  It is
locked in the other two instances.  How about the below?

Date: Mon, 10 Jan 2011 10:46:43 +0530
From: Bharata B Rao <bharata@linux.vnet.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: Mike Galbraith <efault@gmx.de>, Peter Zijlstra <a.p.zijlstra@chello.nl>, Ingo Molnar <mingo@elte.hu>
Subject: [PATCH -tip 2/2] sched: Display autogroup names in /proc/sched_debug.

sched: Display autogroup names in /proc/sched_debug.

Add autogroup name to cfs_rq and tasks information in /proc/sched_debug.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 kernel/sched_autogroup.c |    5 +++++
 kernel/sched_debug.c     |    7 +++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

Index: linux-2.6/kernel/sched_debug.c
===================================================================
--- linux-2.6.orig/kernel/sched_debug.c
+++ linux-2.6/kernel/sched_debug.c
@@ -92,6 +92,9 @@ static void print_cfs_group_stats(struct
 #ifdef CONFIG_CGROUP_SCHED
 static char * task_group_path(struct task_group *tg)
 {
+	if (autogroup_path(tg, group_path, PATH_MAX))
+		return group_path;
+
 	/*
 	 * May be NULL if the underlying cgroup isn't fully-created yet
 	 */
@@ -99,9 +102,7 @@ static char * task_group_path(struct tas
 		group_path[0] = '\0';
 		return group_path;
 	}
-	rcu_read_lock();
 	cgroup_path(tg->css.cgroup, group_path, PATH_MAX);
-	rcu_read_unlock();
 	return group_path;
 }
 #endif
@@ -302,7 +303,9 @@ static void print_cpu(struct seq_file *m
 	print_cfs_stats(m, cpu);
 	print_rt_stats(m, cpu);
 
+	rcu_read_lock();
 	print_rq(m, rq, cpu);
+	rcu_read_unlock();
 	spin_unlock_irqrestore(&sched_debug_lock, flags);
 }
 
Index: linux-2.6/kernel/sched_autogroup.c
===================================================================
--- linux-2.6.orig/kernel/sched_autogroup.c
+++ linux-2.6/kernel/sched_autogroup.c
@@ -231,6 +231,11 @@ void proc_sched_autogroup_show_task(stru
 #ifdef CONFIG_SCHED_DEBUG
 static inline int autogroup_path(struct task_group *tg, char *buf, int buflen)
 {
+	int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
+
+	if (!enabled || tg == &root_task_group)
+		return 0;
+
 	return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id);
 }
 #endif /* CONFIG_SCHED_DEBUG */

Re: [PATCH -tip 2/2] sched: Display autogroup names in /proc/sched_debug.

[Bharata B Rao]

On Mon, Jan 10, 2011 at 10:38:07AM +0100, Mike Galbraith wrote:
> On Mon, 2011-01-10 at 10:46 +0530, Bharata B Rao wrote:
> > sched: Display autogroup names in /proc/sched_debug.
> > 
> > Add autogroup name to cfs_rq and tasks information in /proc/sched_debug.
> > 
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> > ---
> >  kernel/sched_debug.c |    8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > --- a/kernel/sched_debug.c
> > +++ b/kernel/sched_debug.c
> > @@ -92,6 +92,14 @@ static void print_cfs_group_stats(struct
> >  #ifdef CONFIG_CGROUP_SCHED
> >  static char * task_group_path(struct task_group *tg)
> >  {
> > +#ifdef CONFIG_SCHED_AUTOGROUP
> > +	int enabled = ACCESS_ONCE(sysctl_sched_autogroup_enabled);
> > +
> > +	if (enabled && tg->autogroup) {
> > +		autogroup_path(tg, group_path, PATH_MAX);
> > +		return group_path;
> > +	}
> > +#endif
> >  	/*
> >  	 * May be NULL if the underlying cgroup isn't fully-created yet
> >  	 */
> 
> What prevents the task being moved, and the task group being freed under
> you?  Looks to me like task_group_path() needs rcu_read_lock().  It is
> locked in the other two instances.  How about the below?

You are right, we need rcu_read_lock() there. Let me spin out an other
version with the changes.

Thanks,
Bharata.