sched: Get group names back in /proc/sched_debug - v1

PATCH In review Bharata B Rao 5 participants 14 messages Jan 11, 2011 – Jan 18, 2011

lore

Share & tools

lore

Bharata B Rao

author

2011-01-11 10:10 UTC #0 lore raw

cover sched: Get group names back in /proc/sched_debug - v1

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.

Bharata B Rao

author

2011-01-11 10:12 UTC #1 lore raw

PATCH 1/2 sched: Reinstate group names in /proc/sched_debug.

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 <[email protected]>

---
 kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

kernel/sched_debug.c +35 -0

@@ -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[] = {

Yong Zhang

2011-01-11 12:47 UTC #2 lore raw

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

Quoted 13 lines · 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 <[email protected]>
---
 kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

kernel/sched_debug.c line 19

Quoted 4 lines

kernel/sched_debug.c +1 -0

@@ -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

Bharata B Rao

author

2011-01-11 13:42 UTC #3 lore raw

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

Quoted 17 lines · Yong Zhang

Quoted 16 lines · 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 <[email protected]>
---
 kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
kernel/sched_debug.c +0 -0
@@ -16,6 +16,8 @@
 #include <linux/kallsyms.h>
 #include <linux/utsname.h>
 

Quoted 4 lines

kernel/sched_debug.c line 19
Quoted 1 line
kernel/sched_debug.c +1 -0
+static DEFINE_SPINLOCK(sched_debug_lock);
I don't get your point on introducing this lock.
Just to avoid concurrent access?

Yes.

Quoted 1 line

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.

Yong Zhang

2011-01-11 14:00 UTC #4 lore raw

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

Quoted 5 lines · Bharata B Rao

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

Bharata B Rao

author

2011-01-12 07:49 UTC #5 lore raw

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

Quoted 11 lines · Yong Zhang

Quoted 5 lines · Bharata B Rao
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.

Yong Zhang

2011-01-12 09:17 UTC #6 lore raw

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

Quoted 5 lines · Bharata B Rao

Quoted 2 lines
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

Signature · 1 line

Only stand for myself

Peter Zijlstra

2011-01-12 11:18 UTC #7 lore raw

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

In reply to Bharata B Rao

Quoted 3 lines · Bharata B Rao

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.

Paul E. McKenney

2011-01-12 23:16 UTC #8 lore raw

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

Quoted 6 lines · Peter Zijlstra

Quoted 3 lines · Bharata B Rao
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

Yong Zhang

2011-01-13 01:24 UTC #9 lore raw

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

Quoted 5 lines · Paul E. McKenney

Quoted 1 line · Peter Zijlstra
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

Signature · 1 line

Only stand for myself

tip-bot for Bharata B Rao

author

2011-01-18 19:05 UTC #10 lore raw

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

In reply to Bharata B Rao

Commit-ID:  efe25c2c7b3a5d17b0c70987a758d8fe7af8e3d1
Gitweb:     http://git.kernel.org/tip/efe25c2c7b3a5d17b0c70987a758d8fe7af8e3d1
Author:     Bharata B Rao <[email protected]>
AuthorDate: Tue, 11 Jan 2011 15:41:54 +0530
Committer:  Ingo Molnar <[email protected]>
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 <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/sched_debug.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

kernel/sched_debug.c +35 -0

index 1dfae3d..4d36f37 100644
@@ -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[] = {

Bharata B Rao

author

2011-01-11 10:13 UTC #11 lore raw

PATCH 2/2 sched: Display autogroup names in /proc/sched_debug.

In reply to 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 <[email protected]>

---
 kernel/sched_autogroup.c |    5 +++++
 kernel/sched_debug.c     |    3 +++
 2 files changed, 8 insertions(+)

kernel/sched_autogroup.c +5 -0

@@ -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 */

kernel/sched_debug.c +3 -0

@@ -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-bot for Bharata B Rao

author

2011-01-18 19:05 UTC #12 lore raw

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

Commit-ID:  8ecedd7a06d27a31dbb36fab88e2ba6e6edd43ca
Gitweb:     http://git.kernel.org/tip/8ecedd7a06d27a31dbb36fab88e2ba6e6edd43ca
Author:     Bharata B Rao <[email protected]>
AuthorDate: Tue, 11 Jan 2011 15:42:57 +0530
Committer:  Ingo Molnar <[email protected]>
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 <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

---
 kernel/sched_autogroup.c |    5 +++++
 kernel/sched_debug.c     |    3 +++
 2 files changed, 8 insertions(+), 0 deletions(-)

kernel/sched_autogroup.c +5 -0

index 32a723b..938d52f 100644
@@ -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 */

kernel/sched_debug.c +3 -0

index 4d36f37..e4d3725 100644
@@ -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
 	 */

Mike Galbraith

2011-01-11 10:29 UTC #13 lore raw

In reply to Bharata B Rao

Quoted 7 lines · 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.

It's fine by me with or without.

-Mike