All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] perf list: make some improvements and fixes
@ 2015-02-13 13:11 Yunlong Song
  2015-02-13 13:11 ` [PATCH 1/7] perf list: clean up the printing functions of hardware/software events Yunlong Song
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

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

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

-- 
1.8.5.5


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/7] perf list: clean up the printing functions of hardware/software events
  2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
@ 2015-02-13 13:11 ` Yunlong Song
  2015-02-13 13:11 ` [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly Yunlong Song
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 tools/perf/builtin-list.c      |  6 ++++--
 tools/perf/util/parse-events.c | 39 +++------------------------------------
 tools/perf/util/parse-events.h | 11 ++++++++++-
 3 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 198f3c3..051a163 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -53,10 +53,12 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 			print_tracepoint_events(NULL, NULL, false);
 		else if (strcmp(argv[i], "hw") == 0 ||
 			 strcmp(argv[i], "hardware") == 0)
-			print_events_type(PERF_TYPE_HARDWARE);
+			print_symbol_events(NULL, PERF_TYPE_HARDWARE,
+					event_symbols_hw, PERF_COUNT_HW_MAX, false);
 		else if (strcmp(argv[i], "sw") == 0 ||
 			 strcmp(argv[i], "software") == 0)
-			print_events_type(PERF_TYPE_SOFTWARE);
+			print_symbol_events(NULL, PERF_TYPE_SOFTWARE,
+					event_symbols_sw, PERF_COUNT_SW_MAX, false);
 		else if (strcmp(argv[i], "cache") == 0 ||
 			 strcmp(argv[i], "hwcache") == 0)
 			print_hwcache_events(NULL, false);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7f8ec6c..fa876da 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -20,11 +20,6 @@
 
 #define MAX_NAME_LEN 100
 
-struct event_symbol {
-	const char	*symbol;
-	const char	*alias;
-};
-
 #ifdef PARSER_DEBUG
 extern int parse_events_debug;
 #endif
@@ -39,7 +34,7 @@ static struct perf_pmu_event_symbol *perf_pmu_events_list;
  */
 static int perf_pmu_events_list_num;
 
-static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
+struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
 	[PERF_COUNT_HW_CPU_CYCLES] = {
 		.symbol = "cpu-cycles",
 		.alias  = "cycles",
@@ -82,7 +77,7 @@ static struct event_symbol event_symbols_hw[PERF_COUNT_HW_MAX] = {
 	},
 };
 
-static struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
+struct event_symbol event_symbols_sw[PERF_COUNT_SW_MAX] = {
 	[PERF_COUNT_SW_CPU_CLOCK] = {
 		.symbol = "cpu-clock",
 		.alias  = "",
@@ -1233,34 +1228,6 @@ static bool is_event_supported(u8 type, unsigned config)
 	return ret;
 }
 
-static void __print_events_type(u8 type, struct event_symbol *syms,
-				unsigned max)
-{
-	char name[64];
-	unsigned i;
-
-	for (i = 0; i < max ; i++, syms++) {
-		if (!is_event_supported(type, i))
-			continue;
-
-		if (strlen(syms->alias))
-			snprintf(name, sizeof(name),  "%s OR %s",
-				 syms->symbol, syms->alias);
-		else
-			snprintf(name, sizeof(name), "%s", syms->symbol);
-
-		printf("  %-50s [%s]\n", name, event_type_descriptors[type]);
-	}
-}
-
-void print_events_type(u8 type)
-{
-	if (type == PERF_TYPE_SOFTWARE)
-		__print_events_type(type, event_symbols_sw, PERF_COUNT_SW_MAX);
-	else
-		__print_events_type(type, event_symbols_hw, PERF_COUNT_HW_MAX);
-}
-
 int print_hwcache_events(const char *event_glob, bool name_only)
 {
 	unsigned int type, op, i, printed = 0;
@@ -1297,7 +1264,7 @@ int print_hwcache_events(const char *event_glob, bool name_only)
 	return printed;
 }
 
-static void print_symbol_events(const char *event_glob, unsigned type,
+void print_symbol_events(const char *event_glob, unsigned type,
 				struct event_symbol *syms, unsigned max,
 				bool name_only)
 {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index ff6e1fa..5eefb6a 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -116,7 +116,16 @@ void parse_events_update_lists(struct list_head *list_event,
 void parse_events_error(void *data, void *scanner, char const *msg);
 
 void print_events(const char *event_glob, bool name_only);
-void print_events_type(u8 type);
+
+struct event_symbol {
+	const char	*symbol;
+	const char	*alias;
+};
+extern struct event_symbol event_symbols_hw[];
+extern struct event_symbol event_symbols_sw[];
+void print_symbol_events(const char *event_glob, unsigned type,
+				struct event_symbol *syms, unsigned max,
+				bool name_only);
 void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
 			     bool name_only);
 int print_hwcache_events(const char *event_glob, bool name_only);
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly
  2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
  2015-02-13 13:11 ` [PATCH 1/7] perf list: clean up the printing functions of hardware/software events Yunlong Song
@ 2015-02-13 13:11 ` Yunlong Song
  2015-02-13 14:45   ` Arnaldo Carvalho de Melo
  2015-02-13 13:11 ` [PATCH 3/7] perf list: fix some inaccuracy problem when parsing the argument Yunlong Song
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

Example
Before this patch:
$perf list

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

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

After this patch:
$perf list

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

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 tools/perf/builtin-list.c      |   2 -
 tools/perf/util/parse-events.c | 166 +++++++++++++++++++++++++++++++++++------
 2 files changed, 145 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 051a163..de5680e 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -47,8 +47,6 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 	}
 
 	for (i = 0; i < argc; ++i) {
-		if (i)
-			putchar('\n');
 		if (strncmp(argv[i], "tracepoint", 10) == 0)
 			print_tracepoint_events(NULL, NULL, false);
 		else if (strcmp(argv[i], "hw") == 0 ||
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index fa876da..cf35b0a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -1093,6 +1093,13 @@ static const char * const event_type_descriptors[] = {
 	"Hardware breakpoint",
 };
 
+static int cmp_string(const void *a, const void *b)
+{
+	const char * const *as = a;
+	const char * const *bs = b;
+	return strcmp(*as, *bs);
+}
+
 /*
  * Print the events from <debugfs_mount_point>/tracing/events
  */
@@ -1105,6 +1112,9 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
 	char evt_path[MAXPATHLEN];
 	char dir_path[MAXPATHLEN];
 	char sbuf[STRERR_BUFSIZE];
+	char **evt_list = NULL;
+	unsigned int evt_i = 0, evt_num = 0;
+	bool evt_num_known = false;
 
 	if (debugfs_valid_mountpoint(tracing_events_path)) {
 		printf("  [ Tracepoints not available: %s ]\n",
@@ -1112,9 +1122,16 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
 		return;
 	}
 
+restart:
 	sys_dir = opendir(tracing_events_path);
 	if (!sys_dir)
 		return;
+    
+	if (evt_num_known) {
+        evt_list = zalloc(sizeof(char *) * evt_num);
+        if (!evt_list)
+            goto out_enomem;
+    }   
 
 	for_each_subsystem(sys_dir, sys_dirent, sys_next) {
 		if (subsys_glob != NULL &&
@@ -1132,19 +1149,52 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
 			    !strglobmatch(evt_dirent.d_name, event_glob))
 				continue;
 
-			if (name_only) {
-				printf("%s:%s ", sys_dirent.d_name, evt_dirent.d_name);
+			if (!evt_num_known) {
+				evt_num++;
 				continue;
 			}
 
 			snprintf(evt_path, MAXPATHLEN, "%s:%s",
 				 sys_dirent.d_name, evt_dirent.d_name);
-			printf("  %-50s [%s]\n", evt_path,
-				event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+
+			evt_list[evt_i] = strdup(evt_path);
+			if (evt_list[evt_i] == NULL)
+				goto out_enomem;
+			evt_i++;
 		}
 		closedir(evt_dir);
 	}
 	closedir(sys_dir);
+
+	if (!evt_num_known) {
+		evt_num_known = true;
+		goto restart;
+	}
+	qsort(evt_list, evt_num, sizeof(char *), cmp_string);
+	evt_i = 0;
+	while (evt_i < evt_num) {
+		if (name_only) {
+			printf("%s ", evt_list[evt_i++]);
+			continue;
+		}
+		printf("  %-50s [%s]\n", evt_list[evt_i++],
+				event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+	}
+	if (evt_num)
+		printf("\n");
+
+out_free:
+	evt_num = evt_i;
+	for (evt_i = 0; evt_i < evt_num; evt_i++)
+		zfree(&evt_list[evt_i]);
+	zfree(&evt_list);
+	return;
+
+out_enomem:
+	printf("FATAL: not enough memory to print %s\n",
+			event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+	if (evt_list)
+		goto out_free;
 }
 
 /*
@@ -1230,8 +1280,17 @@ static bool is_event_supported(u8 type, unsigned config)
 
 int print_hwcache_events(const char *event_glob, bool name_only)
 {
-	unsigned int type, op, i, printed = 0;
+	unsigned int type, op, i, evt_i = 0, evt_num = 0;
 	char name[64];
+	char **evt_list = NULL;
+	bool evt_num_known = false;
+
+restart:
+	if (evt_num_known) {
+		evt_list = zalloc(sizeof(char *) * evt_num);
+		if (!evt_list)
+			goto out_enomem;
+	}
 
 	for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
 		for (op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
@@ -1249,27 +1308,66 @@ int print_hwcache_events(const char *event_glob, bool name_only)
 							type | (op << 8) | (i << 16)))
 					continue;
 
-				if (name_only)
-					printf("%s ", name);
-				else
-					printf("  %-50s [%s]\n", name,
-					       event_type_descriptors[PERF_TYPE_HW_CACHE]);
-				++printed;
+				if (!evt_num_known) {
+					evt_num++;
+					continue;
+				}
+
+				evt_list[evt_i] = strdup(name);
+				if (evt_list[evt_i] == NULL)
+					goto out_enomem;
+				evt_i++;
 			}
 		}
 	}
 
-	if (printed)
+	if (!evt_num_known) {
+		evt_num_known = true;
+		goto restart;
+	}
+	qsort(evt_list, evt_num, sizeof(char *), cmp_string);
+	evt_i = 0;
+	while (evt_i < evt_num) {
+		if (name_only) {
+			printf("%s ", evt_list[evt_i++]);
+			continue;
+		}
+		printf("  %-50s [%s]\n", evt_list[evt_i++],
+				event_type_descriptors[PERF_TYPE_HW_CACHE]);
+	}
+	if (evt_num)
 		printf("\n");
-	return printed;
+
+out_free:
+	evt_num = evt_i;
+	for (evt_i = 0; evt_i < evt_num; evt_i++)
+		zfree(&evt_list[evt_i]);
+	zfree(&evt_list);
+	return evt_num;
+
+out_enomem:
+	printf("FATAL: not enough memory to print %s\n", event_type_descriptors[PERF_TYPE_HW_CACHE]);
+	if (evt_list)
+		goto out_free;
+	return evt_num;
 }
 
 void print_symbol_events(const char *event_glob, unsigned type,
 				struct event_symbol *syms, unsigned max,
 				bool name_only)
 {
-	unsigned i, printed = 0;
+	unsigned int i, evt_i = 0, evt_num = 0;
 	char name[MAX_NAME_LEN];
+	char **evt_list = NULL;
+	bool evt_num_known = false;
+
+restart:
+	if (evt_num_known) {
+		evt_list = zalloc(sizeof(char *) * evt_num);
+		if (!evt_list)
+			goto out_enomem;
+		syms -= max;
+	}
 
 	for (i = 0; i < max; i++, syms++) {
 
@@ -1281,23 +1379,49 @@ void print_symbol_events(const char *event_glob, unsigned type,
 		if (!is_event_supported(type, i))
 			continue;
 
-		if (name_only) {
-			printf("%s ", syms->symbol);
+		if (!evt_num_known) {
+			evt_num++;
 			continue;
 		}
 
-		if (strlen(syms->alias))
+		if (!name_only && strlen(syms->alias))
 			snprintf(name, MAX_NAME_LEN, "%s OR %s", syms->symbol, syms->alias);
 		else
 			strncpy(name, syms->symbol, MAX_NAME_LEN);
 
-		printf("  %-50s [%s]\n", name, event_type_descriptors[type]);
-
-		printed++;
+		evt_list[evt_i] = strdup(name);
+		if (evt_list[evt_i] == NULL)
+			goto out_enomem;
+		evt_i++;
 	}
 
-	if (printed)
+	if (!evt_num_known) {
+		evt_num_known = true;
+		goto restart;
+	}
+	qsort(evt_list, evt_num, sizeof(char *), cmp_string);
+	evt_i = 0;
+	while (evt_i < evt_num) {
+		if (name_only) {
+			printf("%s ", evt_list[evt_i++]);
+			continue;
+		}
+		printf("  %-50s [%s]\n", evt_list[evt_i++], event_type_descriptors[type]);
+	}
+	if (evt_num)
 		printf("\n");
+
+out_free:
+	evt_num = evt_i;
+	for (evt_i = 0; evt_i < evt_num; evt_i++)
+		zfree(&evt_list[evt_i]);
+	zfree(&evt_list);
+	return;
+
+out_enomem:
+	printf("FATAL: not enough memory to print %s\n", event_type_descriptors[type]);
+	if (evt_list)
+		goto out_free;
 }
 
 /*
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/7] perf list: fix some inaccuracy problem when parsing the argument
  2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
  2015-02-13 13:11 ` [PATCH 1/7] perf list: clean up the printing functions of hardware/software events Yunlong Song
  2015-02-13 13:11 ` [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly Yunlong Song
@ 2015-02-13 13:11 ` Yunlong Song
  2015-02-13 13:11 ` [PATCH 4/7] perf list: fix a bug of segmentation fault Yunlong Song
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

Example
Before this patch:
$perf list tracepoint_foo:*

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

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

After this patch:
$perf list tracepoint_foo:*

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

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 tools/perf/builtin-list.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index de5680e..003dec5 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -47,7 +47,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 	}
 
 	for (i = 0; i < argc; ++i) {
-		if (strncmp(argv[i], "tracepoint", 10) == 0)
+		if (strcmp(argv[i], "tracepoint") == 0)
 			print_tracepoint_events(NULL, NULL, false);
 		else if (strcmp(argv[i], "hw") == 0 ||
 			 strcmp(argv[i], "hardware") == 0)
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] perf list: fix a bug of segmentation fault
  2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
                   ` (2 preceding siblings ...)
  2015-02-13 13:11 ` [PATCH 3/7] perf list: fix some inaccuracy problem when parsing the argument Yunlong Song
@ 2015-02-13 13:11 ` Yunlong Song
  2015-02-18 18:41   ` [tip:perf/core] perf tools: Fix " tip-bot for Yunlong Song
  2015-02-13 13:11 ` [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt Yunlong Song
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

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

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

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 tools/perf/util/parse-options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt
  2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
                   ` (3 preceding siblings ...)
  2015-02-13 13:11 ` [PATCH 4/7] perf list: fix a bug of segmentation fault Yunlong Song
@ 2015-02-13 13:11 ` Yunlong Song
  2015-02-13 14:52   ` Arnaldo Carvalho de Melo
  2015-02-13 13:11 ` [PATCH 6/7] perf list: extend raw-dump to certain kind of events Yunlong Song
  2015-02-13 13:11 ` [PATCH 7/7] perf list: place the guiding text in its right position Yunlong Song
  6 siblings, 1 reply; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

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

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 tools/perf/perf.c               | 1 +
 tools/perf/util/parse-options.c | 2 ++
 2 files changed, 3 insertions(+)

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] perf list: extend raw-dump to certain kind of events
  2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
                   ` (4 preceding siblings ...)
  2015-02-13 13:11 ` [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt Yunlong Song
@ 2015-02-13 13:11 ` Yunlong Song
  2015-02-13 14:55   ` Arnaldo Carvalho de Melo
  2015-02-13 13:11 ` [PATCH 7/7] perf list: place the guiding text in its right position Yunlong Song
  6 siblings, 1 reply; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

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

As shown above, all of the events are printed.

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

As shown above, only the hw events are printed.

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 tools/perf/Documentation/perf-list.txt |  6 ++++++
 tools/perf/builtin-list.c              | 21 ++++++++-------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 3e2aec9..4692d27 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -127,6 +127,12 @@ To limit the list use:
 One or more types can be used at the same time, listing the events for the
 types specified.
 
+Support raw format:
+
+. '--raw-dump', shows the raw-dump of all the events.
+. '--raw-dump [hw|sw|cache|tracepoint|pmu|event_glob]', shows the raw-dump of
+  a certain kind of events.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-top[1],
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 003dec5..b81a62c 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -36,38 +36,33 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	setup_pager();
 
-	if (raw_dump) {
-		print_events(NULL, true);
-		return 0;
-	}
-
 	if (argc == 0) {
-		print_events(NULL, false);
+		print_events(NULL, raw_dump);
 		return 0;
 	}
 
 	for (i = 0; i < argc; ++i) {
 		if (strcmp(argv[i], "tracepoint") == 0)
-			print_tracepoint_events(NULL, NULL, false);
+			print_tracepoint_events(NULL, NULL, raw_dump);
 		else if (strcmp(argv[i], "hw") == 0 ||
 			 strcmp(argv[i], "hardware") == 0)
 			print_symbol_events(NULL, PERF_TYPE_HARDWARE,
-					event_symbols_hw, PERF_COUNT_HW_MAX, false);
+					event_symbols_hw, PERF_COUNT_HW_MAX, raw_dump);
 		else if (strcmp(argv[i], "sw") == 0 ||
 			 strcmp(argv[i], "software") == 0)
 			print_symbol_events(NULL, PERF_TYPE_SOFTWARE,
-					event_symbols_sw, PERF_COUNT_SW_MAX, false);
+					event_symbols_sw, PERF_COUNT_SW_MAX, raw_dump);
 		else if (strcmp(argv[i], "cache") == 0 ||
 			 strcmp(argv[i], "hwcache") == 0)
-			print_hwcache_events(NULL, false);
+			print_hwcache_events(NULL, raw_dump);
 		else if (strcmp(argv[i], "pmu") == 0)
-			print_pmu_events(NULL, false);
+			print_pmu_events(NULL, raw_dump);
 		else {
 			char *sep = strchr(argv[i], ':'), *s;
 			int sep_idx;
 
 			if (sep == NULL) {
-				print_events(argv[i], false);
+				print_events(argv[i], raw_dump);
 				continue;
 			}
 			sep_idx = sep - argv[i];
@@ -76,7 +71,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 				return -1;
 
 			s[sep_idx] = '\0';
-			print_tracepoint_events(s, s + sep_idx + 1, false);
+			print_tracepoint_events(s, s + sep_idx + 1, raw_dump);
 			free(s);
 		}
 	}
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/7] perf list: place the guiding text in its right position
  2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
                   ` (5 preceding siblings ...)
  2015-02-13 13:11 ` [PATCH 6/7] perf list: extend raw-dump to certain kind of events Yunlong Song
@ 2015-02-13 13:11 ` Yunlong Song
  2015-02-18 18:42   ` [tip:perf/core] perf list: Place the header " tip-bot for Yunlong Song
  6 siblings, 1 reply; 18+ messages in thread
From: Yunlong Song @ 2015-02-13 13:11 UTC (permalink / raw)
  To: a.p.zijlstra, paulus, mingo, acme; +Cc: linux-kernel, wangnan0

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

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

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

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

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

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

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

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
 tools/perf/builtin-list.c      | 3 +++
 tools/perf/util/parse-events.c | 5 -----
 2 files changed, 3 insertions(+), 5 deletions(-)

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly
  2015-02-13 13:11 ` [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly Yunlong Song
@ 2015-02-13 14:45   ` Arnaldo Carvalho de Melo
  2015-02-13 14:49     ` Arnaldo Carvalho de Melo
  2015-02-15 10:11     ` Yunlong Song
  0 siblings, 2 replies; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-02-13 14:45 UTC (permalink / raw)
  To: Yunlong Song; +Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

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

chmod +x .git/hooks/*

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

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly
  2015-02-13 14:45   ` Arnaldo Carvalho de Melo
@ 2015-02-13 14:49     ` Arnaldo Carvalho de Melo
  2015-02-15 10:19       ` Yunlong Song
  2015-02-15 10:11     ` Yunlong Song
  1 sibling, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-02-13 14:49 UTC (permalink / raw)
  To: Yunlong Song; +Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

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

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt
  2015-02-13 13:11 ` [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt Yunlong Song
@ 2015-02-13 14:52   ` Arnaldo Carvalho de Melo
  2015-02-15 10:01     ` Yunlong Song
  0 siblings, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-02-13 14:52 UTC (permalink / raw)
  To: Yunlong Song; +Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

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

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

Thanks,

- Arnaldo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/7] perf list: extend raw-dump to certain kind of events
  2015-02-13 13:11 ` [PATCH 6/7] perf list: extend raw-dump to certain kind of events Yunlong Song
@ 2015-02-13 14:55   ` Arnaldo Carvalho de Melo
  2015-02-15 10:05     ` Yunlong Song
  0 siblings, 1 reply; 18+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-02-13 14:55 UTC (permalink / raw)
  To: Yunlong Song; +Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt
  2015-02-13 14:52   ` Arnaldo Carvalho de Melo
@ 2015-02-15 10:01     ` Yunlong Song
  0 siblings, 0 replies; 18+ messages in thread
From: Yunlong Song @ 2015-02-15 10:01 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

Already checked, it's really OK.

-- 
Thanks,
Yunlong Song


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/7] perf list: extend raw-dump to certain kind of events
  2015-02-13 14:55   ` Arnaldo Carvalho de Melo
@ 2015-02-15 10:05     ` Yunlong Song
  0 siblings, 0 replies; 18+ messages in thread
From: Yunlong Song @ 2015-02-15 10:05 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

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

-- 
Thanks,
Yunlong Song


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly
  2015-02-13 14:45   ` Arnaldo Carvalho de Melo
  2015-02-13 14:49     ` Arnaldo Carvalho de Melo
@ 2015-02-15 10:11     ` Yunlong Song
  1 sibling, 0 replies; 18+ messages in thread
From: Yunlong Song @ 2015-02-15 10:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

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

-- 
Thanks,
Yunlong Song


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly
  2015-02-13 14:49     ` Arnaldo Carvalho de Melo
@ 2015-02-15 10:19       ` Yunlong Song
  0 siblings, 0 replies; 18+ messages in thread
From: Yunlong Song @ 2015-02-15 10:19 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: a.p.zijlstra, paulus, mingo, linux-kernel, wangnan0

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

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

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

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

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

-- 
Thanks,
Yunlong Song


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf tools: Fix a bug of segmentation fault
  2015-02-13 13:11 ` [PATCH 4/7] perf list: fix a bug of segmentation fault Yunlong Song
@ 2015-02-18 18:41   ` tip-bot for Yunlong Song
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Yunlong Song @ 2015-02-18 18:41 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: paulus, wangnan0, mingo, yunlong.song, tglx, acme, a.p.zijlstra,
	linux-kernel, hpa

Commit-ID:  3a03005ff9445834f3d3b577a11bcbdbdf7a89cf
Gitweb:     http://git.kernel.org/tip/3a03005ff9445834f3d3b577a11bcbdbdf7a89cf
Author:     Yunlong Song <yunlong.song@huawei.com>
AuthorDate: Fri, 13 Feb 2015 21:11:52 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 13 Feb 2015 11:38:43 -0300

perf tools: Fix a bug of segmentation fault

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

Example:

Before this patch:

  $ perf list --list-cmds
  Segmentation fault
  $

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

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1423833115-11199-5-git-send-email-yunlong.song@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-options.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [tip:perf/core] perf list: Place the header text in its right position
  2015-02-13 13:11 ` [PATCH 7/7] perf list: place the guiding text in its right position Yunlong Song
@ 2015-02-18 18:42   ` tip-bot for Yunlong Song
  0 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Yunlong Song @ 2015-02-18 18:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: paulus, wangnan0, yunlong.song, tglx, mingo, linux-kernel,
	a.p.zijlstra, acme, hpa

Commit-ID:  619a303c1b8bd22abc549477d038ef9b5c1fe1bd
Gitweb:     http://git.kernel.org/tip/619a303c1b8bd22abc549477d038ef9b5c1fe1bd
Author:     Yunlong Song <yunlong.song@huawei.com>
AuthorDate: Fri, 13 Feb 2015 21:11:55 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 13 Feb 2015 11:57:50 -0300

perf list: Place the header text in its right position

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

Example
Before this patch:

 $ perf list hw L1-dcache*

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

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

After this patch:

 $ perf list hw L1-dcache*

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

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

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

Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1423833115-11199-8-git-send-email-yunlong.song@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-list.c      | 3 +++
 tools/perf/util/parse-events.c | 5 -----
 2 files changed, 3 insertions(+), 5 deletions(-)

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-02-18 18:42 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-13 13:11 [PATCH 0/7] perf list: make some improvements and fixes Yunlong Song
2015-02-13 13:11 ` [PATCH 1/7] perf list: clean up the printing functions of hardware/software events Yunlong Song
2015-02-13 13:11 ` [PATCH 2/7] perf list: sort the output of 'perf list' to view more clearly Yunlong Song
2015-02-13 14:45   ` Arnaldo Carvalho de Melo
2015-02-13 14:49     ` Arnaldo Carvalho de Melo
2015-02-15 10:19       ` Yunlong Song
2015-02-15 10:11     ` Yunlong Song
2015-02-13 13:11 ` [PATCH 3/7] perf list: fix some inaccuracy problem when parsing the argument Yunlong Song
2015-02-13 13:11 ` [PATCH 4/7] perf list: fix a bug of segmentation fault Yunlong Song
2015-02-18 18:41   ` [tip:perf/core] perf tools: Fix " tip-bot for Yunlong Song
2015-02-13 13:11 ` [PATCH 5/7] perf list: avoid confusion of perf output and the next command prompt Yunlong Song
2015-02-13 14:52   ` Arnaldo Carvalho de Melo
2015-02-15 10:01     ` Yunlong Song
2015-02-13 13:11 ` [PATCH 6/7] perf list: extend raw-dump to certain kind of events Yunlong Song
2015-02-13 14:55   ` Arnaldo Carvalho de Melo
2015-02-15 10:05     ` Yunlong Song
2015-02-13 13:11 ` [PATCH 7/7] perf list: place the guiding text in its right position Yunlong Song
2015-02-18 18:42   ` [tip:perf/core] perf list: Place the header " tip-bot for Yunlong Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.