[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