linux-kernel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).