Re: [PATCH 1/8] perf tools: Fix bug in 'perf list event_glob'.

Re: [PATCH 1/8] perf tools: Fix bug in 'perf list event_glob'.

[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
 
> 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 <yangds.fnst@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)
> -- 
> 1.8.2.1

[PATCH 0/8] Enhancement for perf list.

[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.7050409@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(-)

-- 
1.8.2.1

[PATCH 1/8] perf tools: Fix bug in 'perf list event_glob'.

[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 <yangds.fnst@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)
-- 
1.8.2.1

[PATCH 2/8] perf tools: Make the all print_xxx_event functions to return unsigned int.

[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 <yangds.fnst@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);
-- 
1.8.2.1

[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 <yangds.fnst@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

[PATCH 4/8] perf tools: Improve the message of perf list for unexpected input.

[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 <yangds.fnst@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;
 }
-- 
1.8.2.1

[PATCH 5/8] perf tools: Add support of name_only for print_events_type() function.

[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 <yangds.fnst@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);
-- 
1.8.2.1

[PATCH 6/8] perf tools: Make --raw-dump work as a proper option for perf list.

[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 <dsahern@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
Cc: Ramkumar Ramachandra <artagnon@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@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);
 		}
-- 
1.8.2.1

[PATCH 7/8] perf tools: Fix bug when --raw-dump is not the first arguement for perf list.

[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 <yangds.fnst@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();
 
-- 
1.8.2.1

[PATCH 8/8] perf tools: Enhancement for perf list with --raw-dump.

[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 <yangds.fnst@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;
 		}
-- 
1.8.2.1

Re: [PATCH 1/8] perf tools: Fix bug in 'perf list event_glob'.

[Dongsheng Yang]

On 12/27/2013 02:21 PM, Arnaldo Carvalho de Melo wrote:
> 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_ :-)

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

Thank you for your explanation. :)

Yang
>
> 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
>   
>> 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 <yangds.fnst@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)
>> -- 
>> 1.8.2.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>