* [PATCH 0/3] perf tools: Patchset for perf sched.
@ 2014-05-05 7:05 Dongsheng Yang
2014-05-05 7:05 ` [PATCH 1/3] perf tools: add missing event for perf sched record Dongsheng Yang
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Dongsheng Yang @ 2014-05-05 7:05 UTC (permalink / raw)
To: jolsa; +Cc: a.p.zijlstra, paulus, mingo, acme, linux-kernel, 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.
tools/perf/builtin-sched.c | 38 +++++++++++++++++++++-----------------
1 file changed, 21 insertions(+), 17 deletions(-)
--
1.8.2.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] perf tools: add missing event for perf sched record.
2014-05-05 7:05 [PATCH 0/3] perf tools: Patchset for perf sched Dongsheng Yang
@ 2014-05-05 7:05 ` Dongsheng Yang
2014-05-12 16:00 ` [tip:perf/core] perf tools: Add " tip-bot for Dongsheng
2014-05-05 7:05 ` [PATCH 2/3] perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space Dongsheng Yang
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Dongsheng Yang @ 2014-05-05 7:05 UTC (permalink / raw)
To: jolsa; +Cc: a.p.zijlstra, paulus, mingo, acme, linux-kernel, Dongsheng
From: Dongsheng <yangds.fnst@cn.fujitsu.com>
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 <yangds.fnst@cn.fujitsu.com>
---
tools/perf/builtin-sched.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index d3fb0ed..7eae501 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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",
};
--
1.8.2.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space.
2014-05-05 7:05 [PATCH 0/3] perf tools: Patchset for perf sched Dongsheng Yang
2014-05-05 7:05 ` [PATCH 1/3] perf tools: add missing event for perf sched record Dongsheng Yang
@ 2014-05-05 7:05 ` Dongsheng Yang
2014-05-12 16:00 ` [tip:perf/core] " tip-bot for Dongsheng
2014-05-05 7:05 ` [PATCH 3/3] perf tools: Clarify the output of perf sched map Dongsheng Yang
2014-05-05 18:25 ` [PATCH 0/3] perf tools: Patchset for perf sched Jiri Olsa
3 siblings, 1 reply; 17+ messages in thread
From: Dongsheng Yang @ 2014-05-05 7:05 UTC (permalink / raw)
To: jolsa; +Cc: a.p.zijlstra, paulus, mingo, acme, linux-kernel, Dongsheng
From: Dongsheng <yangds.fnst@cn.fujitsu.com>
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 <yangds.fnst@cn.fujitsu.com>
---
tools/perf/builtin-sched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 7eae501..4f0dd21 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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,
--
1.8.2.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] perf tools: Clarify the output of perf sched map.
2014-05-05 7:05 [PATCH 0/3] perf tools: Patchset for perf sched Dongsheng Yang
2014-05-05 7:05 ` [PATCH 1/3] perf tools: add missing event for perf sched record Dongsheng Yang
2014-05-05 7:05 ` [PATCH 2/3] perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space Dongsheng Yang
@ 2014-05-05 7:05 ` Dongsheng Yang
2014-05-05 18:24 ` Jiri Olsa
2014-05-05 18:37 ` Ingo Molnar
2014-05-05 18:25 ` [PATCH 0/3] perf tools: Patchset for perf sched Jiri Olsa
3 siblings, 2 replies; 17+ messages in thread
From: Dongsheng Yang @ 2014-05-05 7:05 UTC (permalink / raw)
To: jolsa; +Cc: a.p.zijlstra, paulus, mingo, acme, linux-kernel, Dongsheng
From: Dongsheng <yangds.fnst@cn.fujitsu.com>
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 <yangds.fnst@cn.fujitsu.com>
---
tools/perf/builtin-sched.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 4f0dd21..c2bf8f2 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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(" ");
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] perf tools: Clarify the output of perf sched map.
2014-05-05 7:05 ` [PATCH 3/3] perf tools: Clarify the output of perf sched map Dongsheng Yang
@ 2014-05-05 18:24 ` Jiri Olsa
2014-05-05 18:27 ` David Ahern
2014-05-05 18:37 ` Ingo Molnar
1 sibling, 1 reply; 17+ messages in thread
From: Jiri Olsa @ 2014-05-05 18:24 UTC (permalink / raw)
To: Dongsheng Yang
Cc: a.p.zijlstra, paulus, mingo, acme, linux-kernel, David Ahern
On Mon, May 05, 2014 at 04:05:55PM +0900, Dongsheng Yang wrote:
> From: Dongsheng <yangds.fnst@cn.fujitsu.com>
>
> 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
I've never used 'perf sched map' before, so I'm not
sure about this one.. Arnaldo, David, Ingo? ;-)
thanks,
jirka
>
> Signed-off-by: Dongsheng <yangds.fnst@cn.fujitsu.com>
> ---
> tools/perf/builtin-sched.c | 35 +++++++++++++++++++----------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 4f0dd21..c2bf8f2 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -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(" ");
> }
>
> --
> 1.8.2.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] perf tools: Patchset for perf sched.
2014-05-05 7:05 [PATCH 0/3] perf tools: Patchset for perf sched Dongsheng Yang
` (2 preceding siblings ...)
2014-05-05 7:05 ` [PATCH 3/3] perf tools: Clarify the output of perf sched map Dongsheng Yang
@ 2014-05-05 18:25 ` Jiri Olsa
3 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2014-05-05 18:25 UTC (permalink / raw)
To: Dongsheng Yang; +Cc: a.p.zijlstra, paulus, mingo, acme, linux-kernel
On Mon, May 05, 2014 at 04:05:52PM +0900, Dongsheng Yang wrote:
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] perf tools: Clarify the output of perf sched map.
2014-05-05 18:24 ` Jiri Olsa
@ 2014-05-05 18:27 ` David Ahern
0 siblings, 0 replies; 17+ messages in thread
From: David Ahern @ 2014-05-05 18:27 UTC (permalink / raw)
To: Jiri Olsa, Dongsheng Yang; +Cc: a.p.zijlstra, paulus, mingo, acme, linux-kernel
On 5/5/14, 12:24 PM, Jiri Olsa wrote:
> On Mon, May 05, 2014 at 04:05:55PM +0900, Dongsheng Yang wrote:
>> From: Dongsheng <yangds.fnst@cn.fujitsu.com>
>>
>> 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
>
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] perf tools: Clarify the output of perf sched map.
2014-05-05 7:05 ` [PATCH 3/3] perf tools: Clarify the output of perf sched map Dongsheng Yang
2014-05-05 18:24 ` Jiri Olsa
@ 2014-05-05 18:37 ` Ingo Molnar
2014-05-06 0:40 ` [PATCH V2] " Dongsheng Yang
1 sibling, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2014-05-05 18:37 UTC (permalink / raw)
To: Dongsheng Yang; +Cc: jolsa, a.p.zijlstra, paulus, mingo, acme, linux-kernel
* Dongsheng Yang <yangds.fnst@cn.fujitsu.com> wrote:
> From: Dongsheng <yangds.fnst@cn.fujitsu.com>
>
> 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
Sounds good!
I only have some minor nits about the code:
> tools/perf/builtin-sched.c | 35 +++++++++++++++++++----------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 4f0dd21..c2bf8f2 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -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 {
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 <mingo@kernel.org>
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2] perf tools: Clarify the output of perf sched map.
2014-05-05 18:37 ` Ingo Molnar
@ 2014-05-06 0:40 ` Dongsheng Yang
2014-05-06 6:23 ` Ingo Molnar
0 siblings, 1 reply; 17+ messages in thread
From: Dongsheng Yang @ 2014-05-06 0:40 UTC (permalink / raw)
To: mingo; +Cc: jolsa, a.p.zijlstra, paulus, acme, linux-kernel, Dongsheng
From: Dongsheng <yangds.fnst@cn.fujitsu.com>
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 <yangds.fnst@cn.fujitsu.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
---
tools/perf/builtin-sched.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 4f0dd21..c6a6f5c 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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(" ");
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V2] perf tools: Clarify the output of perf sched map.
2014-05-06 6:23 ` Ingo Molnar
@ 2014-05-06 5:27 ` Dongsheng Yang
2014-05-06 5:39 ` [PATCH V3] " Dongsheng Yang
1 sibling, 0 replies; 17+ messages in thread
From: Dongsheng Yang @ 2014-05-06 5:27 UTC (permalink / raw)
To: Ingo Molnar; +Cc: jolsa, a.p.zijlstra, paulus, acme, linux-kernel
On 05/06/2014 03:23 PM, Ingo Molnar wrote:
> * Dongsheng Yang <yangds.fnst@cn.fujitsu.com> wrote:
>
>> 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:
>
>> *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.
>
>> + 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V3] perf tools: Clarify the output of perf sched map.
2014-05-06 6:23 ` Ingo Molnar
2014-05-06 5:27 ` Dongsheng Yang
@ 2014-05-06 5:39 ` Dongsheng Yang
2014-05-06 7:23 ` Ingo Molnar
2014-05-12 16:01 ` [tip:perf/core] " tip-bot for Dongsheng
1 sibling, 2 replies; 17+ messages in thread
From: Dongsheng Yang @ 2014-05-06 5:39 UTC (permalink / raw)
To: mingo; +Cc: jolsa, a.p.zijlstra, paulus, acme, linux-kernel, Dongsheng
From: Dongsheng <yangds.fnst@cn.fujitsu.com>
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 <yangds.fnst@cn.fujitsu.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
---
tools/perf/builtin-sched.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 4f0dd21..f3b35e5 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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(" ");
}
--
1.8.2.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH V2] perf tools: Clarify the output of perf sched map.
2014-05-06 0:40 ` [PATCH V2] " Dongsheng Yang
@ 2014-05-06 6:23 ` Ingo Molnar
2014-05-06 5:27 ` Dongsheng Yang
2014-05-06 5:39 ` [PATCH V3] " Dongsheng Yang
0 siblings, 2 replies; 17+ messages in thread
From: Ingo Molnar @ 2014-05-06 6:23 UTC (permalink / raw)
To: Dongsheng Yang; +Cc: jolsa, a.p.zijlstra, paulus, acme, linux-kernel
* Dongsheng Yang <yangds.fnst@cn.fujitsu.com> wrote:
> 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:
> *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:
> + sched_in->shortname[0] = ' ';
> + sched_in->shortname[1] = '.';
Thanks,
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V3] perf tools: Clarify the output of perf sched map.
2014-05-06 5:39 ` [PATCH V3] " Dongsheng Yang
@ 2014-05-06 7:23 ` Ingo Molnar
2014-05-06 12:44 ` Jiri Olsa
2014-05-12 16:01 ` [tip:perf/core] " tip-bot for Dongsheng
1 sibling, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2014-05-06 7:23 UTC (permalink / raw)
To: Dongsheng Yang; +Cc: jolsa, a.p.zijlstra, paulus, acme, linux-kernel
* Dongsheng Yang <yangds.fnst@cn.fujitsu.com> wrote:
> From: Dongsheng <yangds.fnst@cn.fujitsu.com>
>
> 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 <yangds.fnst@cn.fujitsu.com>
> Acked-by: Ingo Molnar <mingo@kernel.org>
Looks good to me, thanks!
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH V3] perf tools: Clarify the output of perf sched map.
2014-05-06 7:23 ` Ingo Molnar
@ 2014-05-06 12:44 ` Jiri Olsa
0 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2014-05-06 12:44 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Dongsheng Yang, a.p.zijlstra, paulus, acme, linux-kernel
On Tue, May 06, 2014 at 09:23:04AM +0200, Ingo Molnar wrote:
>
> * Dongsheng Yang <yangds.fnst@cn.fujitsu.com> wrote:
>
> > From: Dongsheng <yangds.fnst@cn.fujitsu.com>
> >
> > 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 <yangds.fnst@cn.fujitsu.com>
> > Acked-by: Ingo Molnar <mingo@kernel.org>
>
> Looks good to me, thanks!
thanks, I'll queue this patchset
jirka
^ permalink raw reply [flat|nested] 17+ messages in thread
* [tip:perf/core] perf tools: Add missing event for perf sched record.
2014-05-05 7:05 ` [PATCH 1/3] perf tools: add missing event for perf sched record Dongsheng Yang
@ 2014-05-12 16:00 ` tip-bot for Dongsheng
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Dongsheng @ 2014-05-12 16:00 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, jolsa, tglx, yangds.fnst
Commit-ID: 7fff959783949b2f50454c49e325697073f48dc0
Gitweb: http://git.kernel.org/tip/7fff959783949b2f50454c49e325697073f48dc0
Author: Dongsheng <yangds.fnst@cn.fujitsu.com>
AuthorDate: Mon, 5 May 2014 16:05:53 +0900
Committer: Jiri Olsa <jolsa@kernel.org>
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 <yangds.fnst@cn.fujitsu.com>
Link: http://lkml.kernel.org/r/710c6edd2162b2cea1711443f54de47c0210d9fd.1399273302.git.yangds.fnst@cn.fujitsu.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-sched.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index d3fb0ed..7eae501 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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",
};
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:perf/core] perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space.
2014-05-05 7:05 ` [PATCH 2/3] perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space Dongsheng Yang
@ 2014-05-12 16:00 ` tip-bot for Dongsheng
0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Dongsheng @ 2014-05-12 16:00 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, jolsa, tglx, yangds.fnst
Commit-ID: e936e8e459e14af7432a775f8139a79b71e41afc
Gitweb: http://git.kernel.org/tip/e936e8e459e14af7432a775f8139a79b71e41afc
Author: Dongsheng <yangds.fnst@cn.fujitsu.com>
AuthorDate: Mon, 5 May 2014 16:05:54 +0900
Committer: Jiri Olsa <jolsa@kernel.org>
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 <yangds.fnst@cn.fujitsu.com>
Link: http://lkml.kernel.org/r/6d2f55dc1e02c1e29a5d70bfeb9d6e8863caf2aa.1399273302.git.yangds.fnst@cn.fujitsu.com
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-sched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 7eae501..4f0dd21 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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,
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [tip:perf/core] perf tools: Clarify the output of perf sched map.
2014-05-06 5:39 ` [PATCH V3] " Dongsheng Yang
2014-05-06 7:23 ` Ingo Molnar
@ 2014-05-12 16:01 ` tip-bot for Dongsheng
1 sibling, 0 replies; 17+ messages in thread
From: tip-bot for Dongsheng @ 2014-05-12 16:01 UTC (permalink / raw)
To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, jolsa, tglx, yangds.fnst
Commit-ID: 6bcab4e1eaa1f669d003051ef3b87a963d8763bb
Gitweb: http://git.kernel.org/tip/6bcab4e1eaa1f669d003051ef3b87a963d8763bb
Author: Dongsheng <yangds.fnst@cn.fujitsu.com>
AuthorDate: Tue, 6 May 2014 14:39:01 +0900
Committer: Jiri Olsa <jolsa@kernel.org>
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 <yangds.fnst@cn.fujitsu.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Link: http://lkml.kernel.org/r/1399354741-19522-1-git-send-email-yangds.fnst@cn.fujitsu.com
[ small style fixes to make checkpatch happy ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-sched.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 4f0dd21..2579215 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -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(" ");
}
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-05-12 16:01 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-05 7:05 [PATCH 0/3] perf tools: Patchset for perf sched Dongsheng Yang
2014-05-05 7:05 ` [PATCH 1/3] perf tools: add missing event for perf sched record Dongsheng Yang
2014-05-12 16:00 ` [tip:perf/core] perf tools: Add " tip-bot for Dongsheng
2014-05-05 7:05 ` [PATCH 2/3] perf tools: Adapt the TASK_STATE_TO_CHAR_STR to new value in kernel space Dongsheng Yang
2014-05-12 16:00 ` [tip:perf/core] " tip-bot for Dongsheng
2014-05-05 7:05 ` [PATCH 3/3] perf tools: Clarify the output of perf sched map Dongsheng Yang
2014-05-05 18:24 ` Jiri Olsa
2014-05-05 18:27 ` David Ahern
2014-05-05 18:37 ` Ingo Molnar
2014-05-06 0:40 ` [PATCH V2] " Dongsheng Yang
2014-05-06 6:23 ` Ingo Molnar
2014-05-06 5:27 ` Dongsheng Yang
2014-05-06 5:39 ` [PATCH V3] " Dongsheng Yang
2014-05-06 7:23 ` Ingo Molnar
2014-05-06 12:44 ` Jiri Olsa
2014-05-12 16:01 ` [tip:perf/core] " tip-bot for Dongsheng
2014-05-05 18:25 ` [PATCH 0/3] perf tools: Patchset for perf sched Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).