[PATCH 3/8] perf tools: Make the print_pmu_events funtion to return unsigned int.

[Dongsheng Yang]

Sometimes we need to know how many events have been printed in print_pmu_events.

This patch make this function to return the number of events we have printed.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---
tools/perf/util/pmu.c | 20 ++++++++++----------
tools/perf/util/pmu.h | 2 +-
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 56fc10a..99b3a64 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -711,12 +711,12 @@ static int cmp_string(const void *a, const void *b)
return strcmp(*as, *bs);
}

-void print_pmu_events(const char *event_glob, bool name_only)
+unsigned int print_pmu_events(const char *event_glob, bool name_only)
{
struct perf_pmu *pmu;
struct perf_pmu_alias *alias;
char buf[1024];
- int printed = 0;
+ unsigned int count = 0;
int len, j;
char **aliases;

@@ -727,7 +727,7 @@ void print_pmu_events(const char *event_glob, bool name_only)
len++;
aliases = malloc(sizeof(char *) * len);
if (!aliases)
- return;
+ return count;
pmu = NULL;
j = 0;
while ((pmu = perf_pmu__scan(pmu)) != NULL)
@@ -750,17 +750,17 @@ void print_pmu_events(const char *event_glob, bool name_only)
len = j;
qsort(aliases, len, sizeof(char *), cmp_string);
for (j = 0; j < len; j++) {
- if (name_only) {
- printf("%s ", aliases[j]);
- continue;
- }
- printf(" %-50s [Kernel PMU event]\n", aliases[j]);
+ if (name_only)
+ printf("%s\n", aliases[j]);
+ else
+ printf(" %-50s [Kernel PMU event]\n", aliases[j]);
free(aliases[j]);
- printed++;
+ count++;
}
- if (printed)
+ if (count)
printf("\n");
free(aliases);
+ return count;
}

bool pmu_have_event(const char *pname, const char *name)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 9183380..2987fe2 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -42,7 +42,7 @@ int perf_pmu__format_parse(char *dir, struct list_head *head);

struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu);

-void print_pmu_events(const char *event_glob, bool name_only);
+unsigned int print_pmu_events(const char *event_glob, bool name_only);
bool pmu_have_event(const char *pname, const char *name);

int perf_pmu__test(void);
--
1.8.2.1

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

[Dongsheng Yang]

Ramkumar reported that perf list --raw-dump was broken by 44d742e.
Fix by making raw-dump a proper argument.

Signed-off-by: David Ahern <dsa...@gmail.com>
Signed-off-by: Ramkumar Ramachandra <arta...@gmail.com>
Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
Cc: Ramkumar Ramachandra <arta...@gmail.com>
Cc: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/builtin-list.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index fed6792..ec90d0a 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,8 +19,10 @@
int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
{
int i;
+ bool raw_dump = false;

unsigned int count = 0;

const struct option list_options[] = {
+ OPT_BOOLEAN(0, "raw-dump", &raw_dump, "raw dump for completion"),
OPT_END()
};
const char * const list_usage[] = {
@@ -34,7 +36,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
setup_pager();

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

@@ -42,26 +44,24 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
if (i)
putchar('\n');
if (strncmp(argv[i], "tracepoint", 10) == 0)
- count += print_tracepoint_events(NULL, NULL, false);
+ count += print_tracepoint_events(NULL, NULL, raw_dump);
else if (strcmp(argv[i], "hw") == 0 ||
strcmp(argv[i], "hardware") == 0)
- count += print_events_type(PERF_TYPE_HARDWARE);
+ count += print_events_type(PERF_TYPE_HARDWARE, raw_dump);
else if (strcmp(argv[i], "sw") == 0 ||
strcmp(argv[i], "software") == 0)
- count += print_events_type(PERF_TYPE_SOFTWARE);
+ count += print_events_type(PERF_TYPE_SOFTWARE, raw_dump);
else if (strcmp(argv[i], "cache") == 0 ||
strcmp(argv[i], "hwcache") == 0)
- count += print_hwcache_events(NULL, false);
+ count += print_hwcache_events(NULL, raw_dump);
else if (strcmp(argv[i], "pmu") == 0)
- count += print_pmu_events(NULL, false);
- else if (strcmp(argv[i], "--raw-dump") == 0)
- count += print_events(NULL, true);
+ count += print_pmu_events(NULL, raw_dump);
else {
char *sep = strchr(argv[i], ':'), *s;
int sep_idx;

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

s[sep_idx] = '\0';
- if (!(count += print_tracepoint_events(s, s + sep_idx + 1, false)))
+ if (!(count += print_tracepoint_events(s, s + sep_idx + 1, raw_dump)))
goto err_out;
free(s);

[Dongsheng Yang]

Example:
# perf list test

List of pre-defined events (to be used in -e):
# echo $?
0

Verification:
# perf list test

No event for test.
Usage:
perf list [hw|sw|cache|tracepoint|pmu|event_glob]
# echo $?
255

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/builtin-list.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 011195e..fed6792 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,6 +19,7 @@

int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
{
int i;

+ unsigned int count = 0;

const struct option list_options[] = {

OPT_END()
};
@@ -41,26 +42,27 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

if (i)
putchar('\n');
if (strncmp(argv[i], "tracepoint", 10) == 0)

- print_tracepoint_events(NULL, NULL, false);
+ count += print_tracepoint_events(NULL, NULL, false);

else if (strcmp(argv[i], "hw") == 0 ||
strcmp(argv[i], "hardware") == 0)

- print_events_type(PERF_TYPE_HARDWARE);
+ count += print_events_type(PERF_TYPE_HARDWARE);

else if (strcmp(argv[i], "sw") == 0 ||
strcmp(argv[i], "software") == 0)

- print_events_type(PERF_TYPE_SOFTWARE);
+ count += print_events_type(PERF_TYPE_SOFTWARE);

else if (strcmp(argv[i], "cache") == 0 ||
strcmp(argv[i], "hwcache") == 0)

- print_hwcache_events(NULL, false);
+ count += print_hwcache_events(NULL, false);

else if (strcmp(argv[i], "pmu") == 0)

- print_pmu_events(NULL, false);
+ count += print_pmu_events(NULL, false);

else if (strcmp(argv[i], "--raw-dump") == 0)

- print_events(NULL, true);
+ count += print_events(NULL, true);

else {
char *sep = strchr(argv[i], ':'), *s;
int sep_idx;

if (sep == NULL) {

- print_events(argv[i], false);
+ if(!(count += print_events(argv[i], false)))
+ goto err_out;
continue;
}
sep_idx = sep - argv[i];
@@ -69,9 +71,16 @@ 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);
+ if (!(count += print_tracepoint_events(s, s + sep_idx + 1, false)))
+ goto err_out;
free(s);
}
}
+
return 0;
+
+err_out:
+ pr_info("\nNo event for %s.\n", argv[i]);
+ pr_info("Usage:\n\t%s\n", list_usage[0]);
+ return -1;

[Dongsheng Yang]

As what the usage showing, we can input a event_glob as an argument to list
all symbols. But it does not work well right now.

Example:
# perf list kvmmmu

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

This patch make it work well as follows.

Verification:
# perf list kvmmmu

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

kvmmmu:kvm_mmu_pagetable_walk [Tracepoint event]
kvmmmu:kvm_mmu_paging_element [Tracepoint event]
kvmmmu:kvm_mmu_set_accessed_bit [Tracepoint event]
kvmmmu:kvm_mmu_set_dirty_bit [Tracepoint event]
kvmmmu:kvm_mmu_walker_error [Tracepoint event]
kvmmmu:kvm_mmu_get_page [Tracepoint event]
kvmmmu:kvm_mmu_sync_page [Tracepoint event]
kvmmmu:kvm_mmu_unsync_page [Tracepoint event]
kvmmmu:kvm_mmu_prepare_zap_page [Tracepoint event]
kvmmmu:mark_mmio_spte [Tracepoint event]
kvmmmu:handle_mmio_page_fault [Tracepoint event]
kvmmmu:fast_page_fault [Tracepoint event]

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 094c28b..d887ea4 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1236,6 +1236,8 @@ void print_events(const char *event_glob, bool name_only)

print_pmu_events(event_glob, name_only);

+ print_tracepoint_events(event_glob, NULL, name_only);
+
if (event_glob != NULL)
return;

@@ -1254,8 +1256,6 @@ void print_events(const char *event_glob, bool name_only)
event_type_descriptors[PERF_TYPE_BREAKPOINT]);
printf("\n");
}
-
- print_tracepoint_events(NULL, NULL, name_only);
}

int parse_events__is_hardcoded_term(struct parse_events_term *term)

[Dongsheng Yang]

As we use PARSE_OPT_STOP_AT_NON_OPTION option in parse_option(), if --raw-dump
is not the first argument, it will be treat as an argument rather than option.

Example:
# ./perf list kvmmmu --raw-dump

kvmmmu:kvm_mmu_pagetable_walk [Tracepoint event]
kvmmmu:kvm_mmu_paging_element [Tracepoint event]
kvmmmu:kvm_mmu_set_accessed_bit [Tracepoint event]
kvmmmu:kvm_mmu_set_dirty_bit [Tracepoint event]
kvmmmu:kvm_mmu_walker_error [Tracepoint event]
kvmmmu:kvm_mmu_get_page [Tracepoint event]
kvmmmu:kvm_mmu_sync_page [Tracepoint event]
kvmmmu:kvm_mmu_unsync_page [Tracepoint event]
kvmmmu:kvm_mmu_prepare_zap_page [Tracepoint event]
kvmmmu:mark_mmio_spte [Tracepoint event]
kvmmmu:handle_mmio_page_fault [Tracepoint event]
kvmmmu:fast_page_fault [Tracepoint event]

This patch replace PARSE_OPT_STOP_AT_NON_OPTION as 0, then we can parse it even if
it is not the first argument.

Verification:
# ./perf list kvmmmu --raw-dump
kvmmmu:kvm_mmu_pagetable_walk kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit kvmmmu:kvm_mmu_set_dirty_b
it kvmmmu:kvm_mmu_walker_error kvmmmu:kv

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/builtin-list.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index ec90d0a..9430666 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -30,8 +30,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
NULL
};

- argc = parse_options(argc, argv, list_options, list_usage,
- PARSE_OPT_STOP_AT_NON_OPTION);
+ argc = parse_options(argc, argv, list_options, list_usage, 0);

setup_pager();

[Dongsheng Yang]

Currently, the most of print_XXX_event() functions are returning void. Then we can
not know whether the printing work is completed well.

This patch change the return type to unsigned int, it means the count of events we
have printed in print_XXX_event() function.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 79 ++++++++++++++++++++++++------------------
tools/perf/util/parse-events.h | 8 ++---
2 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d887ea4..eb0d0eef 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1000,22 +1000,23 @@ static const char * const event_type_descriptors[] = {
* Print the events from <debugfs_mount_point>/tracing/events
*/

-void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
- bool name_only)
+unsigned int print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+ bool name_only)
{
DIR *sys_dir, *evt_dir;
struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
char evt_path[MAXPATHLEN];
char dir_path[MAXPATHLEN];

+ unsigned int count = 0;

if (debugfs_valid_mountpoint(tracing_events_path)) {
printf(" [ Tracepoints not available: %s ]\n", strerror(errno));

- return;
+ return count;
}

sys_dir = opendir(tracing_events_path);
if (!sys_dir)

- return;
+ return count;

for_each_subsystem(sys_dir, sys_dirent, sys_next) {
if (subsys_glob != NULL &&
@@ -1035,6 +1036,7 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,

if (name_only) {
printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);
+ count++;
continue;
}

@@ -1042,10 +1044,13 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
sys_dirent.d_name, evt_dirent.d_name);
printf(" %-50s [%s]\n", evt_path,
event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+ count++;
}
closedir(evt_dir);
}
closedir(sys_dir);
+
+ return count;
}

/*
@@ -1116,11 +1121,12 @@ static bool is_event_supported(u8 type, unsigned config)
return ret;
}

-static void __print_events_type(u8 type, struct event_symbol *syms,
- unsigned max)
+static unsigned int __print_events_type(u8 type, struct event_symbol *syms,
+ unsigned max)
{
char name[64];
unsigned i;

+ unsigned int count = 0;

for (i = 0; i < max ; i++, syms++) {
if (!is_event_supported(type, i))
@@ -1133,20 +1139,23 @@ static void __print_events_type(u8 type, struct event_symbol *syms,
snprintf(name, sizeof(name), "%s", syms->symbol);

printf(" %-50s [%s]\n", name, event_type_descriptors[type]);
+ count++;
}
+
+ return count;
}

-void print_events_type(u8 type)
+unsigned int print_events_type(u8 type)
{
if (type == PERF_TYPE_SOFTWARE)
- __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
+ return __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
else
- __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
+ return __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
}

-int print_hwcache_events(const char *event_glob, bool name_only)
+unsigned int print_hwcache_events(const char *event_glob, bool name_only)
{
- unsigned int type, op, i, printed = 0;
+ unsigned int type, op, i, count = 0;
char name[64];

for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
@@ -1170,21 +1179,22 @@ int print_hwcache_events(const char *event_glob, bool name_only)
else
printf(" %-50s [%s]\n", name,
event_type_descriptors[PERF_TYPE_HW_CACHE]);
- ++printed;

+ count++;
}
}
}

- if (printed)
+ if (count)
printf("\n");

- return printed;
+ return count;
}

-static void print_symbol_events(const char *event_glob, unsigned type,
- struct event_symbol *syms, unsigned max,
- bool name_only)
+static unsigned print_symbol_events(const char *event_glob, unsigned type,
+ struct event_symbol *syms, unsigned max,
+ bool name_only)
{
- unsigned i, printed = 0;
+ unsigned i;

+ unsigned int count = 0;

char name[MAX_NAME_LEN];

for (i = 0; i < max; i++, syms++) {
@@ -1199,6 +1209,7 @@ static void print_symbol_events(const char *event_glob, unsigned type,

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

@@ -1209,37 +1220,37 @@ static void print_symbol_events(const char *event_glob, unsigned type,

printf(" %-50s [%s]\n", name, event_type_descriptors[type]);

- printed++;
+ count++;
}

- if (printed)
+ if (count)
printf("\n");

+ return count;
}

/*
* Print the help text for the event symbols:
*/
-void print_events(const char *event_glob, bool name_only)
+unsigned int 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");
- }

+ unsigned int count = 0;

- print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
- event_symbols_hw, PERF_COUNT_HW_MAX, name_only);
+ count += print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
+ event_symbols_hw, PERF_COUNT_HW_MAX,
+ name_only);

- print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
- event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
+ count += print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
+ event_symbols_sw, PERF_COUNT_SW_MAX,
+ name_only);

- print_hwcache_events(event_glob, name_only);
+ count += print_hwcache_events(event_glob, name_only);

- print_pmu_events(event_glob, name_only);
+ count += print_pmu_events(event_glob, name_only);

- print_tracepoint_events(event_glob, NULL, name_only);
+ count += print_tracepoint_events(event_glob, NULL, name_only);

if (event_glob != NULL)

- return;
+ return count;

if (!name_only) {
printf(" %-50s [%s]\n",
@@ -1256,6 +1267,8 @@ void print_events(const char *event_glob, bool name_only)

event_type_descriptors[PERF_TYPE_BREAKPOINT]);
printf("\n");
}

+
+ return count;
}

int parse_events__is_hardcoded_term(struct parse_events_term *term)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index f1cb4c4..bb7d674 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -101,11 +101,11 @@ void parse_events_update_lists(struct list_head *list_event,
struct list_head *list_all);
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);
-void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+unsigned int print_events(const char *event_glob, bool name_only);
+unsigned int print_events_type(u8 type);
+unsigned int 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);
+unsigned int print_hwcache_events(const char *event_glob, bool name_only);
extern int is_valid_tracepoint(const char *event_string);

extern int valid_debugfs_mount(const char *debugfs);

[Dongsheng Yang]

As we make --raw-dump work as a proper option, we need to
make the output of it more readable.

Example:
# ./perf list kvmmmu --raw-dump

kvmmmu:kvm_mmu_pagetable_walk kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit kvmmmu:kvm_mmu_set_dirty_b
it kvmmmu:kvm_mmu_walker_error kvmmmu:kv

Verification:
# ./perf list kvmmmu --raw-dump
kvmmmu:kvm_mmu_pagetable_walk
kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit
kvmmmu:kvm_mmu_set_dirty_bit
kvmmmu:kvm_mmu_walker_error

kvmmmu:kvm_mmu_get_page
kvmmmu:kvm_mmu_sync_page
kvmmmu:kvm_mmu_unsync_page
kvmmmu:kvm_mmu_prepare_zap_page
kvmmmu:mark_mmio_spte
kvmmmu:handle_mmio_page_fault
kvmmmu:fast_page_fault

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 9d7580b..ab2d146 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1035,7 +1035,7 @@ unsigned int print_tracepoint_events(const char *subsys_glob, const char *event_
continue;

if (name_only) {
- printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);
+ printf("%s:%s\n", sys_dirent.d_name, evt_dirent.d_name);
count++;
continue;
}
@@ -1177,7 +1177,7 @@ unsigned int print_hwcache_events(const char *event_glob, bool name_only)
continue;

if (name_only)
- printf("%s ", name);
+ printf("%s\n", name);

else
printf(" %-50s [%s]\n", name,
event_type_descriptors[PERF_TYPE_HW_CACHE]);

@@ -1210,7 +1210,7 @@ static unsigned print_symbol_events(const char *event_glob, unsigned type,
continue;

if (name_only) {
- printf("%s ", syms->symbol);
+ printf("%s\n", syms->symbol);
count++;
continue;

[Dongsheng Yang]

Hi Arnaldo,
This patchset is an enhancement for perf list command.

The patch [6/8] is from work of Ramkumar and David at
52A8D2DA...@gmail.com, so I add signed-off by them.

Dongsheng Yang (8):
perf tools: Fix bug in 'perf list event_glob'.
perf tools: Make the all print_xxx_event functions to return unsigned
int.
perf tools: Make the print_pmu_events funtion to return unsigned int.
perf tools: Improve the message of perf list for unexpected input.
perf tools: Add support of name_only for print_events_type() function.
perf tools: Make --raw-dump work as a proper option for perf list.
perf tools: Fix bug when --raw-dump is not the first arguement for
perf list.
perf tools: Enhancement for perf list with --raw-dump.

tools/perf/builtin-list.c | 32 +++++++++------
tools/perf/util/parse-events.c | 91 ++++++++++++++++++++++++------------------
tools/perf/util/parse-events.h | 8 ++--
tools/perf/util/pmu.c | 20 +++++-----
tools/perf/util/pmu.h | 2 +-
5 files changed, 88 insertions(+), 65 deletions(-)

[Dongsheng Yang]

The all print_xxx_event() functions are supporting name_only argument except
print_event_type().

This patch add an argument of name_only for print_events_type() function.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 14 ++++++++------
tools/perf/util/parse-events.h | 2 +-
2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index eb0d0eef..9d7580b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1122,7 +1122,7 @@ static bool is_event_supported(u8 type, unsigned config)

}

static unsigned int __print_events_type(u8 type, struct event_symbol *syms,

- unsigned max)
+ unsigned max, bool name_only)

{
char name[64];
unsigned i;

@@ -1137,20 +1137,22 @@ static unsigned int __print_events_type(u8 type, struct event_symbol *syms,
syms->symbol, syms->alias);
else

snprintf(name, sizeof(name), "%s", syms->symbol);

-
- printf(" %-50s [%s]\n", name, event_type_descriptors[type]);
+ if (name_only)
+ printf(" %-50s\n", name);
+ else
+ printf(" %-50s [%s]\n", name, event_type_descriptors[type]);
count++;
}

return count;
}

-unsigned int print_events_type(u8 type)
+unsigned int print_events_type(u8 type, bool name_only)
{
if (type == PERF_TYPE_SOFTWARE)
- return __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
+ return __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
else
- return __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
+ return __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX, name_only);

}

unsigned int print_hwcache_events(const char *event_glob, bool name_only)

diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index bb7d674..148a767 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -102,7 +102,7 @@ void parse_events_update_lists(struct list_head *list_event,

void parse_events_error(void *data, void *scanner, char const *msg);

unsigned int print_events(const char *event_glob, bool name_only);
-unsigned int print_events_type(u8 type);
+unsigned int print_events_type(u8 type, bool name_only);
unsigned int print_tracepoint_events(const char *subsys_glob, const char *event_glob,
bool name_only);
unsigned int print_hwcache_events(const char *event_glob, bool name_only);

[Arnaldo Carvalho de Melo]

Em Fri, Dec 27, 2013 at 06:03:27PM -0500, Dongsheng Yang escreveu:
> As what the usage showing, we can input a event_glob as an argument to list
> all symbols. But it does not work well right now.
>
> Example:
> # perf list kvmmmu
>
> List of pre-defined events (to be used in -e):
>
> This patch make it work well as follows.

It works:

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

[root@zoo ~]# perf list kvmmmu*:*

kvmmmu:kvm_mmu_pagetable_walk [Tracepoint event]
kvmmmu:kvm_mmu_paging_element [Tracepoint event]
kvmmmu:kvm_mmu_set_accessed_bit [Tracepoint event]
kvmmmu:kvm_mmu_set_dirty_bit [Tracepoint event]
kvmmmu:kvm_mmu_walker_error [Tracepoint event]
kvmmmu:kvm_mmu_get_page [Tracepoint event]
kvmmmu:kvm_mmu_sync_page [Tracepoint event]
kvmmmu:kvm_mmu_unsync_page [Tracepoint event]
kvmmmu:kvm_mmu_prepare_zap_page [Tracepoint event]
kvmmmu:mark_mmio_spte [Tracepoint event]
kvmmmu:handle_mmio_page_fault [Tracepoint event]
kvmmmu:fast_page_fault [Tracepoint event]

kvmmmu:kvm_mmu_invalidate_zap_all_pages [Tracepoint event]
kvmmmu:check_mmio_spte [Tracepoint event]
[root@zoo ~]#

The documentation says, as you repeated above, that a glob can be
provided, not a _substring_ :-)

To achive a result similar to:

[root@zoo ~]# perf list hw
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]
stalled-cycles-frontend OR idle-cycles-frontend [Hardware event]
ref-cycles [Hardware event]
[root@zoo ~]#

We could say that if no ':' is found in the provided string, and it
doesn't matches one of the other classes, i.e. hw, sw, cache, etc, and
it is the name of a tracepoint subsystem, then it would be equivalent
to 'tracepoint_subsys:*', i.e. 'perf list skb' woud be equivalent to:

[root@zoo ~]# perf list skb:*
skb:kfree_skb [Tracepoint event]
skb:consume_skb [Tracepoint event]
skb:skb_copy_datagram_iovec [Tracepoint event]
[root@zoo ~]#

- Arnaldo

[Dongsheng Yang]

Yes, you are right. Sorry for that I misunderstood the word of 'glob'.

Thank you for your explanation. :)

Yang

[Dongsheng Yang]

The all print_xxx_event() functions are supporting name_only argument except
print_event_type().

This patch add an argument of name_only for print_events_type() function.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 14 ++++++++------
tools/perf/util/parse-events.h | 2 +-

2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index e2a2066..d70f362 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c

[Dongsheng Yang]

Example:
# perf list test

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

# echo $?
0

Verification:
# perf list test

No event for test.
Usage:
perf list [hw|sw|cache|tracepoint|pmu|event_glob]
# echo $?
255

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/builtin-list.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 011195e..fed6792 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,6 +19,7 @@

int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

{
int i;

+ unsigned int count = 0;

[Dongsheng Yang]

Ramkumar reported that perf list --raw-dump was broken by 44d742e.
Fix by making raw-dump a proper argument.

Signed-off-by: David Ahern <dsa...@gmail.com>
Signed-off-by: Ramkumar Ramachandra <arta...@gmail.com>
Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
Cc: Ramkumar Ramachandra <arta...@gmail.com>
Cc: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/builtin-list.c | 20 ++++++++++----------

1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index fed6792..ec90d0a 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,8 +19,10 @@

int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
{
int i;

+ bool raw_dump = false;

unsigned int count = 0;
const struct option list_options[] = {

+ OPT_BOOLEAN(0, "raw-dump", &raw_dump, "raw dump for completion"),
OPT_END()
};
const char * const list_usage[] = {
@@ -34,7 +36,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
setup_pager();

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

@@ -42,26 +44,24 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

if (i)
putchar('\n');
if (strncmp(argv[i], "tracepoint", 10) == 0)

- count += print_tracepoint_events(NULL, NULL, false);
+ count += print_tracepoint_events(NULL, NULL, raw_dump);

else if (strcmp(argv[i], "hw") == 0 ||
strcmp(argv[i], "hardware") == 0)

- count += print_events_type(PERF_TYPE_HARDWARE);
+ count += print_events_type(PERF_TYPE_HARDWARE, raw_dump);

else if (strcmp(argv[i], "sw") == 0 ||
strcmp(argv[i], "software") == 0)

- count += print_events_type(PERF_TYPE_SOFTWARE);
+ count += print_events_type(PERF_TYPE_SOFTWARE, raw_dump);

else if (strcmp(argv[i], "cache") == 0 ||
strcmp(argv[i], "hwcache") == 0)

- count += print_hwcache_events(NULL, false);
+ count += print_hwcache_events(NULL, raw_dump);

else if (strcmp(argv[i], "pmu") == 0)

- count += print_pmu_events(NULL, false);

- else if (strcmp(argv[i], "--raw-dump") == 0)
- count += print_events(NULL, true);
+ count += print_pmu_events(NULL, raw_dump);

else {
char *sep = strchr(argv[i], ':'), *s;
int sep_idx;

if (sep == NULL) {

- if(!(count += print_events(argv[i], false)))
+ if(!(count += print_events(argv[i], raw_dump)))
goto err_out;
continue;
}

@@ -71,7 +71,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

return -1;

s[sep_idx] = '\0';

- if (!(count += print_tracepoint_events(s, s + sep_idx + 1, false)))
+ if (!(count += print_tracepoint_events(s, s + sep_idx + 1, raw_dump)))
goto err_out;
free(s);

[Dongsheng Yang]

As we make --raw-dump work as a proper option, we need to
make the output of it more readable.

Example:
# ./perf list kvmmmu --raw-dump
kvmmmu:kvm_mmu_pagetable_walk kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit kvmmmu:kvm_mmu_set_dirty_b
it kvmmmu:kvm_mmu_walker_error kvmmmu:kv

Verification:
# ./perf list kvmmmu --raw-dump
kvmmmu:kvm_mmu_pagetable_walk
kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit
kvmmmu:kvm_mmu_set_dirty_bit
kvmmmu:kvm_mmu_walker_error
kvmmmu:kvm_mmu_get_page
kvmmmu:kvm_mmu_sync_page
kvmmmu:kvm_mmu_unsync_page
kvmmmu:kvm_mmu_prepare_zap_page
kvmmmu:mark_mmio_spte
kvmmmu:handle_mmio_page_fault
kvmmmu:fast_page_fault

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 6 +++---
tools/perf/util/pmu.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d70f362..9e3ddc3 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c

@@ -1035,7 +1035,7 @@ unsigned int print_tracepoint_events(const char *subsys_glob, const char *event_
continue;

if (name_only) {
- printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);
+ printf("%s:%s\n", sys_dirent.d_name, evt_dirent.d_name);
count++;
continue;
}
@@ -1177,7 +1177,7 @@ unsigned int print_hwcache_events(const char *event_glob, bool name_only)
continue;

if (name_only)
- printf("%s ", name);
+ printf("%s\n", name);
else

printf(" %-50s [%s]\n", name,

event_type_descriptors[PERF_TYPE_HW_CACHE]);
@@ -1210,7 +1210,7 @@ static unsigned print_symbol_events(const char *event_glob, unsigned type,
continue;

if (name_only) {
- printf("%s ", syms->symbol);
+ printf("%s\n", syms->symbol);
count++;
continue;
}

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 169c480..78f980d 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -751,7 +751,7 @@ unsigned int print_pmu_events(const char *event_glob, bool name_only)

qsort(aliases, len, sizeof(char *), cmp_string);
for (j = 0; j < len; j++) {

if (name_only) {
- printf("%s ", aliases[j]);
+ printf("%s\n", aliases[j]);
zfree(&aliases[j]);
count++;
continue;

[Dongsheng Yang]

As we use PARSE_OPT_STOP_AT_NON_OPTION option in parse_option(), if --raw-dump
is not the first argument, it will be treat as an argument rather than option.

Example:
# ./perf list kvmmmu --raw-dump

kvmmmu:kvm_mmu_pagetable_walk [Tracepoint event]
kvmmmu:kvm_mmu_paging_element [Tracepoint event]
kvmmmu:kvm_mmu_set_accessed_bit [Tracepoint event]
kvmmmu:kvm_mmu_set_dirty_bit [Tracepoint event]
kvmmmu:kvm_mmu_walker_error [Tracepoint event]
kvmmmu:kvm_mmu_get_page [Tracepoint event]
kvmmmu:kvm_mmu_sync_page [Tracepoint event]
kvmmmu:kvm_mmu_unsync_page [Tracepoint event]
kvmmmu:kvm_mmu_prepare_zap_page [Tracepoint event]
kvmmmu:mark_mmio_spte [Tracepoint event]
kvmmmu:handle_mmio_page_fault [Tracepoint event]
kvmmmu:fast_page_fault [Tracepoint event]

This patch replace PARSE_OPT_STOP_AT_NON_OPTION as 0, then we can parse it even if
it is not the first argument.

Verification:
# ./perf list kvmmmu --raw-dump
kvmmmu:kvm_mmu_pagetable_walk kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit kvmmmu:kvm_mmu_set_dirty_b
it kvmmmu:kvm_mmu_walker_error kvmmmu:kv

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/builtin-list.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index ec90d0a..9430666 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c

@@ -30,8 +30,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
NULL
};

- argc = parse_options(argc, argv, list_options, list_usage,
- PARSE_OPT_STOP_AT_NON_OPTION);
+ argc = parse_options(argc, argv, list_options, list_usage, 0);

setup_pager();

[Dongsheng Yang]

Hi Arnaldo,
This patchset is for enhancement of perf list version 2.

Ramkumar and David,
The patch [6/8] is from your work 52A8D2DA...@gmail.com, and
I make --raw-dump works as a option for other argument here. Do you mind
I add signed-off-by Ramkumar and signed-off-by David in this patch?

changelog:
-v1
* remove the patch for event_glob
* add a patch [3/8] to add a missing free in print_pmu_events() fucntion.

Dongsheng Yang (8):

perf tools: Make the all print_xxx_event functions to return unsigned
int.
perf tools: Make the print_pmu_events funtion to return unsigned int.

perf tools: free aliases[j] in pmu.c if name_only is true.

perf tools: Improve the message of perf list for unexpected input.
perf tools: Add support of name_only for print_events_type() function.
perf tools: Make --raw-dump work as a proper option for perf list.
perf tools: Fix bug when --raw-dump is not the first arguement for
perf list.
perf tools: Enhancement for perf list with --raw-dump.

tools/perf/builtin-list.c | 32 +++++++++------

tools/perf/util/parse-events.c | 88 ++++++++++++++++++++++++------------------
tools/perf/util/parse-events.h | 8 ++--
tools/perf/util/pmu.c | 15 ++++---
tools/perf/util/pmu.h | 2 +-
5 files changed, 85 insertions(+), 60 deletions(-)

[Dongsheng Yang]

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/pmu.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 331dc2c..169c480 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -752,6 +752,7 @@ unsigned int print_pmu_events(const char *event_glob, bool name_only)

for (j = 0; j < len; j++) {
if (name_only) {

printf("%s ", aliases[j]);

+ zfree(&aliases[j]);
count++;
continue;
}

[Dongsheng Yang]

Sometimes we need to know how many events have been printed in print_pmu_events.

This patch make this function to return the number of events we have printed.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/pmu.c | 12 +++++++-----
tools/perf/util/pmu.h | 2 +-
2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 0934d64..331dc2c 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c

@@ -711,12 +711,12 @@ static int cmp_string(const void *a, const void *b)
return strcmp(*as, *bs);
}

-void print_pmu_events(const char *event_glob, bool name_only)
+unsigned int print_pmu_events(const char *event_glob, bool name_only)
{
struct perf_pmu *pmu;
struct perf_pmu_alias *alias;
char buf[1024];
- int printed = 0;
+ unsigned int count = 0;
int len, j;
char **aliases;

@@ -727,7 +727,7 @@ void print_pmu_events(const char *event_glob, bool name_only)
len++;
aliases = malloc(sizeof(char *) * len);
if (!aliases)

- return;
+ return count;

pmu = NULL;
j = 0;
while ((pmu = perf_pmu__scan(pmu)) != NULL)

@@ -752,15 +752,17 @@ void print_pmu_events(const char *event_glob, bool name_only)

for (j = 0; j < len; j++) {
if (name_only) {
printf("%s ", aliases[j]);

+ count++;
continue;
}
printf(" %-50s [Kernel PMU event]\n", aliases[j]);
zfree(&aliases[j]);

- printed++;
+ count++;
}
- if (printed)
+ if (count)
printf("\n");

free(aliases);
+ return count;
}

bool pmu_have_event(const char *pname, const char *name)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 9183380..2987fe2 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -42,7 +42,7 @@ int perf_pmu__format_parse(char *dir, struct list_head *head);

struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu);

-void print_pmu_events(const char *event_glob, bool name_only);
+unsigned int print_pmu_events(const char *event_glob, bool name_only);
bool pmu_have_event(const char *pname, const char *name);

int perf_pmu__test(void);

[Dongsheng Yang]

Currently, the most of print_XXX_event() functions are returning void. Then we can
not know whether the printing work is completed well.

This patch change the return type to unsigned int, it means the count of events we
have printed in print_XXX_event() function.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 76 ++++++++++++++++++++++++------------------
tools/perf/util/parse-events.h | 8 ++---
2 files changed, 48 insertions(+), 36 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 0153435..e2a2066 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c

@@ -1000,22 +1000,23 @@ static const char * const event_type_descriptors[] = {
* Print the events from <debugfs_mount_point>/tracing/events
*/

-void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
- bool name_only)
+unsigned int print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+ bool name_only)
{
DIR *sys_dir, *evt_dir;
struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
char evt_path[MAXPATHLEN];
char dir_path[MAXPATHLEN];

+ unsigned int count = 0;

if (debugfs_valid_mountpoint(tracing_events_path)) {
printf(" [ Tracepoints not available: %s ]\n", strerror(errno));

- return;
+ return count;
}

sys_dir = opendir(tracing_events_path);
if (!sys_dir)

- return;
+ return count;

for_each_subsystem(sys_dir, sys_dirent, sys_next) {
if (subsys_glob != NULL &&

@@ -1035,6 +1036,7 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,

if (name_only) {

printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);

+ count++;
continue;
}

@@ -1042,10 +1044,13 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
sys_dirent.d_name, evt_dirent.d_name);
printf(" %-50s [%s]\n", evt_path,
event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+ count++;
}
closedir(evt_dir);
}
closedir(sys_dir);
+
+ return count;
}

/*

@@ -1116,11 +1121,12 @@ static bool is_event_supported(u8 type, unsigned config)
return ret;
}

-static void __print_events_type(u8 type, struct event_symbol *syms,
- unsigned max)
+static unsigned int __print_events_type(u8 type, struct event_symbol *syms,
+ unsigned max)

{
char name[64];
unsigned i;

+ unsigned int count = 0;

for (i = 0; i < max ; i++, syms++) {
if (!is_event_supported(type, i))

@@ -1133,20 +1139,23 @@ static void __print_events_type(u8 type, struct event_symbol *syms,

snprintf(name, sizeof(name), "%s", syms->symbol);

printf(" %-50s [%s]\n", name, event_type_descriptors[type]);
+ count++;
}
+
+ return count;
}

-void print_events_type(u8 type)
+unsigned int print_events_type(u8 type)
{
if (type == PERF_TYPE_SOFTWARE)

- __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
+ return __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
else
- __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
+ return __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
}

-int print_hwcache_events(const char *event_glob, bool name_only)
+unsigned int print_hwcache_events(const char *event_glob, bool name_only)
{
- unsigned int type, op, i, printed = 0;
+ unsigned int type, op, i, count = 0;
char name[64];

for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {

@@ -1170,21 +1179,22 @@ int print_hwcache_events(const char *event_glob, bool name_only)

else
printf(" %-50s [%s]\n", name,
event_type_descriptors[PERF_TYPE_HW_CACHE]);

- ++printed;

+ count++;
}
}
}

- if (printed)
+ if (count)
printf("\n");

- return printed;
+ return count;
}

-static void print_symbol_events(const char *event_glob, unsigned type,
- struct event_symbol *syms, unsigned max,
- bool name_only)
+static unsigned print_symbol_events(const char *event_glob, unsigned type,
+ struct event_symbol *syms, unsigned max,
+ bool name_only)
{

- unsigned i, printed = 0;
+ unsigned i;

+ unsigned int count = 0;

char name[MAX_NAME_LEN];

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

@@ -1199,6 +1209,7 @@ static void print_symbol_events(const char *event_glob, unsigned type,

if (name_only) {

printf("%s ", syms->symbol);
+ count++;
continue;
}

@@ -1209,35 +1220,35 @@ static void print_symbol_events(const char *event_glob, unsigned type,

printf(" %-50s [%s]\n", name, event_type_descriptors[type]);

- printed++;
+ count++;
}

- if (printed)
+ if (count)
printf("\n");

+ return count;
}

/*
* Print the help text for the event symbols:
*/
-void print_events(const char *event_glob, bool name_only)
+unsigned int 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");
- }

+ unsigned int count = 0;

- print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
- event_symbols_hw, PERF_COUNT_HW_MAX, name_only);
+ count += print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
+ event_symbols_hw, PERF_COUNT_HW_MAX,
+ name_only);

- print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
- event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
+ count += print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
+ event_symbols_sw, PERF_COUNT_SW_MAX,
+ name_only);

- print_hwcache_events(event_glob, name_only);
+ count += print_hwcache_events(event_glob, name_only);

- print_pmu_events(event_glob, name_only);
+ count += print_pmu_events(event_glob, name_only);

if (event_glob != NULL)

- return;
+ return count;

if (!name_only) {

printf(" %-50s [%s]\n",

@@ -1256,6 +1267,7 @@ void print_events(const char *event_glob, bool name_only)
}

print_tracepoint_events(NULL, NULL, name_only);

+ return count;
}

int parse_events__is_hardcoded_term(struct parse_events_term *term)

diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index f1cb4c4..bb7d674 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -101,11 +101,11 @@ void parse_events_update_lists(struct list_head *list_event,
struct list_head *list_all);

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);
-void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+unsigned int print_events(const char *event_glob, bool name_only);
+unsigned int print_events_type(u8 type);
+unsigned int 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);
+unsigned int print_hwcache_events(const char *event_glob, bool name_only);
extern int is_valid_tracepoint(const char *event_string);

extern int valid_debugfs_mount(const char *debugfs);

[Ramkumar Ramachandra]

Dongsheng Yang wrote:
> tools/perf/builtin-list.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)

Looks good to me. Thanks for doing this.

[Arnaldo Carvalho de Melo]

Em Mon, Dec 30, 2013 at 10:26:40AM -0500, Dongsheng Yang escreveu:
> The all print_xxx_event() functions are supporting name_only argument except
> print_event_type().
>
> This patch add an argument of name_only for print_events_type() function.

This one looks ok, as the 'name_only' parameter name together with your
changeset comments provides enough context to understand the patch at a
glance.

But it doesn't applies, as the previous one needs changes to address my
comments,

- Arnaldo

[Arnaldo Carvalho de Melo]

What for? Please be clear in the changelogs, it helps with review.

Now I'll have to follow the logic to figure this out, i.e. _why_ we
should free aliases[j] if name_only is true? Is it safe?

- Arnaldo

[Arnaldo Carvalho de Melo]

Em Mon, Dec 30, 2013 at 10:26:39AM -0500, Dongsheng Yang escreveu:
> Example:
> # perf list test
>
> List of pre-defined events (to be used in -e):
> # echo $?
> 0
>
> Verification:
> # perf list test
>
> No event for test.

?

Are we "testing" events?

The message is not clear, I can get some info from looking at the Usage,
but not from 'No event for test'.

At a minimum 'test' (whatevet invalid type/class is specified and found
to be invalid/unknown) should be quoted.

Perhaps the message should be along the lines of:

# perf list test

'test' is not a valid event type, please see the usage below for
acceptable ones.

[Arnaldo Carvalho de Melo]

Em Mon, Dec 30, 2013 at 10:26:37AM -0500, Dongsheng Yang escreveu:
> Sometimes we need to know how many events have been printed in print_pmu_events.

"Sometimes" is too vague, of course it should be needed, otherwise why
bother with the patch? It would be better if you provided an example of
_where_ it will be used, like when printing that newline.

Anyway, reading the other patches,

- Arnaldo

[Arnaldo Carvalho de Melo]

Em Mon, Dec 30, 2013 at 10:26:41AM -0500, Dongsheng Yang escreveu:
> Ramkumar reported that perf list --raw-dump was broken by 44d742e.
> Fix by making raw-dump a proper argument.
>
> Signed-off-by: David Ahern <dsa...@gmail.com>

Are you sure David Signed-off-by' this, isn't this just an Acked-by or
perhaps a Reviewed-by? David?

- Arnaldo

[David Ahern]

On 1/8/14, 8:20 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 30, 2013 at 10:26:41AM -0500, Dongsheng Yang escreveu:
>> >Ramkumar reported that perf list --raw-dump was broken by 44d742e.
>> >Fix by making raw-dump a proper argument.
>> >
>> >Signed-off-by: David Ahern<dsa...@gmail.com>
> Are you sure David Signed-off-by' this, isn't this just an Acked-by or
> perhaps a Reviewed-by? David?

I thought the last version of the patch came from Ramkumar.

This version seems like an add-on to the last patch.

David

[Dongsheng Yang]

On 01/08/2014 04:44 PM, David Ahern wrote:
> On 1/8/14, 8:20 AM, Arnaldo Carvalho de Melo wrote:
>> Em Mon, Dec 30, 2013 at 10:26:41AM -0500, Dongsheng Yang escreveu:
>>> >Ramkumar reported that perf list --raw-dump was broken by 44d742e.
>>> >Fix by making raw-dump a proper argument.
>>> >
>>> >Signed-off-by: David Ahern<dsa...@gmail.com>
>> Are you sure David Signed-off-by' this, isn't this just an Acked-by or
>> perhaps a Reviewed-by? David?
>
> I thought the last version of the patch came from Ramkumar.
>
> This version seems like an add-on to the last patch.
>

So, David, do you mind I add "Signed-off-by: David Ahern" in this patch?

[Dongsheng Yang]

On 01/08/2014 10:19 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 30, 2013 at 10:26:40AM -0500, Dongsheng Yang escreveu:
>> The all print_xxx_event() functions are supporting name_only argument except
>> print_event_type().
>>
>> This patch add an argument of name_only for print_events_type() function.
> This one looks ok, as the 'name_only' parameter name together with your
> changeset comments provides enough context to understand the patch at a
> glance.
>
> But it doesn't applies, as the previous one needs changes to address my
> comments,

Okey, Thanx

[Dongsheng Yang]

On 01/08/2014 10:16 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 30, 2013 at 10:26:39AM -0500, Dongsheng Yang escreveu:
>> Example:
>> # perf list test
>>
>> List of pre-defined events (to be used in -e):
>> # echo $?
>> 0
>>
>> Verification:
>> # perf list test
>>
>> No event for test.
> ?
>
> Are we "testing" events?
>
> The message is not clear, I can get some info from looking at the Usage,
> but not from 'No event for test'.
>
> At a minimum 'test' (whatevet invalid type/class is specified and found
> to be invalid/unknown) should be quoted.
>
> Perhaps the message should be along the lines of:
>
> # perf list test
>
> 'test' is not a valid event type, please see the usage below for
> acceptable ones.

Makes sense. I will update it.

[Dongsheng Yang]

On 01/08/2014 10:12 AM, Arnaldo Carvalho de Melo wrote:
> What for? Please be clear in the changelogs, it helps with review.
>
> Now I'll have to follow the logic to figure this out, i.e. _why_ we
> should free aliases[j] if name_only is true? Is it safe?

Sorry for my lazy on the changelogs here :(.

As aliases is a 2-D array malloced in function print_pmu_events(), we
should free it after printf. But we just did it when (!name_only).

This patch fix the leak error in print_pmu_events() when name_only is true.

I will update the commit message in next version.

- Yang

[Dongsheng Yang]

On 01/08/2014 10:10 AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 30, 2013 at 10:26:37AM -0500, Dongsheng Yang escreveu:
>> Sometimes we need to know how many events have been printed in print_pmu_events.
> "Sometimes" is too vague, of course it should be needed, otherwise why
> bother with the patch? It would be better if you provided an example of
> _where_ it will be used, like when printing that newline.

Sorry for the terrible commit message :(.

What about the following one?

"When we call print_pmu_events(), we want to know how many events has
been printed in this function. But currently it is impossible, as it
return void.

This patch change this function to return the number of events we have
printed."

- Yang

[Dongsheng Yang]

As aliases is a 2-D array malloced in function print_pmu_events(),
we should free it after printf. But we just did it when (!name_only).

This patch fix the leak error in print_pmu_events() when name_only
is true.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/pmu.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 331dc2c..169c480 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -752,6 +752,7 @@ unsigned int print_pmu_events(const char *event_glob, bool name_only)

for (j = 0; j < len; j++) {
if (name_only) {
printf("%s ", aliases[j]);

+ zfree(&aliases[j]);
count++;
continue;
}

[Dongsheng Yang]

The all print_xxx_event() functions are supporting name_only argument except
print_event_type().

This patch add an argument of name_only for print_events_type() function.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

[Dongsheng Yang]

As we make --raw-dump work as a proper option, we need to
make the output of it more readable.

Example:
# ./perf list kvmmmu --raw-dump

kvmmmu:kvm_mmu_pagetable_walk kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit kvmmmu:kvm_mmu_set_dirty_b
it kvmmmu:kvm_mmu_walker_error kvmmmu:kv

Verification:
# ./perf list kvmmmu --raw-dump
kvmmmu:kvm_mmu_pagetable_walk
kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit

kvmmmu:kvm_mmu_set_dirty_bit
kvmmmu:kvm_mmu_walker_error
kvmmmu:kvm_mmu_get_page
kvmmmu:kvm_mmu_sync_page
kvmmmu:kvm_mmu_unsync_page
kvmmmu:kvm_mmu_prepare_zap_page
kvmmmu:mark_mmio_spte
kvmmmu:handle_mmio_page_fault
kvmmmu:fast_page_fault

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 6 +++---
tools/perf/util/pmu.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index d70f362..9e3ddc3 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c

@@ -1035,7 +1035,7 @@ unsigned int print_tracepoint_events(const char *subsys_glob, const char *event_
continue;

if (name_only) {
- printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);
+ printf("%s:%s\n", sys_dirent.d_name, evt_dirent.d_name);
count++;
continue;
}
@@ -1177,7 +1177,7 @@ unsigned int print_hwcache_events(const char *event_glob, bool name_only)
continue;

if (name_only)
- printf("%s ", name);
+ printf("%s\n", name);
else

printf(" %-50s [%s]\n", name,

event_type_descriptors[PERF_TYPE_HW_CACHE]);
@@ -1210,7 +1210,7 @@ static unsigned print_symbol_events(const char *event_glob, unsigned type,
continue;

if (name_only) {
- printf("%s ", syms->symbol);
+ printf("%s\n", syms->symbol);
count++;
continue;
}

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 169c480..78f980d 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c

@@ -751,7 +751,7 @@ unsigned int print_pmu_events(const char *event_glob, bool name_only)
qsort(aliases, len, sizeof(char *), cmp_string);

for (j = 0; j < len; j++) {
if (name_only) {

- printf("%s ", aliases[j]);
+ printf("%s\n", aliases[j]);

zfree(&aliases[j]);
count++;
continue;

[Dongsheng Yang]

Example:
# perf list test

List of pre-defined events (to be used in -e):
# echo $?
0

Verification:
# perf list test

'test' is not a valid event type, please see the usage below for acceptable ones.

Usage:
perf list [hw|sw|cache|tracepoint|pmu|event_glob]
# echo $?
255

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/builtin-list.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c

index 011195e..1aac451 100644

--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,6 +19,7 @@
int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
{
int i;

+ unsigned int count = 0;

+ pr_info("\n'%s' is not a valid event type, please see the usage below for acceptable ones.\n", argv[i]);

+ pr_info("Usage:\n\t%s\n", list_usage[0]);
+ return -1;
}

[Dongsheng Yang]

On 01/08/2014 10:10 AM, Arnaldo Carvalho de Melo wrote:

> Em Mon, Dec 30, 2013 at 10:26:37AM -0500, Dongsheng Yang escreveu:
>> Sometimes we need to know how many events have been printed in print_pmu_events.
> "Sometimes" is too vague, of course it should be needed, otherwise why
> bother with the patch? It would be better if you provided an example of
> _where_ it will be used, like when printing that newline.
>
> Anyway, reading the other patches,

Arnaldo, I squashed this commit into [1/8] in my next version.
They are doing one thing in fact. Please help to review it.

I hope everything looks okey to you.

Thanx

- Yang

[Dongsheng Yang]

Currently, the most of print_XXX_event() functions are returning void. Then we can
not know whether the printing work is completed well.

This patch change the return type to unsigned int, it means the count of events we
have printed in print_XXX_event() function.

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/util/parse-events.c | 76 ++++++++++++++++++++++++------------------
tools/perf/util/parse-events.h | 8 ++---

tools/perf/util/pmu.c | 12 ++++---
tools/perf/util/pmu.h | 2 +-
4 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 0153435..e2a2066 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c

@@ -1000,22 +1000,23 @@ static const char * const event_type_descriptors[] = {
* Print the events from <debugfs_mount_point>/tracing/events
*/

-void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
- bool name_only)
+unsigned int print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+ bool name_only)
{
DIR *sys_dir, *evt_dir;
struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
char evt_path[MAXPATHLEN];
char dir_path[MAXPATHLEN];

+ unsigned int count = 0;

if (debugfs_valid_mountpoint(tracing_events_path)) {
printf(" [ Tracepoints not available: %s ]\n", strerror(errno));

- return;
+ return count;
}

sys_dir = opendir(tracing_events_path);
if (!sys_dir)

- return;
+ return count;

for_each_subsystem(sys_dir, sys_dirent, sys_next) {
if (subsys_glob != NULL &&

@@ -1035,6 +1036,7 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,

if (name_only) {

printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);

+ count++;
continue;
}

@@ -1042,10 +1044,13 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
sys_dirent.d_name, evt_dirent.d_name);
printf(" %-50s [%s]\n", evt_path,
event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+ count++;
}
closedir(evt_dir);
}
closedir(sys_dir);
+
+ return count;
}

/*

@@ -1116,11 +1121,12 @@ static bool is_event_supported(u8 type, unsigned config)
return ret;
}

-static void __print_events_type(u8 type, struct event_symbol *syms,
- unsigned max)
+static unsigned int __print_events_type(u8 type, struct event_symbol *syms,
+ unsigned max)

{
char name[64];
unsigned i;

+ unsigned int count = 0;

for (i = 0; i < max ; i++, syms++) {
if (!is_event_supported(type, i))

@@ -1133,20 +1139,23 @@ static void __print_events_type(u8 type, struct event_symbol *syms,

snprintf(name, sizeof(name), "%s", syms->symbol);

printf(" %-50s [%s]\n", name, event_type_descriptors[type]);
+ count++;
}
+
+ return count;
}

-void print_events_type(u8 type)
+unsigned int print_events_type(u8 type)
{
if (type == PERF_TYPE_SOFTWARE)

- __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
+ return __print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
else
- __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
+ return __print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
}

-int print_hwcache_events(const char *event_glob, bool name_only)
+unsigned int print_hwcache_events(const char *event_glob, bool name_only)
{
- unsigned int type, op, i, printed = 0;
+ unsigned int type, op, i, count = 0;
char name[64];

for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {

@@ -1170,21 +1179,22 @@ int print_hwcache_events(const char *event_glob, bool name_only)

else
printf(" %-50s [%s]\n", name,
event_type_descriptors[PERF_TYPE_HW_CACHE]);

- ++printed;

+ count++;
}
}
}

- if (printed)
+ if (count)
printf("\n");

- return printed;
+ return count;
}

-static void print_symbol_events(const char *event_glob, unsigned type,
- struct event_symbol *syms, unsigned max,
- bool name_only)
+static unsigned print_symbol_events(const char *event_glob, unsigned type,
+ struct event_symbol *syms, unsigned max,
+ bool name_only)
{

- unsigned i, printed = 0;
+ unsigned i;

+ unsigned int count = 0;

char name[MAX_NAME_LEN];

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

@@ -1199,6 +1209,7 @@ static void print_symbol_events(const char *event_glob, unsigned type,

if (name_only) {

printf("%s ", syms->symbol);
+ count++;
continue;
}

@@ -1209,35 +1220,35 @@ static void print_symbol_events(const char *event_glob, unsigned type,

printf(" %-50s [%s]\n", name, event_type_descriptors[type]);

- printed++;
+ count++;
}

- if (printed)
+ if (count)
printf("\n");

+ return count;
}

/*
* Print the help text for the event symbols:
*/
-void print_events(const char *event_glob, bool name_only)
+unsigned int 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");
- }

+ unsigned int count = 0;

- print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
- event_symbols_hw, PERF_COUNT_HW_MAX, name_only);
+ count += print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
+ event_symbols_hw, PERF_COUNT_HW_MAX,
+ name_only);

- print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
- event_symbols_sw, PERF_COUNT_SW_MAX, name_only);
+ count += print_symbol_events(event_glob, PERF_TYPE_SOFTWARE,
+ event_symbols_sw, PERF_COUNT_SW_MAX,
+ name_only);

- print_hwcache_events(event_glob, name_only);
+ count += print_hwcache_events(event_glob, name_only);

- print_pmu_events(event_glob, name_only);
+ count += print_pmu_events(event_glob, name_only);

if (event_glob != NULL)

- return;
+ return count;

if (!name_only) {

printf(" %-50s [%s]\n",

@@ -1256,6 +1267,7 @@ void print_events(const char *event_glob, bool name_only)
}

print_tracepoint_events(NULL, NULL, name_only);
+ return count;
}

int parse_events__is_hardcoded_term(struct parse_events_term *term)

diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index f1cb4c4..bb7d674 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -101,11 +101,11 @@ void parse_events_update_lists(struct list_head *list_event,
struct list_head *list_all);

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);
-void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
+unsigned int print_events(const char *event_glob, bool name_only);
+unsigned int print_events_type(u8 type);
+unsigned int 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);
+unsigned int print_hwcache_events(const char *event_glob, bool name_only);
extern int is_valid_tracepoint(const char *event_string);

extern int valid_debugfs_mount(const char *debugfs);

[Dongsheng Yang]

Hi Arnaldo,
This patchset is for enhancement of perf list v3.

changelog:
-v1
* remove the patch for event_glob
* add a patch [3/8] to add a missing free in print_pmu_events() fucntion.

-v2
* squash [2/8] into [1/8].
* update the error message when perf list with invalid argument.
* update the commit message of [2/7]

Dongsheng Yang (7):

perf tools: Make the all print_xxx_event functions to return unsigned
int.

perf tools: free aliases[j] in pmu.c if name_only is true.
perf tools: Improve the message of perf list for unexpected input.
perf tools: Add support of name_only for print_events_type() function.
perf tools: Make --raw-dump work as a proper option for perf list.
perf tools: Fix bug when --raw-dump is not the first arguement for
perf list.
perf tools: Enhancement for perf list with --raw-dump.

tools/perf/builtin-list.c | 32 +++++++++------
tools/perf/util/parse-events.c | 88 ++++++++++++++++++++++++------------------
tools/perf/util/parse-events.h | 8 ++--
tools/perf/util/pmu.c | 15 ++++---
tools/perf/util/pmu.h | 2 +-
5 files changed, 85 insertions(+), 60 deletions(-)

[Dongsheng Yang]

As we use PARSE_OPT_STOP_AT_NON_OPTION option in parse_option(), if --raw-dump
is not the first argument, it will be treat as an argument rather than option.

Example:
# ./perf list kvmmmu --raw-dump

kvmmmu:kvm_mmu_pagetable_walk [Tracepoint event]
kvmmmu:kvm_mmu_paging_element [Tracepoint event]
kvmmmu:kvm_mmu_set_accessed_bit [Tracepoint event]
kvmmmu:kvm_mmu_set_dirty_bit [Tracepoint event]
kvmmmu:kvm_mmu_walker_error [Tracepoint event]
kvmmmu:kvm_mmu_get_page [Tracepoint event]
kvmmmu:kvm_mmu_sync_page [Tracepoint event]
kvmmmu:kvm_mmu_unsync_page [Tracepoint event]
kvmmmu:kvm_mmu_prepare_zap_page [Tracepoint event]
kvmmmu:mark_mmio_spte [Tracepoint event]
kvmmmu:handle_mmio_page_fault [Tracepoint event]
kvmmmu:fast_page_fault [Tracepoint event]

This patch replace PARSE_OPT_STOP_AT_NON_OPTION with 0, then we can parse it even if

it is not the first argument.

Verification:
# ./perf list kvmmmu --raw-dump
kvmmmu:kvm_mmu_pagetable_walk kvmmmu:kvm_mmu_paging_element
kvmmmu:kvm_mmu_set_accessed_bit kvmmmu:kvm_mmu_set_dirty_b
it kvmmmu:kvm_mmu_walker_error kvmmmu:kv

Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
---

tools/perf/builtin-list.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 9cf98f4..cd9b675 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c

@@ -30,8 +30,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
NULL
};

- argc = parse_options(argc, argv, list_options, list_usage,
- PARSE_OPT_STOP_AT_NON_OPTION);
+ argc = parse_options(argc, argv, list_options, list_usage, 0);

setup_pager();

[Dongsheng Yang]

Ramkumar reported that perf list --raw-dump was broken by 44d742e.
Fix by making raw-dump a proper argument.

Signed-off-by: David Ahern <dsa...@gmail.com>

Signed-off-by: Ramkumar Ramachandra <arta...@gmail.com>
Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>
Cc: Ramkumar Ramachandra <arta...@gmail.com>
Cc: Arnaldo Carvalho de Melo <ac...@redhat.com>
---
tools/perf/builtin-list.c | 20 ++++++++++----------

1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 1aac451..9cf98f4 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -19,8 +19,10 @@

int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

{
int i;
+ bool raw_dump = false;

unsigned int count = 0;

const struct option list_options[] = {

+ OPT_BOOLEAN(0, "raw-dump", &raw_dump, "raw dump for completion"),
OPT_END()
};
const char * const list_usage[] = {
@@ -34,7 +36,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
setup_pager();

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

@@ -42,26 +44,24 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

if (i)
putchar('\n');
if (strncmp(argv[i], "tracepoint", 10) == 0)

- count += print_tracepoint_events(NULL, NULL, false);
+ count += print_tracepoint_events(NULL, NULL, raw_dump);

else if (strcmp(argv[i], "hw") == 0 ||
strcmp(argv[i], "hardware") == 0)

- count += print_events_type(PERF_TYPE_HARDWARE);
+ count += print_events_type(PERF_TYPE_HARDWARE, raw_dump);

else if (strcmp(argv[i], "sw") == 0 ||
strcmp(argv[i], "software") == 0)

- count += print_events_type(PERF_TYPE_SOFTWARE);
+ count += print_events_type(PERF_TYPE_SOFTWARE, raw_dump);

else if (strcmp(argv[i], "cache") == 0 ||
strcmp(argv[i], "hwcache") == 0)

- count += print_hwcache_events(NULL, false);
+ count += print_hwcache_events(NULL, raw_dump);

else if (strcmp(argv[i], "pmu") == 0)

- count += print_pmu_events(NULL, false);

- else if (strcmp(argv[i], "--raw-dump") == 0)
- count += print_events(NULL, true);
+ count += print_pmu_events(NULL, raw_dump);

else {
char *sep = strchr(argv[i], ':'), *s;
int sep_idx;

if (sep == NULL) {

- if(!(count += print_events(argv[i], false)))
+ if(!(count += print_events(argv[i], raw_dump)))
goto err_out;
continue;
}

@@ -71,7 +71,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)

return -1;

s[sep_idx] = '\0';

- if (!(count += print_tracepoint_events(s, s + sep_idx + 1, false)))
+ if (!(count += print_tracepoint_events(s, s + sep_idx + 1, raw_dump)))
goto err_out;
free(s);

[David Ahern]

On 1/9/14, 7:36 AM, Dongsheng Yang wrote:
> On 01/08/2014 04:44 PM, David Ahern wrote:
>> On 1/8/14, 8:20 AM, Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Dec 30, 2013 at 10:26:41AM -0500, Dongsheng Yang escreveu:
>>>> >Ramkumar reported that perf list --raw-dump was broken by 44d742e.
>>>> >Fix by making raw-dump a proper argument.
>>>> >
>>>> >Signed-off-by: David Ahern<dsa...@gmail.com>
>>> Are you sure David Signed-off-by' this, isn't this just an Acked-by or
>>> perhaps a Reviewed-by? David?
>>
>> I thought the last version of the patch came from Ramkumar.
>>
>> This version seems like an add-on to the last patch.
>>
>
> So, David, do you mind I add "Signed-off-by: David Ahern" in this patch?

I didn't write it and content wise very little comes from me.

[Dongsheng Yang]

On 01/10/2014 10:30 AM, David Ahern wrote:
> On 1/9/14, 7:36 AM, Dongsheng Yang wrote:
>> On 01/08/2014 04:44 PM, David Ahern wrote:
>>> On 1/8/14, 8:20 AM, Arnaldo Carvalho de Melo wrote:
>>>> Em Mon, Dec 30, 2013 at 10:26:41AM -0500, Dongsheng Yang escreveu:
>>>>> >Ramkumar reported that perf list --raw-dump was broken by 44d742e.
>>>>> >Fix by making raw-dump a proper argument.
>>>>> >
>>>>> >Signed-off-by: David Ahern<dsa...@gmail.com>
>>>> Are you sure David Signed-off-by' this, isn't this just an Acked-by or
>>>> perhaps a Reviewed-by? David?
>>>
>>> I thought the last version of the patch came from Ramkumar.
>>>
>>> This version seems like an add-on to the last patch.
>>>
>>
>> So, David, do you mind I add "Signed-off-by: David Ahern" in this patch?
>
> I didn't write it and content wise very little comes from me.
>

Okey, David, thanx for your reply. I will remove this line in my patch
next version.

[Dongsheng Yang]

Ramkumar reported that perf list --raw-dump was broken by 44d742e.
Fix by making raw-dump a proper argument.

Signed-off-by: Ramkumar Ramachandra <arta...@gmail.com>
Signed-off-by: Dongsheng Yang <yangd...@cn.fujitsu.com>

---
Changelog:
Remove "Signed-off-by: David Ahern" in commit message.