* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.