Skip to contentSkip to conversation

perf tools: Patchset for perf sched.

PATCH In review Dongsheng Yang 5 participants 17 messages May 5, 2014 – May 12, 2014 perf events

lore
Share & tools
lore

Dongsheng Yang

author
#0 lore raw

cover perf tools: Patchset for perf sched.

Hi Jiri,
This patchset includes three patches for perf sched.
Please help to review. :)

Dongsheng (3):
perf tools: add missing event for perf sched record. perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel
space.
perf tools: Clarify the output of perf sched map.

 tools/perf/builtin-sched.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)
Signature · 1 line
1.8.2.1
author
#1 lore raw

PATCH 1/3 perf tools: add missing event for perf sched record.

From: Dongsheng <[email protected]>

We should record and process sched:sched_wakeup_new event in perf sched tool, but currently, there is the process function for it, without recording it in record subcommand.

This patch add -e sched:sched_wakeup_new to perf sched record.

Signed-off-by Dongsheng <[email protected]>
---
 tools/perf/builtin-sched.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/builtin-sched.c +1 -0
index d3fb0ed..7eae501 100644
@@ -1635,6 +1635,7 @@ static int __cmd_record(int argc, const char **argv)
 		"-e", "sched:sched_stat_runtime",
 		"-e", "sched:sched_process_fork",
 		"-e", "sched:sched_wakeup",
+		"-e", "sched:sched_wakeup_new",
 		"-e", "sched:sched_migrate_task",
 	};
Signature · 1 line
1.8.2.1
merge bot
#2 lore raw

[tip:perf/core] perf tools: Add missing event for perf sched record.

perf tools: Add missing event for perf sched record.
Commit-ID:  7fff959783949b2f50454c49e325697073f48dc0
Gitweb:     http://git.kernel.org/tip/7fff959783949b2f50454c49e325697073f48dc0
Author:     Dongsheng <[email protected]>
AuthorDate: Mon, 5 May 2014 16:05:53 +0900
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 12 May 2014 10:01:41 +0200

perf tools: Add missing event for perf sched record.

We should record and process sched:sched_wakeup_new event in perf sched tool, but currently, there is the process function for it, without recording it in record subcommand.

This patch add -e sched:sched_wakeup_new to perf sched record.

Signed-off-by Dongsheng <[email protected]>
Link http://lkml.kernel.org/r/710c6edd2162b2cea1711443f54de47c0210d9fd.1399273302.git.yangds.fnst@cn.fujitsu.com open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/builtin-sched.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/builtin-sched.c +1 -0
index d3fb0ed..7eae501 100644
@@ -1635,6 +1635,7 @@ static int __cmd_record(int argc, const char **argv)
 		"-e", "sched:sched_stat_runtime",
 		"-e", "sched:sched_process_fork",
 		"-e", "sched:sched_wakeup",
+		"-e", "sched:sched_wakeup_new",
 		"-e", "sched:sched_migrate_task",
 	};
author
#3 lore raw

PATCH 2/3 perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space.

In reply to Dongsheng Yang

From: Dongsheng <[email protected]>

Currently, TASK_STATE_TO_CHAR_STR in kernel space is already expanded to RSDTtZXxKWP, but it is still RSDTtZX in perf sched tool.

This patch update TASK_STATE_TO_CHAR_STR to the new value in kernel space.

Signed-off-by Dongsheng <[email protected]>
---
 tools/perf/builtin-sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-sched.c +1 -1
index 7eae501..4f0dd21 100644
@@ -66,7 +66,7 @@ struct sched_atom {
 	struct task_desc	*wakee;
 };
 
-#define TASK_STATE_TO_CHAR_STR "RSDTtZX"
+#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKWP"
 
 enum thread_state {
 	THREAD_SLEEPING = 0,
Signature · 1 line
1.8.2.1
merge bot
#4 lore raw

[tip:perf/core] perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space.

perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space.
Commit-ID:  e936e8e459e14af7432a775f8139a79b71e41afc
Gitweb:     http://git.kernel.org/tip/e936e8e459e14af7432a775f8139a79b71e41afc
Author:     Dongsheng <[email protected]>
AuthorDate: Mon, 5 May 2014 16:05:54 +0900
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 12 May 2014 10:01:49 +0200

perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space.

Currently, TASK_STATE_TO_CHAR_STR in kernel space is already expanded to RSDTtZXxKWP, but it is still RSDTtZX in perf sched tool.

This patch update TASK_STATE_TO_CHAR_STR to the new value in kernel space.

Signed-off-by Dongsheng <[email protected]>
Link http://lkml.kernel.org/r/6d2f55dc1e02c1e29a5d70bfeb9d6e8863caf2aa.1399273302.git.yangds.fnst@cn.fujitsu.com open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/builtin-sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-sched.c +1 -1
index 7eae501..4f0dd21 100644
@@ -66,7 +66,7 @@ struct sched_atom {
 	struct task_desc	*wakee;
 };
 
-#define TASK_STATE_TO_CHAR_STR "RSDTtZX"
+#define TASK_STATE_TO_CHAR_STR "RSDTtZXxKWP"
 
 enum thread_state {
 	THREAD_SLEEPING = 0,
author
#5 lore raw

PATCH 3/3 perf tools: Clarify the output of perf sched map.

In reply to Dongsheng Yang

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

	      *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs . => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7
Signed-off-by Dongsheng <[email protected]>
---
 tools/perf/builtin-sched.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)
tools/perf/builtin-sched.c +19 -16
index 4f0dd21..c2bf8f2 100644
@@ -1300,17 +1300,23 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 
 	new_shortname = 0;
 	if (!sched_in->shortname[0]) {
-		sched_in->shortname[0] = sched->next_shortname1;
-		sched_in->shortname[1] = sched->next_shortname2;
-
-		if (sched->next_shortname1 < 'Z') {
-			sched->next_shortname1++;
-		} else {
-			sched->next_shortname1='A';
-			if (sched->next_shortname2 < '9') {
-				sched->next_shortname2++;
+		if (!strcmp(thread__comm_str(sched_in), "swapper")) {
+			sched_in->shortname[0] = '.';
+			sched_in->shortname[1] = '\0';
+		}
+		else {
+			sched_in->shortname[0] = sched->next_shortname1;
+			sched_in->shortname[1] = sched->next_shortname2;
+
+			if (sched->next_shortname1 < 'Z') {
+				sched->next_shortname1++;
 			} else {
-				sched->next_shortname2='0';
+				sched->next_shortname1='A';
+				if (sched->next_shortname2 < '9') {
+					sched->next_shortname2++;
+				} else {
+					sched->next_shortname2='0';
+				}
 			}
 		}
 		new_shortname = 1;

@@ -1322,12 +1328,9 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 		else
 			printf("*");
 
-		if (sched->curr_thread[cpu]) {
-			if (sched->curr_thread[cpu]->tid)
-				printf("%2s ", sched->curr_thread[cpu]->shortname);
-			else
-				printf(".  ");
-		} else
+		if (sched->curr_thread[cpu])
+			printf("%2s ", sched->curr_thread[cpu]->shortname);
+		else
 			printf("   ");
 	}
Signature · 1 line
1.8.2.1
#6 lore raw

Re: [PATCH 3/3] perf tools: Clarify the output of perf sched map.

Quoted 21 lines · Dongsheng Yang

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

	      *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Quoted 8 lines

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs . => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7

I've never used 'perf sched map' before, so I'm not sure about this one.. Arnaldo, David, Ingo? ;-)

thanks,
jirka

Quoted 63 lines
Signed-off-by Dongsheng <[email protected]>
---
 tools/perf/builtin-sched.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)
tools/perf/builtin-sched.c +19 -16
index 4f0dd21..c2bf8f2 100644
@@ -1300,17 +1300,23 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 
 	new_shortname = 0;
 	if (!sched_in->shortname[0]) {
-		sched_in->shortname[0] = sched->next_shortname1;
-		sched_in->shortname[1] = sched->next_shortname2;
-
-		if (sched->next_shortname1 < 'Z') {
-			sched->next_shortname1++;
-		} else {
-			sched->next_shortname1='A';
-			if (sched->next_shortname2 < '9') {
-				sched->next_shortname2++;
+		if (!strcmp(thread__comm_str(sched_in), "swapper")) {
+			sched_in->shortname[0] = '.';
+			sched_in->shortname[1] = '\0';
+		}
+		else {
+			sched_in->shortname[0] = sched->next_shortname1;
+			sched_in->shortname[1] = sched->next_shortname2;
+
+			if (sched->next_shortname1 < 'Z') {
+				sched->next_shortname1++;
 			} else {
-				sched->next_shortname2='0';
+				sched->next_shortname1='A';
+				if (sched->next_shortname2 < '9') {
+					sched->next_shortname2++;
+				} else {
+					sched->next_shortname2='0';
+				}
 			}
 		}
 		new_shortname = 1;

@@ -1322,12 +1328,9 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 		else
 			printf("*");
 
-		if (sched->curr_thread[cpu]) {
-			if (sched->curr_thread[cpu]->tid)
-				printf("%2s ", sched->curr_thread[cpu]->shortname);
-			else
-				printf(".  ");
-		} else
+		if (sched->curr_thread[cpu])
+			printf("%2s ", sched->curr_thread[cpu]->shortname);
+		else
 			printf("   ");
 	}
Signature · 1 line
1.8.2.1
#7 lore raw

Re: [PATCH 3/3] perf tools: Clarify the output of perf sched map.

Quoted 31 lines · Jiri Olsa
Quoted 21 lines · Dongsheng Yang

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

	      *A0       228836.978985 secs A0 => perf:23032
           *.   A0       228836.979016 secs B0 => swapper:0
            .  *C0       228836.979099 secs C0 => migration/3:22
   *A0      .   C0       228836.979115 secs
    A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Quoted 8 lines

Example:

               *A0       228836.978985 secs A0 => perf:23032
           * .  A0       228836.979016 secs . => swapper:0
             . *B0       228836.979099 secs B0 => migration/3:22
   *A0       .  B0       228836.979115 secs
    A0       . * .       228836.979115 secs
    A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
    A0     *D0   .       228836.979236 secs D0 => rcu_sched:7
Quoted 2 lines

I've never used 'perf sched map' before, so I'm not sure about this one.. Arnaldo, David, Ingo? ;-)

Patches 1 and 2 look ok to me.

This one is a preference change -- deferring to Ingo and Arnaldo on it.

David

Acked-by on v1 3/3
#8 lore raw

Re: [PATCH 3/3] perf tools: Clarify the output of perf sched map.

In reply to Dongsheng Yang
Quoted 21 lines · Dongsheng Yang

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

	      *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Quoted 8 lines

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs . => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7

Sounds good!

I only have some minor nits about the code:

Quoted 15 lines
 tools/perf/builtin-sched.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)
tools/perf/builtin-sched.c +0 -4
index 4f0dd21..c2bf8f2 100644
@@ -1300,17 +1300,23 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 
 	new_shortname = 0;
 	if (!sched_in->shortname[0]) {
-		sched_in->shortname[0] = sched->next_shortname1;
-		sched_in->shortname[1] = sched->next_shortname2;
-
-		if (sched->next_shortname1 < 'Z') {

tools/perf/builtin-sched.c static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel, · line 1307

Quoted 10 lines
tools/perf/builtin-sched.c +5 -5
-			sched->next_shortname1++;
-		} else {
-			sched->next_shortname1='A';
-			if (sched->next_shortname2 < '9') {
-				sched->next_shortname2++;
+		if (!strcmp(thread__comm_str(sched_in), "swapper")) {
+			sched_in->shortname[0] = '.';
+			sched_in->shortname[1] = '\0';
+		}
+		else {

Should be '} else {' as per kernel coding style.

Also, a comment about handling the idle task in a special way would be useful - for the far future when this discussion will be distant memories or less!

With those nits addressed:

Acked-by Ingo Molnar <[email protected]>

Thanks,

Ingo

author
#9 lore raw

PATCH v2 1/1 perf tools: Clarify the output of perf sched map.

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

	      *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs . => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7
Signed-off-by Dongsheng <[email protected]>
Acked-by Ingo Molnar <[email protected]>
---
 tools/perf/builtin-sched.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)
tools/perf/builtin-sched.c +22 -16
index 4f0dd21..c6a6f5c 100644
@@ -1300,17 +1300,26 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 
 	new_shortname = 0;
 	if (!sched_in->shortname[0]) {
-		sched_in->shortname[0] = sched->next_shortname1;
-		sched_in->shortname[1] = sched->next_shortname2;
-
-		if (sched->next_shortname1 < 'Z') {
-			sched->next_shortname1++;
-		} else {
-			sched->next_shortname1='A';
-			if (sched->next_shortname2 < '9') {
-				sched->next_shortname2++;
+		if (!strcmp(thread__comm_str(sched_in), "swapper")) {
+			/*
+			 * Don't allocate a letter-number for swapper:0
+			 * as a shortname. Instead, we use '.' for it.
+			 */
+			sched_in->shortname[0] = '.';
+			sched_in->shortname[1] = '\0';
+		}else {
+			sched_in->shortname[0] = sched->next_shortname1;
+			sched_in->shortname[1] = sched->next_shortname2;
+
+			if (sched->next_shortname1 < 'Z') {
+				sched->next_shortname1++;
 			} else {
-				sched->next_shortname2='0';
+				sched->next_shortname1='A';
+				if (sched->next_shortname2 < '9') {
+					sched->next_shortname2++;
+				} else {
+					sched->next_shortname2='0';
+				}
 			}
 		}
 		new_shortname = 1;

@@ -1322,12 +1331,9 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 		else
 			printf("*");
 
-		if (sched->curr_thread[cpu]) {
-			if (sched->curr_thread[cpu]->tid)
-				printf("%2s ", sched->curr_thread[cpu]->shortname);
-			else
-				printf(".  ");
-		} else
+		if (sched->curr_thread[cpu])
+			printf("%2s ", sched->curr_thread[cpu]->shortname);
+		else
 			printf("   ");
 	}
Signature · 1 line
1.8.2.1
#10 lore raw

Re: [PATCH V2] perf tools: Clarify the output of perf sched map.

Quoted 8 lines · Dongsheng Yang

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs . => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7

One final thing I noticed, please keep vertical alignment intact, i.e. print:

Quoted 3 lines
              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs .  => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22

(note the extra space and how the arrows now align vertically.)

In the code this could be done via:

Review

Quoted 2 lines
(quoted diff) +2 -0
+			sched_in->shortname[0] = ' ';
+			sched_in->shortname[1] = '.';

Thanks,

Ingo

author
#11 lore raw

Re: [PATCH V2] perf tools: Clarify the output of perf sched map.

Quoted 13 lines · Ingo Molnar
Quoted 8 lines · Dongsheng Yang

Example:

               *A0       228836.978985 secs A0 => perf:23032
           * .  A0       228836.979016 secs . => swapper:0
             . *B0       228836.979099 secs B0 => migration/3:22
   *A0       .  B0       228836.979115 secs
    A0       . * .       228836.979115 secs
    A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
    A0     *D0   .       228836.979236 secs D0 => rcu_sched:7

One final thing I noticed, please keep vertical alignment intact, i.e. print:

Quoted 6 lines
Quoted 3 lines
               *A0       228836.978985 secs A0 => perf:23032
           * .  A0       228836.979016 secs .  => swapper:0
             . *B0       228836.979099 secs B0 => migration/3:22

(note the extra space and how the arrows now align vertically.)

In the code this could be done via:

Oh, yes. Easy to fix. Thanx Ingo and I will update it now.

Quoted 12 lines

Review

Quoted 3 lines
(quoted diff) +2 -0
+			sched_in->shortname[0] = ' ';
+			sched_in->shortname[1] = '.';

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

author
#12 lore raw

PATCH v3 1/1 perf tools: Clarify the output of perf sched map.

In reply to Ingo Molnar

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs .  => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7
Signed-off-by Dongsheng <[email protected]>
Acked-by Ingo Molnar <[email protected]>
---
 tools/perf/builtin-sched.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)
tools/perf/builtin-sched.c +22 -16
index 4f0dd21..f3b35e5 100644
@@ -1300,17 +1300,26 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 
 	new_shortname = 0;
 	if (!sched_in->shortname[0]) {
-		sched_in->shortname[0] = sched->next_shortname1;
-		sched_in->shortname[1] = sched->next_shortname2;
-
-		if (sched->next_shortname1 < 'Z') {
-			sched->next_shortname1++;
-		} else {
-			sched->next_shortname1='A';
-			if (sched->next_shortname2 < '9') {
-				sched->next_shortname2++;
+		if (!strcmp(thread__comm_str(sched_in), "swapper")) {
+			/*
+			 * Don't allocate a letter-number for swapper:0
+			 * as a shortname. Instead, we use '.' for it.
+			 */
+			sched_in->shortname[0] = '.';
+			sched_in->shortname[1] = ' ';
+		}else {
+			sched_in->shortname[0] = sched->next_shortname1;
+			sched_in->shortname[1] = sched->next_shortname2;
+
+			if (sched->next_shortname1 < 'Z') {
+				sched->next_shortname1++;
 			} else {
-				sched->next_shortname2='0';
+				sched->next_shortname1='A';
+				if (sched->next_shortname2 < '9') {
+					sched->next_shortname2++;
+				} else {
+					sched->next_shortname2='0';
+				}
 			}
 		}
 		new_shortname = 1;

@@ -1322,12 +1331,9 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 		else
 			printf("*");
 
-		if (sched->curr_thread[cpu]) {
-			if (sched->curr_thread[cpu]->tid)
-				printf("%2s ", sched->curr_thread[cpu]->shortname);
-			else
-				printf(".  ");
-		} else
+		if (sched->curr_thread[cpu])
+			printf("%2s ", sched->curr_thread[cpu]->shortname);
+		else
 			printf("   ");
 	}
Signature · 1 line
1.8.2.1
#13 lore raw

Re: [PATCH V3] perf tools: Clarify the output of perf sched map.

Quoted 30 lines · Dongsheng Yang

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs .  => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7
Quoted 2 lines
Signed-off-by Dongsheng <[email protected]>
Acked-by Ingo Molnar <[email protected]>

Looks good to me, thanks!

Ingo

#14 lore raw

Re: [PATCH V3] perf tools: Clarify the output of perf sched map.

Quoted 33 lines · Ingo Molnar
Quoted 30 lines · Dongsheng Yang

From: Dongsheng <[email protected]>

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explaination for swapper:0.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs .  => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7
Quoted 4 lines
Quoted 2 lines
Signed-off-by Dongsheng <[email protected]>
Acked-by Ingo Molnar <[email protected]>

Looks good to me, thanks!

thanks, I'll queue this patchset

jirka

merge bot
#15 lore raw

[tip:perf/core] perf tools: Clarify the output of perf sched map.

In reply to Dongsheng Yang
perf tools: Clarify the output of perf sched map.
Commit-ID:  6bcab4e1eaa1f669d003051ef3b87a963d8763bb
Gitweb:     http://git.kernel.org/tip/6bcab4e1eaa1f669d003051ef3b87a963d8763bb
Author:     Dongsheng <[email protected]>
AuthorDate: Tue, 6 May 2014 14:39:01 +0900
Committer:  Jiri Olsa <[email protected]>
CommitDate: Mon, 12 May 2014 11:09:05 +0200

perf tools: Clarify the output of perf sched map.

In output of perf sched map, any shortname of thread will be explained at the first time when it appear.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          *.   A0       228836.979016 secs B0 => swapper:0
           .  *C0       228836.979099 secs C0 => migration/3:22
  *A0      .   C0       228836.979115 secs
   A0      .  *.        228836.979115 secs

But B0, which is explained as swapper:0 did not appear in the left part of output. Instead, we use '.' as the shortname of swapper:0. So the comment of "B0 => swapper:0" is not easy to understand.

This patch clarify the output of perf sched map with not allocating one letter-number shortname for swapper:0 and print ". => swapper:0" as the explanation for swapper:0.

Example:

              *A0       228836.978985 secs A0 => perf:23032
          * .  A0       228836.979016 secs .  => swapper:0
            . *B0       228836.979099 secs B0 => migration/3:22
  *A0       .  B0       228836.979115 secs
   A0       . * .       228836.979115 secs
   A0     *C0   .       228836.979225 secs C0 => ksoftirqd/2:18
   A0     *D0   .       228836.979236 secs D0 => rcu_sched:7
Signed-off-by Dongsheng <[email protected]>
Acked-by Ingo Molnar <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
[ small style fixes to make checkpatch happy ]
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/builtin-sched.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)
tools/perf/builtin-sched.c +20 -15
index 4f0dd21..2579215 100644
@@ -1300,17 +1300,25 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 
 	new_shortname = 0;
 	if (!sched_in->shortname[0]) {
-		sched_in->shortname[0] = sched->next_shortname1;
-		sched_in->shortname[1] = sched->next_shortname2;
-
-		if (sched->next_shortname1 < 'Z') {
-			sched->next_shortname1++;
+		if (!strcmp(thread__comm_str(sched_in), "swapper")) {
+			/*
+			 * Don't allocate a letter-number for swapper:0
+			 * as a shortname. Instead, we use '.' for it.
+			 */
+			sched_in->shortname[0] = '.';
+			sched_in->shortname[1] = ' ';
 		} else {
-			sched->next_shortname1='A';
-			if (sched->next_shortname2 < '9') {
-				sched->next_shortname2++;
+			sched_in->shortname[0] = sched->next_shortname1;
+			sched_in->shortname[1] = sched->next_shortname2;
+
+			if (sched->next_shortname1 < 'Z') {
+				sched->next_shortname1++;
 			} else {
-				sched->next_shortname2='0';
+				sched->next_shortname1 = 'A';
+				if (sched->next_shortname2 < '9')
+					sched->next_shortname2++;
+				else
+					sched->next_shortname2 = '0';
 			}
 		}
 		new_shortname = 1;

@@ -1322,12 +1330,9 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
 		else
 			printf("*");
 
-		if (sched->curr_thread[cpu]) {
-			if (sched->curr_thread[cpu]->tid)
-				printf("%2s ", sched->curr_thread[cpu]->shortname);
-			else
-				printf(".  ");
-		} else
+		if (sched->curr_thread[cpu])
+			printf("%2s ", sched->curr_thread[cpu]->shortname);
+		else
 			printf("   ");
 	}
#16 lore raw
In reply to Dongsheng Yang
Quoted 9 lines · Dongsheng Yang

Hi Jiri,
This patchset includes three patches for perf sched.
Please help to review. :)

Dongsheng (3):
perf tools: add missing event for perf sched record. perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel
space.
perf tools: Clarify the output of perf sched map.

hi,
looks good.. let's see if there's some feedback for 3/3

thanks,
jirka