2015-02-13 13:10:15

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 0/7] perf list: make some improvements and fixes

Hi,
Found some functions to improve and bugs to fix in perf list.

Yunlong Song (7):
perf list: clean up the printing functions of hardware/software events
perf list: sort the output of 'perf list' to view more clearly
perf list: fix some inaccuracy problem when parsing the argument
perf list: fix a bug of segmentation fault
perf list: avoid confusion of perf output and the next command prompt
perf list: extend raw-dump to certain kind of events
perf list: place the guiding text in its right position

tools/perf/Documentation/perf-list.txt | 6 +
tools/perf/builtin-list.c | 28 ++---
tools/perf/perf.c | 1 +
tools/perf/util/parse-events.c | 210 +++++++++++++++++++++++----------
tools/perf/util/parse-events.h | 11 +-
tools/perf/util/parse-options.c | 8 +-
6 files changed, 184 insertions(+), 80 deletions(-)

--
1.8.5.5


2015-02-13 13:10:14

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 1/7] perf list: clean up the printing functions of hardware/software events

Do not need print_events_type or __print_events_type for listing hw/sw
events, let print_symbol_events do its job instead. Moreover,
print_symbol_events can also handle event_glob and name_only.

Signed-off-by: Yunlong Song <[email protected]>
---
tools/perf/builtin-list.c | 6 ++++--
tools/perf/util/parse-events.c | 39 +++------------------------------------
tools/perf/util/parse-events.h | 11 ++++++++++-
3 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 198f3c3..051a163 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -53,10 +53,12 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
print_tracepoint_events(NULL, NULL, false);
else if (strcmp(argv[i], "hw") == 0 ||
strcmp(argv[i], "hardware") == 0)
- print_events_type(PERF_TYPE_HARDWARE);
+ print_symbol_events(NULL, PERF_TYPE_HARDWARE,
+ event_symbols_hw, PERF_COUNT_HW_MAX, false);
else if (strcmp(argv[i], "sw") == 0 ||
strcmp(argv[i], "software") == 0)
- print_events_type(PERF_TYPE_SOFTWARE);
+ print_symbol_events(NULL, PERF_TYPE_SOFTWARE,
+ event_symbols_sw, PERF_COUNT_SW_MAX, false);
else if (strcmp(argv[i], "cache") == 0 ||
strcmp(argv[i], "hwcache") == 0)
print_hwcache_events(NULL, false);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7f8ec6c..fa876da 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -20,11 +20,6 @@

#define MAX_NAME_LEN 100

-struct event_symbol {
- const char *symbol;
- const char *alias;
-};
-
#ifdef PARSER_DEBUG
extern int parse_events_debug;
#endif
@@ -39,7 +34,7 @@ static struct perf_pmu_event_symbol *perf_pmu_events_list;
*/
static int perf_pmu_events_list_num;

-static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
+struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
[PERF_COUNT_HW_CPU_CYCLES] = {
.symbol = "cpu-cycles",
.alias = "cycles",
@@ -82,7 +77,7 @@ static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
},
};

-static struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
+struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
[PERF_COUNT_SW_CPU_CLOCK] = {
.symbol = "cpu-clock",
.alias = "",
@@ -1233,34 +1228,6 @@ static bool is_event_supported(u8 type, unsigned config)
return ret;
}

-static void __print_events_type(u8 type, struct event_symbol *syms,
- unsigned max)
-{
- char name[64];
- unsigned i;
-
- for (i = 0; i < max ; i++, syms++) {
- if (!is_event_supported(type, i))
- continue;
-
- if (strlen(syms->alias))
- snprintf(name, sizeof(name), "%s OR %s",
- syms->symbol, syms->alias);
- else
- snprintf(name, sizeof(name), "%s", syms->symbol);
-
- printf(" %-50s [%s]\n", name, event_type_descriptors[type]);
- }
-}
-
-void print_events_type(u8 type)
-{
- if (type == PERF_TYPE_SOFTWARE)
- __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
- else
- __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
-}
-
int print_hwcache_events(const char *event_glob, bool name_only)
{
unsigned int type, op, i, printed = 0;
@@ -1297,7 +1264,7 @@ int print_hwcache_events(const char *event_glob, bool name_only)
return printed;
}

-static void print_symbol_events(const char *event_glob, unsigned type,
+void print_symbol_events(const char *event_glob, unsigned type,
struct event_symbol *syms, unsigned max,
bool name_only)
{
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ff6e1fa..5eefb6a 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -116,7 +116,16 @@ void parse_events_update_lists(struct list_head *list_event,
void parse_events_error(void *data, void *scanner, char const *msg);

void print_events(const char *event_glob, bool name_only);
-void print_events_type(u8 type);
+
+struct event_symbol {
+ const char *symbol;
+ const char *alias;
+};
+extern struct event_symbol event_symbols_hw[];
+extern struct event_symbol event_symbols_sw[];
+void print_symbol_events(const char *event_glob, unsigned type,
+ struct event_symbol *syms, unsigned max,
+ bool name_only);
void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
bool name_only);
int print_hwcache_events(const char *event_glob, bool name_only);
--
1.8.5.5

2015-02-13 13:10:17

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly

Sort the output according to ASCII character list (using strcmp), which
supports both number sequence and alphabet sequence.

Example
Before this patch:
$perf list

List of pre-defined events (to be used in -e):
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
cache-references [Hardware event]
cache-misses [Hardware event]
branch-instructions OR branches [Hardware event]
branch-misses [Hardware event]
bus-cycles [Hardware event]
... ...

jbd2:jbd2_start_commit [Tracepoint event]
jbd2:jbd2_commit_locking [Tracepoint event]
jbd2:jbd2_run_stats [Tracepoint event]
block:block_rq_issue [Tracepoint event]
block:block_bio_complete [Tracepoint event]
block:block_bio_backmerge [Tracepoint event]
block:block_getrq [Tracepoint event]
... ...

After this patch:
$perf list

List of pre-defined events (to be used in -e):
branch-instructions OR branches [Hardware event]
branch-misses [Hardware event]
bus-cycles [Hardware event]
cache-misses [Hardware event]
cache-references [Hardware event]
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
... ...

block:block_bio_backmerge [Tracepoint event]
block:block_bio_complete [Tracepoint event]
block:block_getrq [Tracepoint event]
block:block_rq_issue [Tracepoint event]
jbd2:jbd2_commit_locking [Tracepoint event]
jbd2:jbd2_run_stats [Tracepoint event]
jbd2:jbd2_start_commit [Tracepoint event]
... ...

Signed-off-by: Yunlong Song <[email protected]>
---
tools/perf/builtin-list.c | 2 -
tools/perf/util/parse-events.c | 166 +++++++++++++++++++++++++++++++++++------
2 files changed, 145 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 051a163..de5680e 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -47,8 +47,6 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
}

for (i = 0; i < argc; ++i) {
- if (i)
- putchar('\n');
if (strncmp(argv[i], "tracepoint", 10) == 0)
print_tracepoint_events(NULL, NULL, false);
else if (strcmp(argv[i], "hw") == 0 ||
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index fa876da..cf35b0a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1093,6 +1093,13 @@ static const char * const event_type_descriptors[] = {
"Hardware breakpoint",
};

+static int cmp_string(const void *a, const void *b)
+{
+ const char * const *as = a;
+ const char * const *bs = b;
+ return strcmp(*as, *bs);
+}
+
/*
* Print the events from <debugfs_mount_point>/tracing/events
*/
@@ -1105,6 +1112,9 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
char evt_path[MAXPATHLEN];
char dir_path[MAXPATHLEN];
char sbuf[STRERR_BUFSIZE];
+ char **evt_list = NULL;
+ unsigned int evt_i = 0, evt_num = 0;
+ bool evt_num_known = false;

if (debugfs_valid_mountpoint(tracing_events_path)) {
printf(" [ Tracepoints not available: %s ]\n",
@@ -1112,9 +1122,16 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
return;
}

+restart:
sys_dir = opendir(tracing_events_path);
if (!sys_dir)
return;
+
+ if (evt_num_known) {
+ evt_list = zalloc(sizeof(char *) * evt_num);
+ if (!evt_list)
+ goto out_enomem;
+ }

for_each_subsystem(sys_dir, sys_dirent, sys_next) {
if (subsys_glob != NULL &&
@@ -1132,19 +1149,52 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
!strglobmatch(evt_dirent.d_name, event_glob))
continue;

- if (name_only) {
- printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);
+ if (!evt_num_known) {
+ evt_num++;
continue;
}

snprintf(evt_path, MAXPATHLEN, "%s:%s",
sys_dirent.d_name, evt_dirent.d_name);
- printf(" %-50s [%s]\n", evt_path,
- event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+
+ evt_list[evt_i] = strdup(evt_path);
+ if (evt_list[evt_i] == NULL)
+ goto out_enomem;
+ evt_i++;
}
closedir(evt_dir);
}
closedir(sys_dir);
+
+ if (!evt_num_known) {
+ evt_num_known = true;
+ goto restart;
+ }
+ qsort(evt_list, evt_num, sizeof(char *), cmp_string);
+ evt_i = 0;
+ while (evt_i < evt_num) {
+ if (name_only) {
+ printf("%s ", evt_list[evt_i++]);
+ continue;
+ }
+ printf(" %-50s [%s]\n", evt_list[evt_i++],
+ event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+ }
+ if (evt_num)
+ printf("\n");
+
+out_free:
+ evt_num = evt_i;
+ for (evt_i = 0; evt_i < evt_num; evt_i++)
+ zfree(&evt_list[evt_i]);
+ zfree(&evt_list);
+ return;
+
+out_enomem:
+ printf("FATAL: not enough memory to print %s\n",
+ event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+ if (evt_list)
+ goto out_free;
}

/*
@@ -1230,8 +1280,17 @@ static bool is_event_supported(u8 type, unsigned config)

int print_hwcache_events(const char *event_glob, bool name_only)
{
- unsigned int type, op, i, printed = 0;
+ unsigned int type, op, i, evt_i = 0, evt_num = 0;
char name[64];
+ char **evt_list = NULL;
+ bool evt_num_known = false;
+
+restart:
+ if (evt_num_known) {
+ evt_list = zalloc(sizeof(char *) * evt_num);
+ if (!evt_list)
+ goto out_enomem;
+ }

for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
for (op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
@@ -1249,27 +1308,66 @@ int print_hwcache_events(const char *event_glob, bool name_only)
type | (op << 8) | (i << 16)))
continue;

- if (name_only)
- printf("%s ", name);
- else
- printf(" %-50s [%s]\n", name,
- event_type_descriptors[PERF_TYPE_HW_CACHE]);
- ++printed;
+ if (!evt_num_known) {
+ evt_num++;
+ continue;
+ }
+
+ evt_list[evt_i] = strdup(name);
+ if (evt_list[evt_i] == NULL)
+ goto out_enomem;
+ evt_i++;
}
}
}

- if (printed)
+ if (!evt_num_known) {
+ evt_num_known = true;
+ goto restart;
+ }
+ qsort(evt_list, evt_num, sizeof(char *), cmp_string);
+ evt_i = 0;
+ while (evt_i < evt_num) {
+ if (name_only) {
+ printf("%s ", evt_list[evt_i++]);
+ continue;
+ }
+ printf(" %-50s [%s]\n", evt_list[evt_i++],
+ event_type_descriptors[PERF_TYPE_HW_CACHE]);
+ }
+ if (evt_num)
printf("\n");
- return printed;
+
+out_free:
+ evt_num = evt_i;
+ for (evt_i = 0; evt_i < evt_num; evt_i++)
+ zfree(&evt_list[evt_i]);
+ zfree(&evt_list);
+ return evt_num;
+
+out_enomem:
+ printf("FATAL: not enough memory to print %s\n", event_type_descriptors[PERF_TYPE_HW_CACHE]);
+ if (evt_list)
+ goto out_free;
+ return evt_num;
}

void print_symbol_events(const char *event_glob, unsigned type,
struct event_symbol *syms, unsigned max,
bool name_only)
{
- unsigned i, printed = 0;
+ unsigned int i, evt_i = 0, evt_num = 0;
char name[MAX_NAME_LEN];
+ char **evt_list = NULL;
+ bool evt_num_known = false;
+
+restart:
+ if (evt_num_known) {
+ evt_list = zalloc(sizeof(char *) * evt_num);
+ if (!evt_list)
+ goto out_enomem;
+ syms -= max;
+ }

for (i = 0; i < max; i++, syms++) {

@@ -1281,23 +1379,49 @@ void print_symbol_events(const char *event_glob, unsigned type,
if (!is_event_supported(type, i))
continue;

- if (name_only) {
- printf("%s ", syms->symbol);
+ if (!evt_num_known) {
+ evt_num++;
continue;
}

- if (strlen(syms->alias))
+ if (!name_only && strlen(syms->alias))
snprintf(name, MAX_NAME_LEN, "%s OR %s", syms->symbol, syms->alias);
else
strncpy(name, syms->symbol, MAX_NAME_LEN);

- printf(" %-50s [%s]\n", name, event_type_descriptors[type]);
-
- printed++;
+ evt_list[evt_i] = strdup(name);
+ if (evt_list[evt_i] == NULL)
+ goto out_enomem;
+ evt_i++;
}

- if (printed)
+ if (!evt_num_known) {
+ evt_num_known = true;
+ goto restart;
+ }
+ qsort(evt_list, evt_num, sizeof(char *), cmp_string);
+ evt_i = 0;
+ while (evt_i < evt_num) {
+ if (name_only) {
+ printf("%s ", evt_list[evt_i++]);
+ continue;
+ }
+ printf(" %-50s [%s]\n", evt_list[evt_i++], event_type_descriptors[type]);
+ }
+ if (evt_num)
printf("\n");
+
+out_free:
+ evt_num = evt_i;
+ for (evt_i = 0; evt_i < evt_num; evt_i++)
+ zfree(&evt_list[evt_i]);
+ zfree(&evt_list);
+ return;
+
+out_enomem:
+ printf("FATAL: not enough memory to print %s\n", event_type_descriptors[type]);
+ if (evt_list)
+ goto out_free;
}

/*
--
1.8.5.5

2015-02-13 13:11:43

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 3/7] perf list: fix some inaccuracy problem when parsing the argument

If somebody happens to name an event with the beginning of 'tracepoint'
(e.g. tracepoint_foo), then it will never be showed with perf list
event_glob, thus we parse the argument 'tracepoint' more carefully for
accuracy.

Example
Before this patch:
$perf list tracepoint_foo:*

jbd2:jbd2_start_commit [Tracepoint event]
jbd2:jbd2_commit_locking [Tracepoint event]
jbd2:jbd2_run_stats [Tracepoint event]
block:block_rq_issue [Tracepoint event]
block:block_bio_complete [Tracepoint event]
block:block_bio_backmerge [Tracepoint event]
block:block_getrq [Tracepoint event]
... ...

As shown above, all of the tracepoint events are printed. In fact, the
command's real intention is to print the events of tracepoint_foo.

After this patch:
$perf list tracepoint_foo:*

tracepoint_foo:tp_foo_enter [Tracepoint event]
tracepoint_foo:tp_foo_exit [Tracepoint event]

As shown above, only the events of tracepoint_foo are printed.

Signed-off-by: Yunlong Song <[email protected]>
---
tools/perf/builtin-list.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index de5680e..003dec5 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -47,7 +47,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
}

for (i = 0; i < argc; ++i) {
- if (strncmp(argv[i], "tracepoint", 10) == 0)
+ if (strcmp(argv[i], "tracepoint") == 0)
print_tracepoint_events(NULL, NULL, false);
else if (strcmp(argv[i], "hw") == 0 ||
strcmp(argv[i], "hardware") == 0)
--
1.8.5.5

2015-02-13 13:10:18

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 4/7] perf list: fix a bug of segmentation fault

Fix the 'segmentation fault' bug of 'perf list --list-cmds', which
also happens in other cases (e.g. record, report ...). This bug happens
when there are no cmds to list at all.

Example
Before this patch:
$perf list --list-cmds
Segmentation fault
$

After this patch:
$perf list --list-cmds
$

As shown above, the result prints nothing rather than a segmentation
fault. The null result means 'perf list' has no cmds to display at this
time.

Signed-off-by: Yunlong Song <[email protected]>
---
tools/perf/util/parse-options.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 4a015f7..4ee9a86 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -510,8 +510,10 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
}
exit(130);
case PARSE_OPT_LIST_SUBCMDS:
- for (int i = 0; subcommands[i]; i++)
- printf("%s ", subcommands[i]);
+ if (subcommands) {
+ for (int i = 0; subcommands[i]; i++)
+ printf("%s ", subcommands[i]);
+ }
exit(130);
default: /* PARSE_OPT_UNKNOWN */
if (ctx.argv[0][1] == '-') {
--
1.8.5.5

2015-02-13 13:11:05

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt

Distinguish the output of 'perf list --list-opts' or 'perf --list-cmds'
with the next command prompt, which also happens in other cases (e.g.
record, report ...).

Example
Before this patch:
$perf list --list-opts
--raw-dump $ <-- the output and the next command prompt are at
the same line

After this patch:
$perf list --list-opts
--raw-dump
$ <-- the new line

Signed-off-by: Yunlong Song <[email protected]>
---
tools/perf/perf.c | 1 +
tools/perf/util/parse-options.c | 2 ++
2 files changed, 3 insertions(+)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 3700a7f..3effb95 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -222,6 +222,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
struct cmd_struct *p = commands+i;
printf("%s ", p->cmd);
}
+ putchar('\n');
exit(0);
} else if (!strcmp(cmd, "--debug")) {
if (*argc < 2) {
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 4ee9a86..b0ef2d8 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -508,12 +508,14 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
printf("--%s ", options->long_name);
options++;
}
+ putchar('\n');
exit(130);
case PARSE_OPT_LIST_SUBCMDS:
if (subcommands) {
for (int i = 0; subcommands[i]; i++)
printf("%s ", subcommands[i]);
}
+ putchar('\n');
exit(130);
default: /* PARSE_OPT_UNKNOWN */
if (ctx.argv[0][1] == '-') {
--
1.8.5.5

2015-02-13 13:11:25

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 6/7] perf list: extend raw-dump to certain kind of events

Extend 'perf list --raw-dump' to 'perf list --raw-dump [hw|sw|cache
|tracepoint|pmu|event_glob]' in order to show the raw-dump of a
certain kind of events rather than all of the events.

Example
Before this patch:
$perf list --raw-dump hw
branch-instructions branch-misses bus-cycles cache-misses
cache-references cpu-cycles instructions stalled-cycles-backend
stalled-cycles-frontend
alignment-faults context-switches cpu-clock cpu-migrations
emulation-faults major-faults minor-faults page-faults task-clock
...
...
writeback:writeback_thread_start writeback:writeback_thread_stop
writeback:writeback_wait_iff_congested
writeback:writeback_wake_background writeback:writeback_wake_thread

As shown above, all of the events are printed.

After this patch:
$perf list --raw-dump hw
branch-instructions branch-misses bus-cycles cache-misses
cache-references cpu-cycles instructions stalled-cycles-backend
stalled-cycles-frontend

As shown above, only the hw events are printed.

Signed-off-by: Yunlong Song <[email protected]>
---
tools/perf/Documentation/perf-list.txt | 6 ++++++
tools/perf/builtin-list.c | 21 ++++++++-------------
2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 3e2aec9..4692d27 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -127,6 +127,12 @@ To limit the list use:
One or more types can be used at the same time, listing the events for the
types specified.

+Support raw format:
+
+. '--raw-dump', shows the raw-dump of all the events.
+. '--raw-dump [hw|sw|cache|tracepoint|pmu|event_glob]', shows the raw-dump of
+ a certain kind of events.
+
SEE ALSO
--------
linkperf:perf-stat[1], linkperf:perf-top[1],
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 003dec5..b81a62c 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -36,38 +36,33 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

setup_pager();

- if (raw_dump) {
- print_events(NULL, true);
- return 0;
- }
-
if (argc == 0) {
- print_events(NULL, false);
+ print_events(NULL, raw_dump);
return 0;
}

for (i = 0; i < argc; ++i) {
if (strcmp(argv[i], "tracepoint") == 0)
- print_tracepoint_events(NULL, NULL, false);
+ print_tracepoint_events(NULL, NULL, raw_dump);
else if (strcmp(argv[i], "hw") == 0 ||
strcmp(argv[i], "hardware") == 0)
print_symbol_events(NULL, PERF_TYPE_HARDWARE,
- event_symbols_hw, PERF_COUNT_HW_MAX, false);
+ event_symbols_hw, PERF_COUNT_HW_MAX, raw_dump);
else if (strcmp(argv[i], "sw") == 0 ||
strcmp(argv[i], "software") == 0)
print_symbol_events(NULL, PERF_TYPE_SOFTWARE,
- event_symbols_sw, PERF_COUNT_SW_MAX, false);
+ event_symbols_sw, PERF_COUNT_SW_MAX, raw_dump);
else if (strcmp(argv[i], "cache") == 0 ||
strcmp(argv[i], "hwcache") == 0)
- print_hwcache_events(NULL, false);
+ print_hwcache_events(NULL, raw_dump);
else if (strcmp(argv[i], "pmu") == 0)
- print_pmu_events(NULL, false);
+ print_pmu_events(NULL, raw_dump);
else {
char *sep = strchr(argv[i], ':'), *s;
int sep_idx;

if (sep == NULL) {
- print_events(argv[i], false);
+ print_events(argv[i], raw_dump);
continue;
}
sep_idx = sep - argv[i];
@@ -76,7 +71,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
return -1;

s[sep_idx] = '\0';
- print_tracepoint_events(s, s + sep_idx + 1, false);
+ print_tracepoint_events(s, s + sep_idx + 1, raw_dump);
free(s);
}
}
--
1.8.5.5

2015-02-13 13:11:41

by Yunlong Song

[permalink] [raw]
Subject: [PATCH 7/7] perf list: place the guiding text in its right position

The guiding text 'List of pre-defined events (to be used in -e):' is
placed in an improper function, which causes an abnormal output, e.g.
'perf list hw' shows no guiding text at all, and 'perf list hw
L1-dcache*' shows the guiding text incorrectly in the middle of the
output.

Example
Before this patch:
$perf list hw L1-dcache*

branch-instructions OR branches [Hardware event]
branch-misses [Hardware event]
bus-cycles [Hardware event]
cache-misses [Hardware event]
cache-references [Hardware event]
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
stalled-cycles-backend OR idle-cycles-backend [Hardware event]
stalled-cycles-frontend OR idle-cycles-frontend [Hardware event]

List of pre-defined events (to be used in -e): <-- incorrect position
L1-dcache-load-misses [Hardware cache event]
L1-dcache-loads [Hardware cache event]
L1-dcache-prefetch-misses [Hardware cache event]
L1-dcache-prefetches [Hardware cache event]
L1-dcache-store-misses [Hardware cache event]
L1-dcache-stores [Hardware cache event]

After this patch:
$perf list hw L1-dcache*

List of pre-defined events (to be used in -e): <-- correct position

branch-instructions OR branches [Hardware event]
branch-misses [Hardware event]
bus-cycles [Hardware event]
cache-misses [Hardware event]
cache-references [Hardware event]
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
stalled-cycles-backend OR idle-cycles-backend [Hardware event]
stalled-cycles-frontend OR idle-cycles-frontend [Hardware event]

L1-dcache-load-misses [Hardware cache event]
L1-dcache-loads [Hardware cache event]
L1-dcache-prefetch-misses [Hardware cache event]
L1-dcache-prefetches [Hardware cache event]
L1-dcache-store-misses [Hardware cache event]
L1-dcache-stores [Hardware cache event]

Signed-off-by: Yunlong Song <[email protected]>
---
tools/perf/builtin-list.c | 3 +++
tools/perf/util/parse-events.c | 5 -----
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index b81a62c..af5bd05 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -36,6 +36,9 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

setup_pager();

+ if (!raw_dump)
+ printf("\nList of pre-defined events (to be used in -e):\n\n");
+
if (argc == 0) {
print_events(NULL, raw_dump);
return 0;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index cf35b0a..df1994c 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1429,11 +1429,6 @@ out_enomem:
*/
void print_events(const char *event_glob, bool name_only)
{
- if (!name_only) {
- printf("\n");
- printf("List of pre-defined events (to be used in -e):\n");
- }
-
print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
event_symbols_hw, PERF_COUNT_HW_MAX, name_only);

--
1.8.5.5

2015-02-13 14:45:25

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly

Em Fri, Feb 13, 2015 at 09:11:50PM +0800, Yunlong Song escreveu:
> return;
> +
> + if (evt_num_known) {
> + evt_list = zalloc(sizeof(char *) * evt_num);
> + if (!evt_list)
> + goto out_enomem;
> + }

I am fixing this up this time, but please use either
scripts/checkpatch.pl or enable the pre commit hooks in your git
configuration:

chmod +x .git/hooks/*

So that those spaces at the beginning of the line, indentation artifacts
don't get submitted, ok?

- Arnaldo

2015-02-13 14:48:37

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly

Em Fri, Feb 13, 2015 at 11:45:46AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Feb 13, 2015 at 09:11:50PM +0800, Yunlong Song escreveu:
> > return;
> > +
> > + if (evt_num_known) {
> > + evt_list = zalloc(sizeof(char *) * evt_num);
> > + if (!evt_list)
> > + goto out_enomem;
> > + }
>
> I am fixing this up this time, but please use either
> scripts/checkpatch.pl or enable the pre commit hooks in your git
> configuration:
>
> chmod +x .git/hooks/*
>
> So that those spaces at the beginning of the line, indentation artifacts
> don't get submitted, ok?

Well, it doesn't apply to my perf/core branch, wait a bit till I send a
new pull request to Ingo and try again, please check which csets from
this patchset got applied, at least one so far was.

- Arnaldo

2015-02-13 14:52:20

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt

Em Fri, Feb 13, 2015 at 09:11:53PM +0800, Yunlong Song escreveu:
> Distinguish the output of 'perf list --list-opts' or 'perf --list-cmds'
> with the next command prompt, which also happens in other cases (e.g.
> record, report ...).
>
> Example
> Before this patch:
> $perf list --list-opts
> --raw-dump $ <-- the output and the next command prompt are at
> the same line
>
> After this patch:
> $perf list --list-opts
> --raw-dump
> $ <-- the new line

Unsure about this one, IIRC this --list-opts thing was added to be used
together with shell autocompletion, have you checked that this extra
newline is OK with that?

Please try searching for the changeset that introduced --list-opts to
get info about how to test this,

Thanks,

- Arnaldo

2015-02-13 14:55:32

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 6/7] perf list: extend raw-dump to certain kind of events

Em Fri, Feb 13, 2015 at 09:11:54PM +0800, Yunlong Song escreveu:
> Extend 'perf list --raw-dump' to 'perf list --raw-dump [hw|sw|cache
> |tracepoint|pmu|event_glob]' in order to show the raw-dump of a
> certain kind of events rather than all of the events.

Again, please check that this doesn't break shell autocompletion, I
haven't checked, do you keep the existing behaviour, i.e. if does
--raw-dump without an argument shows all the events?

- Arnaldo

> Example
> Before this patch:
> $perf list --raw-dump hw
> branch-instructions branch-misses bus-cycles cache-misses
> cache-references cpu-cycles instructions stalled-cycles-backend
> stalled-cycles-frontend
> alignment-faults context-switches cpu-clock cpu-migrations
> emulation-faults major-faults minor-faults page-faults task-clock
> ...
> ...
> writeback:writeback_thread_start writeback:writeback_thread_stop
> writeback:writeback_wait_iff_congested
> writeback:writeback_wake_background writeback:writeback_wake_thread
>
> As shown above, all of the events are printed.
>
> After this patch:
> $perf list --raw-dump hw
> branch-instructions branch-misses bus-cycles cache-misses
> cache-references cpu-cycles instructions stalled-cycles-backend
> stalled-cycles-frontend
>
> As shown above, only the hw events are printed.
>
> Signed-off-by: Yunlong Song <[email protected]>
> ---
> tools/perf/Documentation/perf-list.txt | 6 ++++++
> tools/perf/builtin-list.c | 21 ++++++++-------------
> 2 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> index 3e2aec9..4692d27 100644
> --- a/tools/perf/Documentation/perf-list.txt
> +++ b/tools/perf/Documentation/perf-list.txt
> @@ -127,6 +127,12 @@ To limit the list use:
> One or more types can be used at the same time, listing the events for the
> types specified.
>
> +Support raw format:
> +
> +. '--raw-dump', shows the raw-dump of all the events.
> +. '--raw-dump [hw|sw|cache|tracepoint|pmu|event_glob]', shows the raw-dump of
> + a certain kind of events.
> +
> SEE ALSO
> --------
> linkperf:perf-stat[1], linkperf:perf-top[1],
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 003dec5..b81a62c 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -36,38 +36,33 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
>
> setup_pager();
>
> - if (raw_dump) {
> - print_events(NULL, true);
> - return 0;
> - }
> -
> if (argc == 0) {
> - print_events(NULL, false);
> + print_events(NULL, raw_dump);
> return 0;
> }
>
> for (i = 0; i < argc; ++i) {
> if (strcmp(argv[i], "tracepoint") == 0)
> - print_tracepoint_events(NULL, NULL, false);
> + print_tracepoint_events(NULL, NULL, raw_dump);
> else if (strcmp(argv[i], "hw") == 0 ||
> strcmp(argv[i], "hardware") == 0)
> print_symbol_events(NULL, PERF_TYPE_HARDWARE,
> - event_symbols_hw, PERF_COUNT_HW_MAX, false);
> + event_symbols_hw, PERF_COUNT_HW_MAX, raw_dump);
> else if (strcmp(argv[i], "sw") == 0 ||
> strcmp(argv[i], "software") == 0)
> print_symbol_events(NULL, PERF_TYPE_SOFTWARE,
> - event_symbols_sw, PERF_COUNT_SW_MAX, false);
> + event_symbols_sw, PERF_COUNT_SW_MAX, raw_dump);
> else if (strcmp(argv[i], "cache") == 0 ||
> strcmp(argv[i], "hwcache") == 0)
> - print_hwcache_events(NULL, false);
> + print_hwcache_events(NULL, raw_dump);
> else if (strcmp(argv[i], "pmu") == 0)
> - print_pmu_events(NULL, false);
> + print_pmu_events(NULL, raw_dump);
> else {
> char *sep = strchr(argv[i], ':'), *s;
> int sep_idx;
>
> if (sep == NULL) {
> - print_events(argv[i], false);
> + print_events(argv[i], raw_dump);
> continue;
> }
> sep_idx = sep - argv[i];
> @@ -76,7 +71,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
> return -1;
>
> s[sep_idx] = '\0';
> - print_tracepoint_events(s, s + sep_idx + 1, false);
> + print_tracepoint_events(s, s + sep_idx + 1, raw_dump);
> free(s);
> }
> }
> --
> 1.8.5.5

2015-02-15 10:03:20

by Yunlong Song

[permalink] [raw]
Subject: Re: [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt

On 2015/2/13 22:52, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 13, 2015 at 09:11:53PM +0800, Yunlong Song escreveu:
>> Distinguish the output of 'perf list --list-opts' or 'perf --list-cmds'
>> with the next command prompt, which also happens in other cases (e.g.
>> record, report ...).
>>
>> Example
>> Before this patch:
>> $perf list --list-opts
>> --raw-dump $ <-- the output and the next command prompt are at
>> the same line
>>
>> After this patch:
>> $perf list --list-opts
>> --raw-dump
>> $ <-- the new line
>
> Unsure about this one, IIRC this --list-opts thing was added to be used
> together with shell autocompletion, have you checked that this extra
> newline is OK with that?
>
> Please try searching for the changeset that introduced --list-opts to
> get info about how to test this,
>
> Thanks,
>
> - Arnaldo
>
> .
>

Already checked, it's really OK.

--
Thanks,
Yunlong Song

2015-02-15 10:05:54

by Yunlong Song

[permalink] [raw]
Subject: Re: [PATCH 6/7] perf list: extend raw-dump to certain kind of events

On 2015/2/13 22:55, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 13, 2015 at 09:11:54PM +0800, Yunlong Song escreveu:
>> Extend 'perf list --raw-dump' to 'perf list --raw-dump [hw|sw|cache
>> |tracepoint|pmu|event_glob]' in order to show the raw-dump of a
>> certain kind of events rather than all of the events.
>
> Again, please check that this doesn't break shell autocompletion, I
> haven't checked, do you keep the existing behaviour, i.e. if does
> --raw-dump without an argument shows all the events?
>
> - Arnaldo
>

Already checked, it's really OK. It doesn't break shell autocompletion,
it keeps the existing behaviour (--raw-dump without an argument shows
all the events).

--
Thanks,
Yunlong Song

2015-02-15 10:11:38

by Yunlong Song

[permalink] [raw]
Subject: Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly

On 2015/2/13 22:45, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 13, 2015 at 09:11:50PM +0800, Yunlong Song escreveu:
>> return;
>> +
>> + if (evt_num_known) {
>> + evt_list = zalloc(sizeof(char *) * evt_num);
>> + if (!evt_list)
>> + goto out_enomem;
>> + }
>
> I am fixing this up this time, but please use either
> scripts/checkpatch.pl or enable the pre commit hooks in your git
> configuration:
>
> chmod +x .git/hooks/*
>
> So that those spaces at the beginning of the line, indentation artifacts
> don't get submitted, ok?
>
> - Arnaldo
>
>

Already fix it, please see PATCH v2 which I already resent.

--
Thanks,
Yunlong Song

2015-02-15 10:20:14

by Yunlong Song

[permalink] [raw]
Subject: Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly

On 2015/2/13 22:49, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 13, 2015 at 11:45:46AM -0300, Arnaldo Carvalho de Melo escreveu:

>
> Well, it doesn't apply to my perf/core branch, wait a bit till I send a
> new pull request to Ingo and try again, please check which csets from
> this patchset got applied, at least one so far was.
>
> - Arnaldo
>
>

In this patchset, I use the linux mainline:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master

In my new patchset PATCH v2, I use your git repo:
https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/ perf/core

See http://lkml.org/lkml/2015/2/15/75 for PATCH v2

--
Thanks,
Yunlong Song

Subject: [tip:perf/core] perf tools: Fix a bug of segmentation fault

Commit-ID: 3a03005ff9445834f3d3b577a11bcbdbdf7a89cf
Gitweb: http://git.kernel.org/tip/3a03005ff9445834f3d3b577a11bcbdbdf7a89cf
Author: Yunlong Song <[email protected]>
AuthorDate: Fri, 13 Feb 2015 21:11:52 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 13 Feb 2015 11:38:43 -0300

perf tools: Fix a bug of segmentation fault

Fix the 'segmentation fault' bug of 'perf list --list-cmds', which also
happens in other cases (e.g. record, report ...). This bug happens when
there are no cmds to list at all.

Example:

Before this patch:

$ perf list --list-cmds
Segmentation fault
$

After this patch:
$ perf list --list-cmds
$

As shown above, the result prints nothing rather than a segmentation
fault. The null result means 'perf list' has no cmds to display at this
time.

Signed-off-by: Yunlong Song <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Wang Nan <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/parse-options.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 4a015f7..4ee9a86 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -510,8 +510,10 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
}
exit(130);
case PARSE_OPT_LIST_SUBCMDS:
- for (int i = 0; subcommands[i]; i++)
- printf("%s ", subcommands[i]);
+ if (subcommands) {
+ for (int i = 0; subcommands[i]; i++)
+ printf("%s ", subcommands[i]);
+ }
exit(130);
default: /* PARSE_OPT_UNKNOWN */
if (ctx.argv[0][1] == '-') {

Subject: [tip:perf/core] perf list: Place the header text in its right position

Commit-ID: 619a303c1b8bd22abc549477d038ef9b5c1fe1bd
Gitweb: http://git.kernel.org/tip/619a303c1b8bd22abc549477d038ef9b5c1fe1bd
Author: Yunlong Song <[email protected]>
AuthorDate: Fri, 13 Feb 2015 21:11:55 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 13 Feb 2015 11:57:50 -0300

perf list: Place the header text in its right position

The hearer text 'List of pre-defined events (to be used in -e):' is
placed in an improper function, which causes an abnormal output, e.g.
'perf list hw' shows no guiding text at all, and 'perf list hw
L1-dcache*' shows the guiding text incorrectly in the middle of the
output.

Example
Before this patch:

$ perf list hw L1-dcache*

branch-instructions OR branches [Hardware event]
branch-misses [Hardware event]
bus-cycles [Hardware event]
cache-misses [Hardware event]
cache-references [Hardware event]
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
stalled-cycles-backend OR idle-cycles-backend [Hardware event]
stalled-cycles-frontend OR idle-cycles-frontend [Hardware event]

List of pre-defined events (to be used in -e): <-- incorrect position
L1-dcache-load-misses [Hardware cache event]
L1-dcache-loads [Hardware cache event]
L1-dcache-prefetch-misses [Hardware cache event]
L1-dcache-prefetches [Hardware cache event]
L1-dcache-store-misses [Hardware cache event]
L1-dcache-stores [Hardware cache event]

After this patch:

$ perf list hw L1-dcache*

List of pre-defined events (to be used in -e): <-- correct position

branch-instructions OR branches [Hardware event]
branch-misses [Hardware event]
bus-cycles [Hardware event]
cache-misses [Hardware event]
cache-references [Hardware event]
cpu-cycles OR cycles [Hardware event]
instructions [Hardware event]
stalled-cycles-backend OR idle-cycles-backend [Hardware event]
stalled-cycles-frontend OR idle-cycles-frontend [Hardware event]

L1-dcache-load-misses [Hardware cache event]
L1-dcache-loads [Hardware cache event]
L1-dcache-prefetch-misses [Hardware cache event]
L1-dcache-prefetches [Hardware cache event]
L1-dcache-store-misses [Hardware cache event]
L1-dcache-stores [Hardware cache event]

Signed-off-by: Yunlong Song <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Wang Nan <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-list.c | 3 +++
tools/perf/util/parse-events.c | 5 -----
2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 198f3c3..ad8018e 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -41,6 +41,9 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
return 0;
}

+ if (!raw_dump)
+ printf("\nList of pre-defined events (to be used in -e):\n\n");
+
if (argc == 0) {
print_events(NULL, false);
return 0;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index ecf069b..109ba5c 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1319,11 +1319,6 @@ static void print_symbol_events(const char *event_glob, unsigned type,
*/
void print_events(const char *event_glob, bool name_only)
{
- if (!name_only) {
- printf("\n");
- printf("List of pre-defined events (to be used in -e):\n");
- }
-
print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
event_symbols_hw, PERF_COUNT_HW_MAX, name_only);