perf/core improvements and fixes

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit 0afb1704010f60e7ae85aef0f93fc10f2d99761e:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-02-26 12:25:20 +0100)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo

for you to fetch changes up to fefd2d9619de3bf0bf02a8622e9f445c3d19cc3f:

perf report: Fix branch stack mode cannot be set (2015-02-27 15:52:42 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- Fix SIGBUS failures due to misaligned accesses in Sparc64 (David Ahern)

- Fix branch stack mode in 'perf report' (He Kuang)

- Fix a 'perf probe' operator precedence bug (He Kuang)

- Fix Support for different binaries with same name in 'perf diff' (Kan Liang)

- Check kprobes blacklist when adding new events via 'perf probe' (Masami Hiramatsu)

- Add --purge FILE to remove all caches of FILE in 'perf buildid-cache' (Masami Hiramatsu)

- Show usage with some incorrect params (Masami Hiramatsu)

- Add new buildid cache if update target is not cached in 'buildid-cache' (Masami Hiramatsu)

- Allow listing events with 'tracepoint' prefix in 'perf list' (Yunlong Song)

- Sort the output of 'perf list' (Yunlong Song)

- Fix bash completion of 'perf --' (Yunlong Song)

Developer Zone:

- Handle strdup() failure path in 'perf probe' (Arnaldo Carvalho de Melo)

- Fix get_real_path to free allocated memory in error path in 'perf probe' (Masami Hiramatsu)

- Use pr_debug instead of verbose && pr_info perf buildid-cache (Masami Hiramatsu)

- Fix building of 'perf data' with some gcc versions due to incorrect array struct entry (Yunlong Song)

----------------------------------------------------------------
Arnaldo Carvalho de Melo (1):
perf probe: Handle strdup() failure

David Ahern (1):
perf trace: Fix SIGBUS failures due to misaligned accesses

He Kuang (2):
perf probe: Fix a precedence bug
perf report: Fix branch stack mode cannot be set

Kan Liang (1):
perf diff: Support for different binaries

Masami Hiramatsu (6):
perf probe: Check kprobes blacklist when adding new events perf probe: Fix get_real_path to free allocated memory in error path perf buildid-cache: Add new buildid cache if update target is not cached perf buildid-cache: Add --purge FILE to remove all caches of FILE perf buildid-cache: Use pr_debug instead of verbose && pr_info perf buildid-cache: Show usage with incorrect params

Yunlong Song (8):
perf data: Fix sentinel setting for data_cmds array
perf list: Sort the output of 'perf list' to view more clearly perf list: Allow listing events with 'tracepoint' prefix perf list: Avoid confusion of perf output and the next command prompt perf tools: Remove the '--(null)' long_name for --list-opts perf list: Clean up the printing functions of hardware/software events perf list: Extend raw-dump to certain kind of events
perf tools: Fix the bash completion problem of 'perf --*'

 tools/perf/Documentation/perf-buildid-cache.txt |  24 ++-
 tools/perf/Documentation/perf-diff.txt          |   5 +
 tools/perf/Documentation/perf-list.txt          |   6 +
 tools/perf/builtin-buildid-cache.c              |  72 ++++++--
 tools/perf/builtin-data.c                       |   2 +-
 tools/perf/builtin-list.c                       |  27 ++-
 tools/perf/builtin-report.c                     |   2 +-
 tools/perf/builtin-trace.c                      |  36 +++-
 tools/perf/perf-completion.sh                   |   6 +-
 tools/perf/perf.c                               |  28 ++++
 tools/perf/util/build-id.c                      | 105 ++++++++++--
 tools/perf/util/build-id.h                      |   4 +
 tools/perf/util/parse-events.c                  | 210 +++++++++++++++++-------
 tools/perf/util/parse-events.h                  |  11 +-
 tools/perf/util/parse-options.c                 |   5 +-
 tools/perf/util/probe-event.c                   | 117 ++++++++++++-
 tools/perf/util/sort.c                          |   9 +
 17 files changed, 542 insertions(+), 127 deletions(-)

From: David Ahern <[email protected]>

On Sparc64 perf-trace is failing in many spots due to extended load instructions being used on misaligned accesses.

(gdb) run trace ls
Starting program: /tmp/perf/perf trace ls
[Thread debugging using libthread_db enabled]
Detaching after fork from child process 169460.

<ls output removed>

Program received signal SIGBUS, Bus error.
0x000000000014f4dc in tp_field__u64 (field=0x4cc700, sample=0x7feffffa098) at builtin-trace.c:61
warning: Source file is more recent than executable.
61 TP_UINT_FIELD(64);

(gdb) bt
0 0x000000000014f4dc in tp_field__u64 (field=0x4cc700, sample=0x7feffffa098) at builtin-trace.c:61 1 0x0000000000156ad4 in trace__sys_exit (trace=0x7feffffc268, evsel=0x4cc580, event=0xfffffc0104912000,

    sample=0x7feffffa098) at builtin-trace.c:1701
 2  0x0000000000158c14 in trace__run (trace=0x7feffffc268, argc=1, argv=0x7fefffff360) at builtin-trace.c:2160
 3  0x000000000015b78c in cmd_trace (argc=1, argv=0x7fefffff360, prefix=0x0) at builtin-trace.c:2609
 4  0x0000000000107d94 in run_builtin (p=0x4549c8, argc=2, argv=0x7fefffff360) at perf.c:341
 5  0x0000000000108140 in handle_internal_command (argc=2, argv=0x7fefffff360) at perf.c:400
 6  0x0000000000108308 in run_argv (argcp=0x7feffffef2c, argv=0x7feffffef20) at perf.c:444
 7  0x0000000000108728 in main (argc=2, argv=0x7fefffff360) at perf.c:559

(gdb) p *sample
$1 = {ip = 4391276, pid = 169472, tid = 169472, time = 6303014583281250, addr = 0, id = 72082,
stream_id = 18446744073709551615, period = 1, weight = 0, transaction = 0, cpu = 73, raw_size = 36, data_src = 84410401, flags = 0, insn_len = 0, raw_data = 0xfffffc010491203c, callchain = 0x0, branch_stack = 0x0, user_regs = {abi = 0, mask = 0, regs = 0x0, cache_regs = 0x7feffffa098, cache_mask = 0},

  intr_regs = {abi = 0, mask = 0, regs = 0x0, cache_regs = 0x7feffffa098, cache_mask = 0}, user_stack = {
    offset = 0, size = 0, data = 0x0}, read = {time_enabled = 0, time_running = 0, {group = {nr = 0,
        values = 0x0}, one = {value = 0, id = 0}}}}

(gdb) p *field
$2 = {offset = 16, {integer = 0x14f4a8 <tp_field__u64>, pointer = 0x14f4a8 <tp_field__u64>}}

sample->raw_data is guaranteed to not be 8-byte aligned because it is preceded by the size as a u3. So accessing raw data with an extended load instruction causes the SIGBUS. Resolve by using memcpy to a temporary variable of appropriate size.

---
 tools/perf/builtin-trace.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

index 5cd8497445fe..d95a8f4d988c 100644
@@ -52,7 +52,9 @@ struct tp_field {
 #define TP_UINT_FIELD(bits) \
 static u64 tp_field__u##bits(struct tp_field *field, struct perf_sample *sample) \
 { \
-	return *(u##bits *)(sample->raw_data + field->offset); \
+	u##bits value; \
+	memcpy(&value, sample->raw_data + field->offset, sizeof(value)); \
+	return value;  \
 }
 
 TP_UINT_FIELD(8);

@@ -63,7 +65,8 @@ TP_UINT_FIELD(64);
 #define TP_UINT_FIELD__SWAPPED(bits) \
 static u64 tp_field__swapped_u##bits(struct tp_field *field, struct perf_sample *sample) \
 { \
-	u##bits value = *(u##bits *)(sample->raw_data + field->offset); \
+	u##bits value; \
+	memcpy(&value, sample->raw_data + field->offset, sizeof(value)); \
 	return bswap_##bits(value);\
 }
 

@@ -1517,11 +1520,22 @@ static int trace__read_syscall_info(struct trace *trace, int id)
 	return syscall__set_arg_fmts(sc);
 }
 
+/*
+ * args is to be interpreted as a series of longs but we need to handle
+ * 8-byte unaligned accesses. args points to raw_data within the event
+ * and raw_data is guaranteed to be 8-byte unaligned because it is
+ * preceded by raw_size which is a u32. So we need to copy args to a temp
+ * variable to read it. Most notably this avoids extended load instructions
+ * on unaligned addresses
+ */
+
 static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size,
-				      unsigned long *args, struct trace *trace,
+				      unsigned char *args, struct trace *trace,
 				      struct thread *thread)
 {
 	size_t printed = 0;
+	unsigned char *p;
+	unsigned long val;
 
 	if (sc->tp_format != NULL) {
 		struct format_field *field;

@@ -1537,12 +1551,17 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size,
 		     field = field->next, ++arg.idx, bit <<= 1) {
 			if (arg.mask & bit)
 				continue;
+
+			/* special care for unaligned accesses */
+			p = args + sizeof(unsigned long) * arg.idx;
+			memcpy(&val, p, sizeof(val));
+
 			/*
  			 * Suppress this argument if its value is zero and
  			 * and we don't have a string associated in an
  			 * strarray for it.
  			 */
-			if (args[arg.idx] == 0 &&
+			if (val == 0 &&
 			    !(sc->arg_scnprintf &&
 			      sc->arg_scnprintf[arg.idx] == SCA_STRARRAY &&
 			      sc->arg_parm[arg.idx]))

@@ -1551,23 +1570,26 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size,
 			printed += scnprintf(bf + printed, size - printed,
 					     "%s%s: ", printed ? ", " : "", field->name);
 			if (sc->arg_scnprintf && sc->arg_scnprintf[arg.idx]) {
-				arg.val = args[arg.idx];
+				arg.val = val;
 				if (sc->arg_parm)
 					arg.parm = sc->arg_parm[arg.idx];
 				printed += sc->arg_scnprintf[arg.idx](bf + printed,
 								      size - printed, &arg);
 			} else {
 				printed += scnprintf(bf + printed, size - printed,
-						     "%ld", args[arg.idx]);
+						     "%ld", val);
 			}
 		}
 	} else {
 		int i = 0;
 
 		while (i < 6) {
+			/* special care for unaligned accesses */
+			p = args + sizeof(unsigned long) * i;
+			memcpy(&val, p, sizeof(val));
 			printed += scnprintf(bf + printed, size - printed,
 					     "%sarg%d: %ld",
-					     printed ? ", " : "", i, args[i]);
+					     printed ? ", " : "", i, val);
 			++i;
 		}
 	}

1.9.3

From: Masami Hiramatsu <[email protected]>

Recent linux kernel provides a blacklist of the functions which can not be probed. perf probe can now check this blacklist before setting new events and indicate better error message for users.

Without this patch,
----

  # perf probe --add vmalloc_fault
  Added new event:
  Failed to write event: Invalid argument
    Error: Failed to add events.
  ----
With this patch
  ----
  # perf probe --add vmalloc_fault
  Added new event:
  Warning: Skipped probing on blacklisted function: vmalloc_fault
  ----

---
 tools/perf/util/probe-event.c | 109 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 108 insertions(+), 1 deletion(-)

index 9dfbed96bf39..662d454cb667 100644
@@ -1903,6 +1903,95 @@ static struct strlist *get_probe_trace_command_rawlist(int fd)
 	return sl;
 }
 
+struct kprobe_blacklist_node {
+	struct list_head list;
+	unsigned long start;
+	unsigned long end;
+	char *symbol;
+};
+
+static void kprobe_blacklist__delete(struct list_head *blacklist)
+{
+	struct kprobe_blacklist_node *node;
+
+	while (!list_empty(blacklist)) {
+		node = list_first_entry(blacklist,
+					struct kprobe_blacklist_node, list);
+		list_del(&node->list);
+		free(node->symbol);
+		free(node);
+	}
+}
+
+static int kprobe_blacklist__load(struct list_head *blacklist)
+{
+	struct kprobe_blacklist_node *node;
+	const char *__debugfs = debugfs_find_mountpoint();
+	char buf[PATH_MAX], *p;
+	FILE *fp;
+	int ret;
+
+	if (__debugfs == NULL)
+		return -ENOTSUP;
+
+	ret = e_snprintf(buf, PATH_MAX, "%s/kprobes/blacklist", __debugfs);
+	if (ret < 0)
+		return ret;
+
+	fp = fopen(buf, "r");
+	if (!fp)
+		return -errno;
+
+	ret = 0;
+	while (fgets(buf, PATH_MAX, fp)) {
+		node = zalloc(sizeof(*node));
+		if (!node) {
+			ret = -ENOMEM;
+			break;
+		}
+		INIT_LIST_HEAD(&node->list);
+		list_add_tail(&node->list, blacklist);
+		if (sscanf(buf, "0x%lx-0x%lx", &node->start, &node->end) != 2) {
+			ret = -EINVAL;
+			break;
+		}
+		p = strchr(buf, '\t');
+		if (p) {
+			p++;
+			if (p[strlen(p) - 1] == '\n')
+				p[strlen(p) - 1] = '\0';
+		} else
+			p = (char *)"unknown";
+		node->symbol = strdup(p);
+		if (!node->symbol) {
+			ret = -ENOMEM;
+			break;
+		}
+		pr_debug2("Blacklist: 0x%lx-0x%lx, %s\n",
+			  node->start, node->end, node->symbol);
+		ret++;
+	}
+	if (ret < 0)
+		kprobe_blacklist__delete(blacklist);
+	fclose(fp);
+
+	return ret;
+}
+
+static struct kprobe_blacklist_node *
+kprobe_blacklist__find_by_address(struct list_head *blacklist,
+				  unsigned long address)
+{
+	struct kprobe_blacklist_node *node;
+
+	list_for_each_entry(node, blacklist, list) {
+		if (node->start <= address && address <= node->end)
+			return node;
+	}
+
+	return NULL;
+}
+
 /* Show an event */
 static int show_perf_probe_event(struct perf_probe_event *pev,
 				 const char *module)

@@ -2117,6 +2206,8 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	char buf[64];
 	const char *event, *group;
 	struct strlist *namelist;
+	LIST_HEAD(blacklist);
+	struct kprobe_blacklist_node *node;
 
 	if (pev->uprobes)
 		fd = open_uprobe_events(true);

@@ -2134,11 +2225,25 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		pr_debug("Failed to get current event list.\n");
 		return -EIO;
 	}
+	/* Get kprobe blacklist if exists */
+	if (!pev->uprobes) {
+		ret = kprobe_blacklist__load(&blacklist);
+		if (ret < 0)
+			pr_debug("No kprobe blacklist support, ignored\n");
+	}
 
 	ret = 0;
 	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
 	for (i = 0; i < ntevs; i++) {
 		tev = &tevs[i];
+		/* Ensure that the address is NOT blacklisted */
+		node = kprobe_blacklist__find_by_address(&blacklist,
+							 tev->point.address);
+		if (node) {
+			pr_warning("Warning: Skipped probing on blacklisted function: %s\n", node->symbol);
+			continue;
+		}
+
 		if (pev->event)
 			event = pev->event;
 		else

@@ -2189,13 +2294,15 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		allow_suffix = true;
 	}
 
-	if (ret >= 0) {
+	/* Note that it is possible to skip all events because of blacklist */
+	if (ret >= 0 && tev->event) {
 		/* Show how to use the event. */
 		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
 		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", tev->group,
 			 tev->event);
 	}
 
+	kprobe_blacklist__delete(&blacklist);
 	strlist__delete(namelist);
 	close(fd);
 	return ret;

1.9.3

From: Masami Hiramatsu <[email protected]>

Fix get_real_path to free allocated memory when comp_dir is used for complementing path and getting an error.

---
 tools/perf/util/probe-event.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index 662d454cb667..4a93bf433344 100644
@@ -549,9 +549,11 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
 		if (access(*new_path, R_OK) == 0)
 			return 0;
 
-		if (!symbol_conf.source_prefix)
+		if (!symbol_conf.source_prefix) {
 			/* In case of searching comp_dir, don't retry */
+			zfree(new_path);
 			return -errno;
+		}
 
 		switch (errno) {
 		case ENAMETOOLONG:

1.9.3

From: Arnaldo Carvalho de Melo <[email protected]>

We could end up returning 0 (Ok) with a NULL raw_path. Fix it.

---
 tools/perf/util/probe-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 4a93bf433344..9526cf37682e 100644
@@ -533,7 +533,7 @@ static int get_real_path(const char *raw_path, const char *comp_dir,
 		else {
 			if (access(raw_path, R_OK) == 0) {
 				*new_path = strdup(raw_path);
-				return 0;
+				return *new_path ? 0 : -ENOMEM;
 			} else
 				return -errno;
 		}

1.9.3

From: Masami Hiramatsu <[email protected]>

Add new buildid cache if the update target file is not cached.

This can happen when an old binary is replaced by new one after caching the old one. In this case, user sees his operation just failed.

But it does not look straight, since user just pass the binary "path", not "build-id".

----

  # ./perf buildid-cache --add ./perf
  (update ./perf to new binary)
  # ./perf buildid-cache --update ./perf
  ./perf wasn't in the cache
  #
  ----

This patch adds given new binary to cache if the new binary is not cached. So we'll not see the above error.

----

  # ./perf buildid-cache --add ./perf
  (update ./perf to new binary)
  # ./perf buildid-cache --update ./perf
  #
  ----

---
 tools/perf/Documentation/perf-buildid-cache.txt | 11 ++++++++---
 tools/perf/builtin-buildid-cache.c              |  6 ++++--
 tools/perf/util/build-id.c                      | 12 ++++++++++++
 tools/perf/util/build-id.h                      |  1 +
 4 files changed, 25 insertions(+), 5 deletions(-)

index 0294c57b1f5e..cec6b57e8be6 100644
@@ -41,9 +41,14 @@ OPTIONS
 --missing=::
 	List missing build ids in the cache for the specified file.
 -u::
---update::
-	Update specified file of the cache. It can be used to update kallsyms
-	kernel dso to vmlinux in order to support annotation.
+--update=::
+	Update specified file of the cache. Note that this doesn't remove
+	older entires since those may be still needed for annotating old
+	(or remote) perf.data. Only if there is already a cache which has
+	exactly same build-id, that is replaced by new one. It can be used
+	to update kallsyms and kernel dso to vmlinux in order to support
+	annotation.
+
 -v::
 --verbose::
 	Be more verbose.

index d929d9544664..e7568f5844ad 100644
@@ -255,7 +255,7 @@ static int build_id_cache__update_file(const char *filename)
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 
-	int err;
+	int err = 0;
 
 	if (filename__read_build_id(filename, &build_id, sizeof(build_id)) < 0) {
 		pr_debug("Couldn't read a build-id in %s\n", filename);

@@ -263,7 +263,9 @@ static int build_id_cache__update_file(const char *filename)
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__remove_s(sbuild_id);
+	if (build_id_cache__cached(sbuild_id))
+		err = build_id_cache__remove_s(sbuild_id);
+
 	if (!err)
 		err = build_id_cache__add_s(sbuild_id, filename, false, false);
 

index adbc36028636..0bc33be5a78c 100644
@@ -352,6 +352,18 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
 	return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
 }
 
+bool build_id_cache__cached(const char *sbuild_id)
+{
+	bool ret = false;
+	char *filename = build_id__filename(sbuild_id, NULL, 0);
+
+	if (filename && !access(filename, F_OK))
+		ret = true;
+	free(filename);
+
+	return ret;
+}
+
 int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;

index 31b3c6332a1a..2a094982f954 100644
@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
 int perf_session__write_buildid_table(struct perf_session *session, int fd);
 int perf_session__cache_build_ids(struct perf_session *session);
 
+bool build_id_cache__cached(const char *sbuild_id);
 int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);
 int build_id_cache__remove_s(const char *sbuild_id);

1.9.3

From: Kan Liang <[email protected]>

Currently, the perf diff only works with same binaries. That's because it compares the symbol start address. It doesn't work if the perf.data comes from different binaries. This patch matches the symbol names.

Actually, perf diff once intended to compare the symbol names. The commit as below can look for a pair by name.

604c5c92972d (perf diff: Change the default sort order to "dso,symbol") However, at that time, perf diff used a global list of dsos. That means the binaries which has same name can only be loaded once. That's a problem for comparing different binaries.

For example, we have an old binary and an updated binary. They very likely have same name and most of the functions, so only dsos from old binary will be loaded. When processing the data from updated binary, perf still use the symbol information from old binary. That's wrong.

Then the commit as below used IP to replace symbol name. 9c443dfdd31e ("perf diff: Fix support for all --sort combinations")

The global dsos is discarded from a patch in 2010. a1645ce12adb ("perf: 'perf kvm' tool for monitoring guest performance from host")
However, at that time, perf diff already compared by address. So perf diff cannot work for different binaries as well.

This patch actually rolls back the perf diff to original design. The document is also changed, so everybody knows the original design is to compare the symbol names.

Here are some examples:

The only difference between example_v1.c and example_v2.c is the location of f2 and f3. There is no change in behavior, but the previous perf diff display the wrong differential profile.

example_v1.c
noinline void f3(void)

{
        volatile int i;
        for (i = 0; i < 10000;) {

                if(i%2)
                        i++;
                else
                        i++;
        }
}

noinline void f2(void)

{
        volatile int a = 100, b, c;
        for (b = 0; b < 10000; b++)
                c = a * b;

}

noinline void f1(void)

{
                f2();
                f3();
}

int main()

{
        int i;
        for (i = 0; i < 100000; i++)
                f1();
}

example_v2.c
noinline void f2(void)

{
        volatile int a = 100, b, c;
        for (b = 0; b < 10000; b++)
                c = a * b;
}

noinline void f3(void)

{
        volatile int i;
        for (i = 0; i < 10000;) {
                if(i%2)
                        i++;
                else
                        i++;
        }
}

noinline void f1(void)

{
                f2();
                f3();
}

int main()

{
        int i;
        for (i = 0; i < 100000; i++)
                f1();
}

[lk@localhost perf_diff]$ gcc example_v1.c -o example [lk@localhost perf_diff]$ perf record -o example_v1.data ./example [ perf record: Woken up 4 times to write data ]
[ perf record: Captured and wrote 0.813 MB example_v1.data (~35522 samples) ]

[lk@localhost perf_diff]$ gcc example_v2.c -o example [lk@localhost perf_diff]$ perf record -o example_v2.data ./example [ perf record: Woken up 4 times to write data ]
[ perf record: Captured and wrote 0.824 MB example_v2.data (~36015 samples) ]

Old perf diff result:

[lk@localhost perf_diff]$ perf diff example_v1.data example_v2.data
Event 'cycles'

 Baseline    Delta  Shared Object     Symbol
 ........  .......  ................  ...............................

                     [kernel.vmlinux]  [k] __perf_event_task_sched_out
     0.00%           [kernel.vmlinux]  [k] apic_timer_interrupt
                     [kernel.vmlinux]  [k] idle_cpu
                     [kernel.vmlinux]  [k] intel_pstate_timer_func
                     [kernel.vmlinux]  [k] native_read_msr_safe
     0.00%           [kernel.vmlinux]  [k] native_read_tsc
     0.00%           [kernel.vmlinux]  [k] native_write_msr_safe
                     [kernel.vmlinux]  [k] ntp_tick_length
     0.00%           [kernel.vmlinux]  [k] rb_erase
     0.00%           [kernel.vmlinux]  [k] tick_sched_timer
     0.00%           [kernel.vmlinux]  [k] unmap_single_vma
     0.00%           [kernel.vmlinux]  [k] update_wall_time
     0.00%           example           [.] f1
    46.24%           example           [.] f2
    53.71%   -7.55%  example           [.] f3
            +53.81%  example           [.] f3
     0.02%           example           [.] main

New perf diff result:

[lk@localhost perf_diff]$ perf diff example_v1.data example_v2.data

                     [kernel.vmlinux]  [k] __perf_event_task_sched_out
     0.00%           [kernel.vmlinux]  [k] apic_timer_interrupt
                     [kernel.vmlinux]  [k] idle_cpu
                     [kernel.vmlinux]  [k] intel_pstate_timer_func
                     [kernel.vmlinux]  [k] native_read_msr_safe
     0.00%           [kernel.vmlinux]  [k] native_read_tsc
     0.00%           [kernel.vmlinux]  [k] native_write_msr_safe
                     [kernel.vmlinux]  [k] ntp_tick_length
     0.00%           [kernel.vmlinux]  [k] rb_erase
     0.00%           [kernel.vmlinux]  [k] tick_sched_timer
     0.00%           [kernel.vmlinux]  [k] unmap_single_vma
     0.00%           [kernel.vmlinux]  [k] update_wall_time
     0.00%           example           [.] f1
    46.24%   -0.08%  example           [.] f2
    53.71%   +0.11%  example           [.] f3
     0.02%           example           [.] main

---
 tools/perf/Documentation/perf-diff.txt | 5 +++++
 tools/perf/util/sort.c                 | 9 +++++++++
 2 files changed, 14 insertions(+)

index e463caa3eb49..518266192d67 100644
@@ -20,6 +20,11 @@ If no parameters are passed it will assume perf.data.old and perf.data.
 The differential profile is displayed only for events matching both
 specified perf.data files.
 
+If no parameters are passed the samples will be sorted by dso and symbol.
+As the perf.data files could come from different binaries, the symbols addresses
+could vary. So perf diff is based on the comparison of the files and
+symbols name.
+
 OPTIONS
 -------
 -D::

index 7a39c1ed8d37..4593f36ecc4c 100644
@@ -1463,6 +1463,15 @@ int sort_dimension__add(const char *tok)
 			sort__has_parent = 1;
 		} else if (sd->entry == &sort_sym) {
 			sort__has_sym = 1;
+			/*
+			 * perf diff displays the performance difference amongst
+			 * two or more perf.data files. Those files could come
+			 * from different binaries. So we should not compare
+			 * their ips, but the name of symbol.
+			 */
+			if (sort__mode == SORT_MODE__DIFF)
+				sd->entry->se_collapse = sort__sym_sort;
+
 		} else if (sd->entry == &sort_dso) {
 			sort__has_dso = 1;
 		}

1.9.3

From: He Kuang <[email protected]>

The minus operator has higher precedence than ?: Add parentheses around ?: fix this.

Before this patch:

  $ echo 'p:myprobe do_sys_open' > /sys/kernel/debug/tracing/kprobe_events
  $ perf probe -l -k ../vmlinux
    kprobes:myprobe      (on do_sys_open)

After this patch:

  $ echo 'p:myprobe do_sys_open' > /sys/kernel/debug/tracing/kprobe_events
  $ perf probe -l -k ../vmlinux
    kprobes:myprobe      (on [email protected]/fs/open.c)

---
 tools/perf/util/probe-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 9526cf37682e..7c0e765fa2e3 100644
@@ -151,7 +151,7 @@ static u64 kernel_get_symbol_address_by_name(const char *name, bool reloc)
 		sym = __find_kernel_function_by_name(name, &map);
 		if (sym)
 			return map->unmap_ip(map, sym->start) -
-				(reloc) ? 0 : map->reloc;
+				((reloc) ? 0 : map->reloc);
 	}
 	return 0;
 }

1.9.3

From: Yunlong Song <[email protected]>

The recent new patch "perf tools: Add new 'perf data' command" (commit 2245bf14 in acme's git repo perf/core) has caused a building error when compiling the source code of perf:

cc1: warnings being treated as errors
builtin-data.c:89: error: missing initializer builtin-data.c:89: error: (near initialization for ‘data_cmds[1].summary’) make[2]: *** [builtin-data.o] Error 1
make[2]: *** Waiting for unfinished jobs....
LD bench/perf-in.o
LD tests/perf-in.o
make[1]: *** [perf-in.o] Error 2
make: *** [all] Error 2

This patch fixes the building error above.

---
 tools/perf/builtin-data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 9705ba7e4c16..155cf75b8199 100644
@@ -86,7 +86,7 @@ static int cmd_data_convert(int argc, const char **argv,
 
 static struct data_cmd data_cmds[] = {
 	{ "convert", "converts data file between formats", cmd_data_convert },
-	{ NULL },
+	{ .name = NULL, },
 };
 
 int cmd_data(int argc, const char **argv, const char *prefix)

1.9.3

From: Yunlong Song <[email protected]>

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]
   ...                                                ...

---
 tools/perf/util/parse-events.c | 216 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 193 insertions(+), 23 deletions(-)

index 109ba5c8c2e5..f6822d9b2b53 100644
@@ -1089,6 +1089,14 @@ 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
  */

@@ -1100,11 +1108,21 @@ void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
 	struct dirent *sys_next, *evt_next, sys_dirent, evt_dirent;
 	char evt_path[MAXPATHLEN];
 	char dir_path[MAXPATHLEN];
+	char **evt_list = NULL;
+	unsigned int evt_i = 0, evt_num = 0;
+	bool evt_num_known = false;
 
+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_close_sys_dir;
+	}
+
 	for_each_subsystem(sys_dir, sys_dirent, sys_next) {
 		if (subsys_glob != NULL &&
 		    !strglobmatch(sys_dirent.d_name, subsys_glob))

@@ -1121,19 +1139,56 @@ 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_close_evt_dir;
+			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_close_evt_dir:
+	closedir(evt_dir);
+out_close_sys_dir:
+	closedir(sys_dir);
+
+	printf("FATAL: not enough memory to print %s\n",
+			event_type_descriptors[PERF_TYPE_TRACEPOINT]);
+	if (evt_list)
+		goto out_free;
 }
 
 /*

@@ -1218,20 +1273,61 @@ static void __print_events_type(u8 type, struct event_symbol *syms,
 				unsigned max)
 {
 	char name[64];
-	unsigned i;
+	unsigned int i, evt_i = 0, evt_num = 0;
+	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++) {
 		if (!is_event_supported(type, i))
 			continue;
 
+		if (!evt_num_known) {
+			evt_num++;
+			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]);
+		evt_list[evt_i] = strdup(name);
+		if (evt_list[evt_i] == NULL)
+			goto out_enomem;
+		evt_i++;
+	}
+
+	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)
+		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;
 }
 
 void print_events_type(u8 type)

@@ -1244,8 +1340,17 @@ void print_events_type(u8 type)
 
 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++) {

@@ -1263,27 +1368,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;
 }
 
 static 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++) {
 

@@ -1295,23 +1439,49 @@ static 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.9.3

From: Yunlong Song <[email protected]>

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.

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

index ad8018e26aa0..2acbcf0b554f 100644
@@ -50,9 +50,7 @@ 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)
+		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.9.3

From: Yunlong Song <[email protected]>

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

---
 tools/perf/perf.c               | 1 +
 tools/perf/util/parse-options.c | 2 ++
 2 files changed, 3 insertions(+)

index f3c66b81c6be..3df2665022be 100644
@@ -223,6 +223,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) {

index 4ee9a86705ed..b0ef2d813d2f 100644
@@ -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.9.3

From: Yunlong Song <[email protected]>

If the long_name of a 'struct option' is defined as NULL, --list-opts will incorrectly print '--(null)' in its output. As a result, '--(null)' will finally appear in the case of bash completion, e.g. 'perf record --'.

Example:

Before this patch:

 $ perf record --list-opts

--event --filter --pid --tid --realtime --no-buffering --raw-samples --all-cpus --cpu --count --output --no-inherit --freq --mmap-pages --group --(null) --call-graph --verbose --quiet --stat --data --timestamp --period --no-samples --no-buildid-cache --no-buildid --cgroup --delay --uid --branch-any --branch-filter --weight --transaction --per-thread --intr-regs

After this patch:

 $ perf record --list-opts

--event --filter --pid --tid --realtime --no-buffering --raw-samples --all-cpus --cpu --count --output --no-inherit --freq --mmap-pages --group --call-graph --verbose --quiet --stat --data --timestamp --period --no-samples --no-buildid-cache --no-buildid --cgroup --delay --uid --branch-any --branch-filter --weight --transaction --per-thread --intr-regs

---
 tools/perf/util/parse-options.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index b0ef2d813d2f..1457d6639b60 100644
@@ -505,7 +505,8 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
 		break;
 	case PARSE_OPT_LIST_OPTS:
 		while (options->type != OPTION_END) {
-			printf("--%s ", options->long_name);
+			if (options->long_name)
+				printf("--%s ", options->long_name);
 			options++;
 		}
 		putchar('\n');

1.9.3

From: Yunlong Song <[email protected]>

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.

---
 tools/perf/builtin-list.c      |  6 ++--
 tools/perf/util/parse-events.c | 80 ++----------------------------------------
 tools/perf/util/parse-events.h | 11 +++++-
 3 files changed, 17 insertions(+), 80 deletions(-)

index 2acbcf0b554f..8b323e015458 100644
@@ -54,10 +54,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);

index 39c3b57965d1..52a2dda4f954 100644
@@ -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.9.3

From: Yunlong Song <[email protected]>

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.

---
 tools/perf/Documentation/perf-list.txt |  6 ++++++
 tools/perf/builtin-list.c              | 21 ++++++++-------------
 2 files changed, 14 insertions(+), 13 deletions(-)

index 3e2aec94f806..4692d277980b 100644
@@ -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],

index 8b323e015458..af5bd0514108 100644
@@ -36,41 +36,36 @@ 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 (!raw_dump)
 		printf("\nList of pre-defined events (to be used in -e):\n\n");
 
 	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];

@@ -79,7 +74,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.9.3

From: Yunlong Song <[email protected]>

The perf-completion.sh uses a predefined string '--help --version --exec-path --html-path --paginate --no-pager --perf-dir --work-tree --debugfs-dir' for the bash completion of 'perf --*', which has two problems:

Problem 1: If the options of perf are changed (see handle_options() in perf.c), the perf-completion.sh has to be changed at the same time. If not, the bash completion of 'perf --*' and the options which perf really supports will be inconsistent.

Problem 2: When typing another single character after 'perf --', e.g. 'h', and hit TAB key to get the bash completion of 'perf --h', the character 'h' disappears at once. This is not what we want, we wish the bash completion can return '--help --html-path' and then we can continue to choose one.

To solve this problem, we add '--list-opts' to perf, which now supports 'perf --list-opts' directly, and its result can be used in bash completion now.

Example:

Before this patch:

 $ perf --h                 <-- hit TAB key after character 'h'
 $ perf --                  <-- 'h' disappears and no required result

After this patch:

 $ perf --h                 <-- hit TAB key after character 'h'
 --help       --html-path   <-- the required result

---
 tools/perf/perf-completion.sh |  6 ++----
 tools/perf/perf.c             | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

index 33569847fdcc..c2595e9bd69b 100644
@@ -110,13 +110,11 @@ __perf_main ()
 	# List perf subcommands or long options
 	if [ $cword -eq 1 ]; then
 		if [[ $cur == --* ]]; then
-			__perfcomp '--help --version \
-			--exec-path --html-path --paginate --no-pager \
-			--perf-dir --work-tree --debugfs-dir' -- "$cur"
+			cmds=$($cmd --list-opts)
 		else
 			cmds=$($cmd --list-cmds)
-			__perfcomp "$cmds" "$cur"
 		fi
+		__perfcomp "$cmds" "$cur"
 	# List possible events for -e option
 	elif [[ $prev == "-e" && "${words[1]}" == @(record|stat|top) ]]; then
 		evts=$($cmd list --raw-dump)

index 3df2665022be..b857fcbd00cf 100644
@@ -13,6 +13,7 @@
 #include "util/quote.h"
 #include "util/run-command.h"
 #include "util/parse-events.h"
+#include "util/parse-options.h"
 #include "util/debug.h"
 #include <api/fs/debugfs.h>
 #include <pthread.h>

@@ -125,6 +126,23 @@ static void commit_pager_choice(void)
 	}
 }
 
+struct option options[] = {
+	OPT_ARGUMENT("help", "help"),
+	OPT_ARGUMENT("version", "version"),
+	OPT_ARGUMENT("exec-path", "exec-path"),
+	OPT_ARGUMENT("html-path", "html-path"),
+	OPT_ARGUMENT("paginate", "paginate"),
+	OPT_ARGUMENT("no-pager", "no-pager"),
+	OPT_ARGUMENT("perf-dir", "perf-dir"),
+	OPT_ARGUMENT("work-tree", "work-tree"),
+	OPT_ARGUMENT("debugfs-dir", "debugfs-dir"),
+	OPT_ARGUMENT("buildid-dir", "buildid-dir"),
+	OPT_ARGUMENT("list-cmds", "list-cmds"),
+	OPT_ARGUMENT("list-opts", "list-opts"),
+	OPT_ARGUMENT("debug", "debug"),
+	OPT_END()
+};
+
 static int handle_options(const char ***argv, int *argc, int *envchanged)
 {
 	int handled = 0;

@@ -225,6 +243,15 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			}
 			putchar('\n');
 			exit(0);
+		} else if (!strcmp(cmd, "--list-opts")) {
+			unsigned int i;
+
+			for (i = 0; i < ARRAY_SIZE(options)-1; i++) {
+				struct option *p = options+i;
+				printf("--%s ", p->long_name);
+			}
+			putchar('\n');
+			exit(0);
 		} else if (!strcmp(cmd, "--debug")) {
 			if (*argc < 2) {
 				fprintf(stderr, "No variable specified for --debug.\n");

1.9.3

From: Masami Hiramatsu <[email protected]>

Add --purge FILE to remove all caches of FILE.

Since the current --remove FILE removes a cache which has same build-id of given FILE. Since the command takes a FILE path, it can confuse user who tries to remove cache about FILE path.

-----

  # ./perf buildid-cache -v --add ./perf
  Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  # (update the ./perf binary)
  # ./perf buildid-cache -v --remove ./perf
  Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
  ./perf wasn't in the cache
  -----
Actually, the --remove's FAIL is not shown, it just silently fails.

So, this patch adds --purge FILE action for such usecase.

perf buildid-cache --purge FILE removes all caches which has same FILE path.

In other words, it removes all caches including old binaries.

-----

  # ./perf buildid-cache -v --add ./perf
  Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  # (update the ./perf binary)
  # ./perf buildid-cache -v --purge ./perf
  Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  -----

BTW, if you want to purge all the caches, remove ~/.debug/* .

---
 tools/perf/Documentation/perf-buildid-cache.txt | 13 ++--
 tools/perf/builtin-buildid-cache.c              | 48 +++++++++++++
 tools/perf/util/build-id.c                      | 93 ++++++++++++++++++++-----
 tools/perf/util/build-id.h                      |  3 +
 4 files changed, 136 insertions(+), 21 deletions(-)

index cec6b57e8be6..dd07b55f58d8 100644
@@ -12,9 +12,9 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-This command manages the build-id cache. It can add and remove files to/from
-the cache. In the future it should as well purge older entries, set upper
-limits for the space used by the cache, etc.
+This command manages the build-id cache. It can add, remove, update and purge
+files to/from the cache. In the future it should as well set upper limits for
+the space used by the cache, etc.
 
 OPTIONS
 -------

@@ -36,7 +36,12 @@ OPTIONS
         actually made.
 -r::
 --remove=::
-        Remove specified file from the cache.
+        Remove a cached binary which has same build-id of specified file
+        from the cache.
+-p::
+--purge=::
+        Purge all cached binaries including older caches which have specified
+	path from the cache.
 -M::
 --missing=::
 	List missing build ids in the cache for the specified file.

index 2a094982f954..85011222cc14 100644
@@ -4,6 +4,7 @@
 #define BUILD_ID_SIZE 20
 
 #include "tool.h"
+#include "strlist.h"
 #include <linux/types.h>
 
 extern struct perf_tool build_id__mark_dso_hit_ops;

@@ -22,6 +23,8 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
 int perf_session__write_buildid_table(struct perf_session *session, int fd);
 int perf_session__cache_build_ids(struct perf_session *session);
 
+int build_id_cache__list_build_ids(const char *pathname,
+				   struct strlist **result);
 bool build_id_cache__cached(const char *sbuild_id);
 int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);

1.9.3

From: Masami Hiramatsu <[email protected]>

Use pr_debug instead of the combination of verbose and pr_info.

"if (verbose) pr_info(...)" is same as "pr_debug(...)", replace it.

---
 tools/perf/builtin-buildid-cache.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

index 86f9d78195a4..04466c4ff9d7 100644
@@ -196,9 +196,8 @@ static int build_id_cache__add_file(const char *filename)
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
 	err = build_id_cache__add_s(sbuild_id, filename,
 				    false, false);
-	if (verbose)
-		pr_info("Adding %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 	return err;
 }
 

@@ -216,9 +215,8 @@ static int build_id_cache__remove_file(const char *filename)
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
 	err = build_id_cache__remove_s(sbuild_id);
-	if (verbose)
-		pr_info("Removing %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 
 	return err;
 }

@@ -235,17 +233,15 @@ static int build_id_cache__purge_path(const char *pathname)
 
 	strlist__for_each(pos, list) {
 		err = build_id_cache__remove_s(pos->s);
-		if (verbose)
-			pr_info("Removing %s %s: %s\n", pos->s, pathname,
-				err ? "FAIL" : "Ok");
+		pr_debug("Removing %s %s: %s\n", pos->s, pathname,
+			 err ? "FAIL" : "Ok");
 		if (err)
 			break;
 	}
 	strlist__delete(list);
 
 out:
-	if (verbose)
-		pr_info("Purging %s: %s\n", pathname, err ? "FAIL" : "Ok");
+	pr_debug("Purging %s: %s\n", pathname, err ? "FAIL" : "Ok");
 
 	return err;
 }

@@ -296,9 +292,8 @@ static int build_id_cache__update_file(const char *filename)
 	if (!err)
 		err = build_id_cache__add_s(sbuild_id, filename, false, false);
 
-	if (verbose)
-		pr_info("Updating %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Updating %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 
 	return err;
 }

1.9.3

From: Masami Hiramatsu <[email protected]>

Show usage if no action is specified or unexpected parameter is given. In other words, be more user friendly.

---
 tools/perf/builtin-buildid-cache.c | 5 +++++
 1 file changed, 5 insertions(+)

index 04466c4ff9d7..d47a0cdc71c9 100644
@@ -343,6 +343,11 @@ int cmd_buildid_cache(int argc, const char **argv,
 	argc = parse_options(argc, argv, buildid_cache_options,
 			     buildid_cache_usage, 0);
 
+	if (argc || (!add_name_list_str && !kcore_filename &&
+		     !remove_name_list_str && !purge_name_list_str &&
+		     !missing_filename && !update_name_list_str))
+		usage_with_options(buildid_cache_usage, buildid_cache_options);
+
 	if (missing_filename) {
 		file.path = missing_filename;
 		file.force = force;

1.9.3

From: He Kuang <[email protected]>

When perf.data file is obtained using 'perf record -b', perf report should use branch stack mode to generate output. But this function is broken by improper comparison between boolean and constant -1.

before this patch:

  $ perf report -b -i perf.data
  Samples: 16  of event 'cycles', Event count (approx.): 3171896
  Overhead  Command  Shared Object      Symbol
    13.59%  ls       [kernel.kallsyms]  [k] prio_tree_remove
    13.16%  ls       [kernel.kallsyms]  [k] change_pte_range
    12.09%  ls       [kernel.kallsyms]  [k] page_fault
    12.02%  ls       [kernel.kallsyms]  [k] zap_pte_range
  ...

after this patch:

  $ perf report -b -i perf.data
  Samples: 256  of event 'cycles', Event count (approx.): 256
  Overhead  Command  Source Shared Object  Source Symbol                               Target Shared Object  Target Symbol
     9.38%  ls       [unknown]             [k] 0000000000000000                        [unknown]             [k] 0000000000000000
     6.25%  ls       libc-2.19.so          [.] _dl_addr                                libc-2.19.so          [.] _dl_addr
     6.25%  ls       [kernel.kallsyms]     [k] zap_pte_range                           [kernel.kallsyms]     [k] zap_pte_range
     6.25%  ls       [kernel.kallsyms]     [k] change_pte_range                        [kernel.kallsyms]     [k] change_pte_range
     0.39%  ls       [kernel.kallsyms]     [k] prio_tree_remove                        [kernel.kallsyms]     [k] prio_tree_remove
  ...

---
 tools/perf/builtin-report.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 0ba5f07906fb..fb350343b1d7 100644
@@ -768,7 +768,7 @@ repeat:
 	 * 0/1 means the user chose a mode.
 	 */
 	if (((branch_mode == -1 && has_br_stack) || branch_mode == 1) &&
-	    branch_call_mode == -1) {
+	    !branch_call_mode) {
 		sort__mode = SORT_MODE__BRANCH;
 		symbol_conf.cumulate_callchain = false;
 	}

1.9.3

Pulled, but let me rant a bit:

recent changes to perf bench broke the build on an Ubuntu image I have:

CC bench/futex-hash.o
In file included from bench/futex-hash.c:17:0:
bench/futex.h:73:19: error: conflicting types for ‘pthread_attr_setaffinity_np’
static inline int pthread_attr_setaffinity_np(pthread_attr_t *attr,

                   ^
In file included from bench/futex.h:72:0,
                 from bench/futex-hash.c:17:
/usr/include/pthread.h:407:12: note: previous declaration of 
‘pthread_attr_setaffinity_np’ was here
 extern int pthread_attr_setaffinity_np (pthread_attr_t *__attr,
            ^

So this is a config detection system confusion, and while trying to read the code I got lost in it. For example in:

0695e57b9a6a perf tools: Factor features display code

Firstly, 'factor' isn't a verb we use for code, 'factor out' is. But it's not really factoring out - it separates the code. Did this title want to say:

perf tools: Separate feature display code into three parts

? The title was absolutely unreadable.

Then it introduces two new makefile variables: LIB_FEATURE_TESTS and VF_FEATURE_TESTS. LIB_FEATURE_TESTS is reasonably self-explanatory, but wth is VF_FEATURE_TESTS?

More importantly, why isn't there a _single_ comment explaining their use in the Makefile - _especially_ since CORE_FEATURE_TESTS is explained so well, it's not like a bad example had to be followed.

Using cryptic abbreviations like 'VF' adds insult to injury. It took me some time to figure out that it probably means 'Verbose Features'.

New feature detection adds to all these variables so people will just guess to which to add and why.

Guys, this particular change was rushed and not explained well enough, and it made debugging from that point on harder. Please slow down a bit.

</rant>

Back to the bug, using the (now decyphered VF=1), the pthread-attr test fails:

... pthread-attr-setaffinity-np: [ OFF ]

but that failure is bogus - this is why the build bug triggers. The failure happens because:

triton:~/tip/tools/perf/config/feature-checks> make
test-pthread-attr-setaffinity-np.bin
gcc -MD -o test-pthread-attr-setaffinity-np.bin
test-pthread-attr-setaffinity-np.c -D_GNU_SOURCE -Werror -lpthread test-pthread-attr-setaffinity-np.c: In function ‘main’:
test-pthread-attr-setaffinity-np.c:11:2: error: null argument where non-null required (argument 3) [-Werror=nonnull]
ret = pthread_attr_setaffinity_np(&thread_attr, 0, NULL);
^
cc1: all warnings being treated as errors
Makefile:53: recipe for target 'test-pthread-attr-setaffinity-np.bin' failed

It's a new -Werror=nonnull warning by GCC 4.9.1 that appears to have caused this - extending the testcase to a more substantial pthread_attr_setaffinity_np() use fixes it for me and perf builds fine.

Find further below another feature detection message fix for the same system - libslang-dev was replaced by libslang2-dev.

Thanks,

Ingo

index 0a0d3ecb4e8a..85ab83e6a42f 100644
@@ -5,10 +5,12 @@ int main(void)
 {
 	int ret = 0;
 	pthread_attr_t thread_attr;
+	cpu_set_t cpu_mask;
 
 	pthread_attr_init(&thread_attr);
-	/* don't care abt exact args, just the API itself in libpthread */
-	ret = pthread_attr_setaffinity_np(&thread_attr, 0, NULL);
+	CPU_ZERO(&cpu_mask);
+
+	ret = pthread_attr_setaffinity_np(&thread_attr, sizeof(cpu_mask), &cpu_mask);
 
 	return ret;
 }

index c3570b5..c13184d 100644
@@ -489,7 +489,7 @@ endif
 
 ifndef NO_SLANG
   ifneq ($(feature-libslang), 1)
-    msg := $(warning slang not found, disables TUI support. Please install slang-devel or libslang-dev);
+    msg := $(warning slang not found, disables TUI support. Please install slang-devel or libslang-dev/libslang2-dev);
     NO_SLANG := 1
   else
     # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h

On Sat, Feb 28, 2015 at 08:49:27AM +0100, Ingo Molnar wrote:

SNIP

agreed, sry about that.. I'll try to clean it up while moving features detection into tools/ as you suggested before

SNIP

That would be nice!

Ok, I'll wait for his set of fixes.

Thanks,

Ingo

It's an auto-generated file.

index 40399c3d97d6..68328f517a2e 100644
@@ -1,6 +1,7 @@
 PERF-CFLAGS
 PERF-GUI-VARS
 PERF-VERSION-FILE
+PERF-FEATURES
 perf
 perf-read-vdso32
 perf-read-vdsox32

This message:

Makefile:153: The path 'python-config' is not executable.

Appears on every perf build that does not have a sufficient python environment installed. It's really just an internal detail of python configuration pass and users should not see it - and it's pretty meaningless to them in any case because the message is not very helpful. (So it's not executable. Why does that matter? What can the user do about it?)

Remove the warning, the missing python feature warning is sufficient:

config/Makefile:566: No python-config tool was found config/Makefile:566: Python support will not be built

although even that one isn't very helpful to users: so no Python support will be built, what can the user do to fix that? Most other such warnings give package install suggestions.

index 7076a62d0ff7..c16ce833079c 100644
@@ -175,6 +175,5 @@ _ge-abspath = $(if $(is-executable),$(1))
 define get-executable-or-default
 $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
 endef
-_ge_attempt = $(if $(get-executable),$(get-executable),$(_gea_warn)$(call _gea_err,$(2)))
-_gea_warn = $(warning The path '$(1)' is not executable.)
+_ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2)))
 _gea_err  = $(if $(1),$(error Please set '$(1)' appropriately))

Change the Python detection message from:

config/Makefile:566: No python-config tool was found config/Makefile:566: Python support will not be built

To:

config/Makefile:565: No 'python-config' tool was found: disables Python support - please install python-devel/python-dev

It's now a standard one-line message with a package install suggestion, and it also uses the standard language used by other feature detection messages.

index c3570b5f3bf3..44a14bd40e96 100644

disable-python = $(eval $(disable-python_code))
define disable-python_code
CFLAGS += -DNO_LIBPYTHON

-  $(if $(1),$(warning No $(1) was found))
-  $(warning Python support will not be built)
+  $(warning $1)

NO_LIBPYTHON := 1
endef

ifdef NO_LIBPYTHON
- $(call disable-python)
+ $(call disable-python,Python support disabled by user) else

ifndef PYTHON
- $(call disable-python,python interpreter)
+ $(call disable-python,No python interpreter was found: disables Python support - please install python-devel/python-dev)
else

     PYTHON_WORD := $(call shell-wordify,$(PYTHON))
 
     ifndef PYTHON_CONFIG
-      $(call disable-python,python-config tool)
+      $(call disable-python,No 'python-config' tool was found: disables Python support - please install python-devel/python-dev)
     else
 
       PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
@@ -575,7 +574,7 @@ else
       FLAGS_PYTHON_EMBED := $(PYTHON_EMBED_CCOPTS) $(PYTHON_EMBED_LDOPTS)
 
       ifneq ($(feature-libpython), 1)
-        $(call disable-python,Python.h (for Python 2.x))
+        $(call disable-python,No 'Python.h' (for Python 2.x support) was found: disables Python support - please install python-devel/python-dev)
       else
 
         ifneq ($(feature-libpython-version), 1)

Em Sat, Feb 28, 2015 at 09:33:45AM +0100, Ingo Molnar escreveu:

These patches came with no --- separating the log message from the patch and the following hunk has no line number info for where to apply the first hunk, fixing these up for you.

- Arnaldo

Oops - I wasn't using a proper Git flow, just some crappy quick, ad-hoc manual patch creation flow ...

Thanks,

Ingo

Before:

Missing perl devel files. Disabling perl scripting support, consider installing perl-ExtUtils-Embed

After:

Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev

Change the message to the standard 'please install' language and adds Debian-ish package suggestion.

index c3570b5f3bf3..2f939a04807f 100644
@@ -531,7 +531,7 @@ else
   ifneq ($(feature-libperl), 1)
     CFLAGS += -DNO_LIBPERL
     NO_LIBPERL := 1
-    msg := $(warning Missing perl devel files. Disabling perl scripting support, consider installing perl-ExtUtils-Embed);
+    msg := $(warning Missing perl devel files. Disabling perl scripting support, please install perl-ExtUtils-Embed/libperl-dev);
   else
     LDFLAGS += $(PERL_EMBED_LDFLAGS)
     EXTLIBS += $(PERL_EMBED_LIBADD)

Before:

No bfd.h/libbfd found, install binutils-dev[el]/zlib-static to gain symbol demangling

After:

No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static to gain symbol demangling

Change the message to the standard 'please install' language.

index c3570b5f3bf3..0134af0d1877 100644
@@ -636,7 +635,7 @@ else
             EXTLIBS += -liberty
             CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
           else
-            msg := $(warning No bfd.h/libbfd found, install binutils-dev[el]/zlib-static to gain symbol demangling)
+            msg := $(warning No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static to gain symbol demangling)
             CFLAGS += -DNO_DEMANGLE
           endif
         endif

Before:

No bfd.h/libbfd found, install binutils-dev[el]/zlib-static to gain symbol demangling

After:

No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static/libiberty-dev to gain symbol demangling

Change the message to the standard 'please install' language and also add libiberty-dev suggestion for Ubuntu systems.

--

v2: Added the 'libiberty-dev' package suggestion.

index c3570b5f3bf3..7649cb87a03a 100644
@@ -636,7 +635,7 @@ else
             EXTLIBS += -liberty
             CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
           else
-            msg := $(warning No bfd.h/libbfd found, install binutils-dev[el]/zlib-static to gain symbol demangling)
+            msg := $(warning No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static/libiberty-dev to gain symbol demangling)
             CFLAGS += -DNO_DEMANGLE
           endif
         endif

Certain feature tests fail with link errors:

triton:~/tip/tools/perf/config/feature-checks> make test-libbabeltrace.bin
gcc -MD -o test-libbabeltrace.bin test-libbabeltrace.c # -lbabeltrace provided by /tmp/cc6dRSqd.o: In function `main':
test-libbabeltrace.c:(.text+0xf): undefined reference to `bt_ctf_stream_class_get_packet_context_type'

although they should already fail with a build error due to lack of a proper prototype for the function. Due to this I first tried to find which library was missing - while it was the whole feature that was missing from the .h file already.

To solve this, propagate -Wall -Werror to all testcases and remove them from testcase Makefile rules that used them explicitly.

A missing feature now outputs:

triton:~/tip/tools/perf/config/feature-checks> make test-libbabeltrace.bin
gcc -MD -Wall -Werror -o test-libbabeltrace.bin test-libbabeltrace.c # -lbabeltrace provided by
test-libbabeltrace.c: In function ‘main’:
test-libbabeltrace.c:6:2: error: implicit declaration of function ‘bt_ctf_stream_class_get_packet_context_type’ [-Werror=implicit-function-declaration]

index 70c9aebe9da3..8fe067864957 100644
@@ -39,24 +39,24 @@ PKG_CONFIG := $(CROSS_COMPILE)pkg-config
 
 all: $(FILES)
 
-BUILD = $(CC) $(CFLAGS) -o $(OUTPUT)$@ $(patsubst %.bin,%.c,$@) $(LDFLAGS)
+BUILD = $(CC) $(CFLAGS) -Wall -Werror -o $(OUTPUT)$@ $(patsubst %.bin,%.c,$@) $(LDFLAGS)
 
 ###############################
 
 test-all.bin:
-	$(BUILD) -Werror -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -laudit -I/usr/include/slang -lslang $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -lbabeltrace
+	$(BUILD) -fstack-protector-all -O2 -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -laudit -I/usr/include/slang -lslang $(shell $(PKG_CONFIG) --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl -lz -lbabeltrace
 
 test-hello.bin:
 	$(BUILD)
 
 test-pthread-attr-setaffinity-np.bin:
-	$(BUILD) -D_GNU_SOURCE -Werror -lpthread
+	$(BUILD) -D_GNU_SOURCE -lpthread
 
 test-stackprotector-all.bin:
-	$(BUILD) -Werror -fstack-protector-all
+	$(BUILD) -fstack-protector-all
 
 test-fortify-source.bin:
-	$(BUILD) -O2 -Werror -D_FORTIFY_SOURCE=2
+	$(BUILD) -O2 -D_FORTIFY_SOURCE=2
 
 test-bionic.bin:
 	$(BUILD)

@@ -119,10 +119,10 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl
 
 test-liberty.bin:
-	$(CC) -o $(OUTPUT)$@ test-libbfd.c -DPACKAGE='"perf"' -lbfd -ldl -liberty
+	$(CC) -Wall -Werror -o $(OUTPUT)$@ test-libbfd.c -DPACKAGE='"perf"' -lbfd -ldl -liberty
 
 test-liberty-z.bin:
-	$(CC) -o $(OUTPUT)$@ test-libbfd.c -DPACKAGE='"perf"' -lbfd -ldl -liberty -lz
+	$(CC) -Wall -Werror -o $(OUTPUT)$@ test-libbfd.c -DPACKAGE='"perf"' -lbfd -ldl -liberty -lz
 
 test-cplus-demangle.bin:
 	$(BUILD) -liberty

@@ -140,7 +140,7 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 	$(BUILD) # -lbabeltrace provided by $(FEATURE_CHECK_LDFLAGS-libbabeltrace)
 
 test-sync-compare-and-swap.bin:
-	$(BUILD) -Werror
+	$(BUILD)
 
 test-compile-32.bin:
 	$(CC) -m32 -o $(OUTPUT)$@ test-compile.c

On Debian-ish systems libbabeltrace-dev should be suggested as a package install as well.

index 40399c3d97d6..68328f517a2e 100644
@@ -707,7 +706,7 @@ endif
 
 ifndef NO_LIBBABELTRACE
   ifeq ($(feature-libbabeltrace), 0)
-    msg := $(warning No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-devel/libbabeltrace-ctf-dev);
+    msg := $(warning No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev);
     NO_LIBBABELTRACE := 1
   else
     CFLAGS += -DHAVE_LIBBABELTRACE_SUPPORT $(LIBBABELTRACE_CFLAGS)

Em Sat, Feb 28, 2015 at 10:18:49AM +0100, Ingo Molnar escreveu:

I really suck at editing patches manually! :-/

Thanks!

Ingo