* [PATCH 0/9] perf tools: Add support for build id with different sizes
@ 2020-09-30 17:15 Jiri Olsa
2020-09-30 17:15 ` [PATCH 1/9] perf tools: Add build id shell test Jiri Olsa
` (8 more replies)
0 siblings, 9 replies; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
hi,
currently we support only one storage size (20 bytes) for build
ids. That fits for SHA1 build ids, but there can in theory be
any size. The gcc linker supports also MD5, which is 16 bytes.
Currently the MD5 build id will be stored in .debug cache with
additional zeros, like:
$ find ~/.debug
.../.debug/.build-id/a5/0e350e97c43b4708d09bcd85ebfff700000000
...
.../buildid-ex-md5/a50e350e97c43b4708d09bcd85ebfff700000000
.../buildid-ex-md5/a50e350e97c43b4708d09bcd85ebfff700000000/elf
.../buildid-ex-md5/a50e350e97c43b4708d09bcd85ebfff700000000/probes
And the same reflected in buildid-list as well:
$ perf buildid-list
17f4e448cc746582ea1881528deb549f7fdb3fd5 [kernel.kallsyms]
a50e350e97c43b4708d09bcd85ebfff700000000 .../buildid-ex-md5
This will cause problems in future when we will ask debuginfod
for binaries/debuginfo based on build id.
This patchset is adding 'struct build_id' object, that holds
the build id data and size and use it to store build ids and
changes all related functions to use it.
Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/build_id_size
thanks,
jirka
---
Jiri Olsa (9):
perf tools: Add build id shell test
perf tools: Use build_id object in dso
perf tools: Pass build_id object to filename__read_build_id
perf tools: Pass build id object to sysfs__read_build_id
perf tools: Pass build_id object to build_id__sprintf
perf tools: Pass build_id object to dso__set_build_id
perf tools: Pass build_id object to dso__build_id_equal
perf tools: Add size to struct perf_record_header_build_id
perf tools: Align buildid list output for short build ids
tools/lib/perf/include/perf/event.h | 12 +++++++++++-
tools/perf/Makefile.perf | 14 ++++++++++++++
tools/perf/builtin-buildid-cache.c | 25 ++++++++++++-------------
tools/perf/builtin-inject.c | 3 +--
tools/perf/tests/pe-file-parsing.c | 10 +++++-----
tools/perf/tests/sdt.c | 6 +++---
tools/perf/tests/shell/buildid.sh | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/annotate.c | 3 +--
tools/perf/util/build-id.c | 48 ++++++++++++++++++++++++++++--------------------
tools/perf/util/build-id.h | 8 +++++++-
tools/perf/util/dso.c | 23 ++++++++++-------------
tools/perf/util/dso.h | 7 +++----
tools/perf/util/dsos.c | 9 +++++----
tools/perf/util/header.c | 15 ++++++++++-----
tools/perf/util/map.c | 4 +---
tools/perf/util/probe-event.c | 9 ++++++---
tools/perf/util/probe-finder.c | 8 +++++---
tools/perf/util/scripting-engines/trace-event-python.c | 2 +-
tools/perf/util/symbol-elf.c | 35 +++++++++++++++++++++--------------
tools/perf/util/symbol-minimal.c | 31 ++++++++++++++++---------------
tools/perf/util/symbol.c | 15 +++++++--------
tools/perf/util/symbol.h | 5 +++--
tools/perf/util/synthetic-events.c | 2 +-
23 files changed, 261 insertions(+), 123 deletions(-)
create mode 100755 tools/perf/tests/shell/buildid.sh
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 1/9] perf tools: Add build id shell test
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 2:00 ` Ian Rogers
` (2 more replies)
2020-09-30 17:15 ` [PATCH 2/9] perf tools: Use build_id object in dso Jiri Olsa
` (7 subsequent siblings)
8 siblings, 3 replies; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Adding test for build id cache that adds binary
with sha1 and md5 build ids and verifies it's
added properly.
The test updates build id cache with perf record
and perf buildid-cache -a.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/Makefile.perf | 14 +++++
tools/perf/tests/shell/buildid.sh | 90 +++++++++++++++++++++++++++++++
2 files changed, 104 insertions(+)
create mode 100755 tools/perf/tests/shell/buildid.sh
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 920d8afb9238..b2aeefa64e92 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -126,6 +126,8 @@ include ../scripts/utilities.mak
#
# Define NO_LIBDEBUGINFOD if you do not want support debuginfod
#
+# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
+#
# As per kernel Makefile, avoid funny character set dependencies
unexport LC_ALL
@@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
PROGRAMS += $(OUTPUT)perf-read-vdsox32
endif
+ifndef NO_BUILDID_EX
+PROGRAMS += $(OUTPUT)buildid-ex-sha1
+PROGRAMS += $(OUTPUT)buildid-ex-md5
+endif
+
LIBJVMTI = libperf-jvmti.so
ifndef NO_JVMTI
@@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
$(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
endif
+ifndef NO_BUILDID_EX
+$(OUTPUT)buildid-ex-sha1:
+ $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
+$(OUTPUT)buildid-ex-md5:
+ $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
+endif
+
ifndef NO_JVMTI
LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
new file mode 100755
index 000000000000..57fcd28bc4bd
--- /dev/null
+++ b/tools/perf/tests/shell/buildid.sh
@@ -0,0 +1,90 @@
+#!/bin/sh
+# build id cache operations
+# SPDX-License-Identifier: GPL-2.0
+
+# skip if there are no test binaries
+if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
+ echo "failed: no test binaries"
+ exit 2
+fi
+
+# skip if there's no readelf
+if [ ! -x `which readelf` ]; then
+ echo "failed: no readelf, install binutils"
+ exit 2
+fi
+
+check()
+{
+ id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
+
+ echo "build id: ${id}"
+
+ link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
+ echo "link: ${link}"
+
+ if [ ! -h $link ]; then
+ echo "failed: link ${link} does not exist"
+ exit 1
+ fi
+
+ file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
+ echo "file: ${file}"
+
+ if [ ! -x $file ]; then
+ echo "failed: file ${file} does not exist"
+ exit 1
+ fi
+
+ diff ${file} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: ${file} do not match"
+ exit 1
+ fi
+
+ echo "OK for ${1}"
+}
+
+test_add()
+{
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} buildid-cache -v -a ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: add ${1} to build id cache"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+}
+
+test_record()
+{
+ data=$(mktemp /tmp/perf.data.XXX)
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} record --buildid-all -o ${data} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: record ${1}"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+ rm -rf ${data}
+}
+
+# add binaries manual via perf buildid-cache -a
+test_add buildid-ex-sha1
+test_add buildid-ex-md5
+
+# add binaries via perf record post processing
+test_record buildid-ex-sha1
+test_record buildid-ex-md5
+
+exit ${err}
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 2/9] perf tools: Use build_id object in dso
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
2020-09-30 17:15 ` [PATCH 1/9] perf tools: Add build id shell test Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 2:01 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 3/9] perf tools: Pass build_id object to filename__read_build_id Jiri Olsa
` (6 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Replace build_id byte array with struct build_id
object and all the code that references it.
The objective is to carry size together with build
id array, so it's better to keep both together.
This is preparatory change for following patches,
and there's no functional change.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-buildid-cache.c | 2 +-
tools/perf/builtin-inject.c | 4 ++--
tools/perf/util/annotate.c | 4 ++--
tools/perf/util/build-id.c | 6 +++---
tools/perf/util/build-id.h | 5 +++++
tools/perf/util/dso.c | 18 +++++++++---------
tools/perf/util/dso.h | 2 +-
tools/perf/util/dsos.c | 4 ++--
| 2 +-
tools/perf/util/map.c | 4 ++--
tools/perf/util/probe-event.c | 2 +-
.../scripting-engines/trace-event-python.c | 2 +-
tools/perf/util/symbol.c | 2 +-
tools/perf/util/synthetic-events.c | 2 +-
14 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 39efa51d7fb3..a523c629f321 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -284,7 +284,7 @@ static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
pr_warning("Problems with %s file, consider removing it from the cache\n",
filename);
- } else if (memcmp(dso->build_id, build_id, sizeof(dso->build_id))) {
+ } else if (memcmp(dso->bid.data, build_id, sizeof(dso->bid.data))) {
pr_warning("Problems with %s file, consider removing it from the cache\n",
filename);
}
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 6d2f410d773a..5cf32d8e3aa6 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -408,8 +408,8 @@ static int dso__read_build_id(struct dso *dso)
if (dso->has_build_id)
return 0;
- if (filename__read_build_id(dso->long_name, dso->build_id,
- sizeof(dso->build_id)) > 0) {
+ if (filename__read_build_id(dso->long_name, dso->bid.data,
+ sizeof(dso->bid.data)) > 0) {
dso->has_build_id = true;
return 0;
}
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index fc17af7ba845..a016e1bd7b8d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1578,8 +1578,8 @@ int symbol__strerror_disassemble(struct map_symbol *ms, int errnum, char *buf, s
char *build_id_msg = NULL;
if (dso->has_build_id) {
- build_id__sprintf(dso->build_id,
- sizeof(dso->build_id), bf + 15);
+ build_id__sprintf(dso->bid.data,
+ sizeof(dso->bid.data), bf + 15);
build_id_msg = bf;
}
scnprintf(buf, buflen,
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 31207b6e2066..7da13ddb0d50 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -272,7 +272,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size,
if (!dso->has_build_id)
return NULL;
- build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
+ build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
if (!linkname)
return NULL;
@@ -355,7 +355,7 @@ static int machine__write_buildid_table(struct machine *machine,
in_kernel = pos->kernel ||
is_kernel_module(name,
PERF_RECORD_MISC_CPUMODE_UNKNOWN);
- err = write_buildid(name, name_len, pos->build_id, machine->pid,
+ err = write_buildid(name, name_len, pos->bid.data, machine->pid,
in_kernel ? kmisc : umisc, fd);
if (err)
break;
@@ -841,7 +841,7 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)
is_kallsyms = true;
name = machine->mmap_name;
}
- return build_id_cache__add_b(dso->build_id, sizeof(dso->build_id), name,
+ return build_id_cache__add_b(dso->bid.data, sizeof(dso->bid.data), name,
dso->nsinfo, is_kallsyms, is_vdso);
}
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index aad419bb165c..cc7fc62303ee 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -8,6 +8,11 @@
#include "tool.h"
#include <linux/types.h>
+struct build_id {
+ u8 data[BUILD_ID_SIZE];
+ size_t size;
+};
+
struct nsinfo;
extern struct perf_tool build_id__mark_dso_hit_ops;
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 5a3b4755f0b3..d2c1ed08c879 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -172,8 +172,8 @@ int dso__read_binary_type_filename(const struct dso *dso,
break;
}
- build_id__sprintf(dso->build_id,
- sizeof(dso->build_id),
+ build_id__sprintf(dso->bid.data,
+ sizeof(dso->bid.data),
build_id_hex);
len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
snprintf(filename + len, size - len, "%.2s/%s.debug",
@@ -1330,13 +1330,13 @@ void dso__put(struct dso *dso)
void dso__set_build_id(struct dso *dso, void *build_id)
{
- memcpy(dso->build_id, build_id, sizeof(dso->build_id));
+ memcpy(dso->bid.data, build_id, sizeof(dso->bid.data));
dso->has_build_id = 1;
}
bool dso__build_id_equal(const struct dso *dso, u8 *build_id)
{
- return memcmp(dso->build_id, build_id, sizeof(dso->build_id)) == 0;
+ return memcmp(dso->bid.data, build_id, sizeof(dso->bid.data)) == 0;
}
void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
@@ -1346,8 +1346,8 @@ void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
if (machine__is_default_guest(machine))
return;
sprintf(path, "%s/sys/kernel/notes", machine->root_dir);
- if (sysfs__read_build_id(path, dso->build_id,
- sizeof(dso->build_id)) == 0)
+ if (sysfs__read_build_id(path, dso->bid.data,
+ sizeof(dso->bid.data)) == 0)
dso->has_build_id = true;
}
@@ -1365,8 +1365,8 @@ int dso__kernel_module_get_build_id(struct dso *dso,
"%s/sys/module/%.*s/notes/.note.gnu.build-id",
root_dir, (int)strlen(name) - 1, name);
- if (sysfs__read_build_id(filename, dso->build_id,
- sizeof(dso->build_id)) == 0)
+ if (sysfs__read_build_id(filename, dso->bid.data,
+ sizeof(dso->bid.data)) == 0)
dso->has_build_id = true;
return 0;
@@ -1376,7 +1376,7 @@ size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
{
char sbuild_id[SBUILD_ID_SIZE];
- build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
+ build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
return fprintf(fp, "%s", sbuild_id);
}
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 8ad17f395a19..eac004210b47 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -176,7 +176,7 @@ struct dso {
bool sorted_by_name;
bool loaded;
u8 rel;
- u8 build_id[BUILD_ID_SIZE];
+ struct build_id bid;
u64 text_offset;
const char *short_name;
const char *long_name;
diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index 939471731ea6..e3af46e818f1 100644
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -73,8 +73,8 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
continue;
}
nsinfo__mountns_enter(pos->nsinfo, &nsc);
- if (filename__read_build_id(pos->long_name, pos->build_id,
- sizeof(pos->build_id)) > 0) {
+ if (filename__read_build_id(pos->long_name, pos->bid.data,
+ sizeof(pos->bid.data)) > 0) {
have_build_id = true;
pos->has_build_id = true;
}
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 9cf4efdcbbbd..cde8f6eba479 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2095,7 +2095,7 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
free(m.name);
}
- build_id__sprintf(dso->build_id, sizeof(dso->build_id),
+ build_id__sprintf(dso->bid.data, sizeof(dso->bid.data),
sbuild_id);
pr_debug("build id event received for %s: %s\n",
dso->long_name, sbuild_id);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index cc0faf8f1321..d08540193822 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -346,8 +346,8 @@ int map__load(struct map *map)
if (map->dso->has_build_id) {
char sbuild_id[SBUILD_ID_SIZE];
- build_id__sprintf(map->dso->build_id,
- sizeof(map->dso->build_id),
+ build_id__sprintf(map->dso->bid.data,
+ sizeof(map->dso->bid.data),
sbuild_id);
pr_debug("%s with build id %s not found", name, sbuild_id);
} else
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3a1b58a92673..2041ad849851 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -473,7 +473,7 @@ static struct debuginfo *open_from_debuginfod(struct dso *dso, struct nsinfo *ns
if (!c)
return NULL;
- build_id__sprintf(dso->build_id, BUILD_ID_SIZE, sbuild_id);
+ build_id__sprintf(dso->bid.data, BUILD_ID_SIZE, sbuild_id);
fd = debuginfod_find_debuginfo(c, (const unsigned char *)sbuild_id,
0, &path);
if (fd >= 0)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 739516fdf6e3..db3b1c275d8f 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -1064,7 +1064,7 @@ static int python_export_dso(struct db_export *dbe, struct dso *dso,
char sbuild_id[SBUILD_ID_SIZE];
PyObject *t;
- build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
+ build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
t = tuple_new(5);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5ddf76fb691c..c772c8c70db5 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2153,7 +2153,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
goto proc_kallsyms;
}
- build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
+ build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
/* Find kallsyms in build-id cache with kcore */
scnprintf(path, sizeof(path), "%s/%s/%s",
diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 3ca5d9399680..8a23391558cf 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -1961,7 +1961,7 @@ int perf_event__synthesize_build_id(struct perf_tool *tool, struct dso *pos, u16
len = pos->long_name_len + 1;
len = PERF_ALIGN(len, NAME_ALIGN);
- memcpy(&ev.build_id.build_id, pos->build_id, sizeof(pos->build_id));
+ memcpy(&ev.build_id.build_id, pos->bid.data, sizeof(pos->bid.data));
ev.build_id.header.type = PERF_RECORD_HEADER_BUILD_ID;
ev.build_id.header.misc = misc;
ev.build_id.pid = machine->pid;
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 3/9] perf tools: Pass build_id object to filename__read_build_id
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
2020-09-30 17:15 ` [PATCH 1/9] perf tools: Add build id shell test Jiri Olsa
2020-09-30 17:15 ` [PATCH 2/9] perf tools: Use build_id object in dso Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 5:17 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 4/9] perf tools: Pass build id object to sysfs__read_build_id Jiri Olsa
` (5 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Passing build_id object to filename__read_build_id function,
so it can populate the size of the build_id object.
Changing filename__read_build_id code for both elf/non-elf
code.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-buildid-cache.c | 25 +++++++++++-----------
tools/perf/builtin-inject.c | 3 +--
tools/perf/tests/pe-file-parsing.c | 10 ++++-----
tools/perf/tests/sdt.c | 6 +++---
tools/perf/util/build-id.c | 8 +++----
tools/perf/util/dsos.c | 3 +--
tools/perf/util/symbol-elf.c | 16 ++++++++------
tools/perf/util/symbol-minimal.c | 34 +++++++++++++++++-------------
tools/perf/util/symbol.c | 6 +++---
tools/perf/util/symbol.h | 3 ++-
10 files changed, 59 insertions(+), 55 deletions(-)
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index a523c629f321..c3cb168d546d 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -174,19 +174,19 @@ static int build_id_cache__add_kcore(const char *filename, bool force)
static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
{
char sbuild_id[SBUILD_ID_SIZE];
- u8 build_id[BUILD_ID_SIZE];
+ struct build_id bid;
int err;
struct nscookie nsc;
nsinfo__mountns_enter(nsi, &nsc);
- err = filename__read_build_id(filename, &build_id, sizeof(build_id));
+ err = filename__read_build_id(filename, &bid);
nsinfo__mountns_exit(&nsc);
if (err < 0) {
pr_debug("Couldn't read a build-id in %s\n", filename);
return -1;
}
- build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
+ build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
err = build_id_cache__add_s(sbuild_id, filename, nsi,
false, false);
pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
@@ -196,21 +196,21 @@ static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
static int build_id_cache__remove_file(const char *filename, struct nsinfo *nsi)
{
- u8 build_id[BUILD_ID_SIZE];
char sbuild_id[SBUILD_ID_SIZE];
+ struct build_id bid;
struct nscookie nsc;
int err;
nsinfo__mountns_enter(nsi, &nsc);
- err = filename__read_build_id(filename, &build_id, sizeof(build_id));
+ err = filename__read_build_id(filename, &bid);
nsinfo__mountns_exit(&nsc);
if (err < 0) {
pr_debug("Couldn't read a build-id in %s\n", filename);
return -1;
}
- build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
+ build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
err = build_id_cache__remove_s(sbuild_id);
pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
err ? "FAIL" : "Ok");
@@ -274,17 +274,16 @@ static int build_id_cache__purge_all(void)
static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
{
char filename[PATH_MAX];
- u8 build_id[BUILD_ID_SIZE];
+ struct build_id bid;
if (dso__build_id_filename(dso, filename, sizeof(filename), false) &&
- filename__read_build_id(filename, build_id,
- sizeof(build_id)) != sizeof(build_id)) {
+ filename__read_build_id(filename, &bid) == -1) {
if (errno == ENOENT)
return false;
pr_warning("Problems with %s file, consider removing it from the cache\n",
filename);
- } else if (memcmp(dso->bid.data, build_id, sizeof(dso->bid.data))) {
+ } else if (memcmp(dso->bid.data, bid.data, bid.size)) {
pr_warning("Problems with %s file, consider removing it from the cache\n",
filename);
}
@@ -300,14 +299,14 @@ static int build_id_cache__fprintf_missing(struct perf_session *session, FILE *f
static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
{
- u8 build_id[BUILD_ID_SIZE];
char sbuild_id[SBUILD_ID_SIZE];
+ struct build_id bid;
struct nscookie nsc;
int err;
nsinfo__mountns_enter(nsi, &nsc);
- err = filename__read_build_id(filename, &build_id, sizeof(build_id));
+ err = filename__read_build_id(filename, &bid);
nsinfo__mountns_exit(&nsc);
if (err < 0) {
pr_debug("Couldn't read a build-id in %s\n", filename);
@@ -315,7 +314,7 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
}
err = 0;
- build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
+ build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
if (build_id_cache__cached(sbuild_id))
err = build_id_cache__remove_s(sbuild_id);
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 5cf32d8e3aa6..a7a5ac5ea9d5 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -408,8 +408,7 @@ static int dso__read_build_id(struct dso *dso)
if (dso->has_build_id)
return 0;
- if (filename__read_build_id(dso->long_name, dso->bid.data,
- sizeof(dso->bid.data)) > 0) {
+ if (filename__read_build_id(dso->long_name, &dso->bid) > 0) {
dso->has_build_id = true;
return 0;
}
diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
index 19eae3e8e229..58b90c42eb38 100644
--- a/tools/perf/tests/pe-file-parsing.c
+++ b/tools/perf/tests/pe-file-parsing.c
@@ -24,7 +24,7 @@ static int run_dir(const char *d)
{
char filename[PATH_MAX];
char debugfile[PATH_MAX];
- char build_id[BUILD_ID_SIZE];
+ struct build_id bid;
char debuglink[PATH_MAX];
char expect_build_id[] = {
0x5a, 0x0f, 0xd8, 0x82, 0xb5, 0x30, 0x84, 0x22,
@@ -36,10 +36,10 @@ static int run_dir(const char *d)
int ret;
scnprintf(filename, PATH_MAX, "%s/pe-file.exe", d);
- ret = filename__read_build_id(filename, build_id, BUILD_ID_SIZE);
+ ret = filename__read_build_id(filename, &bid);
TEST_ASSERT_VAL("Failed to read build_id",
ret == sizeof(expect_build_id));
- TEST_ASSERT_VAL("Wrong build_id", !memcmp(build_id, expect_build_id,
+ TEST_ASSERT_VAL("Wrong build_id", !memcmp(bid.data, expect_build_id,
sizeof(expect_build_id)));
ret = filename__read_debuglink(filename, debuglink, PATH_MAX);
@@ -48,10 +48,10 @@ static int run_dir(const char *d)
!strcmp(debuglink, expect_debuglink));
scnprintf(debugfile, PATH_MAX, "%s/%s", d, debuglink);
- ret = filename__read_build_id(debugfile, build_id, BUILD_ID_SIZE);
+ ret = filename__read_build_id(debugfile, &bid);
TEST_ASSERT_VAL("Failed to read debug file build_id",
ret == sizeof(expect_build_id));
- TEST_ASSERT_VAL("Wrong build_id", !memcmp(build_id, expect_build_id,
+ TEST_ASSERT_VAL("Wrong build_id", !memcmp(bid.data, expect_build_id,
sizeof(expect_build_id)));
dso = dso__new(filename);
diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
index 60f0e9ee04fb..3ef37f739203 100644
--- a/tools/perf/tests/sdt.c
+++ b/tools/perf/tests/sdt.c
@@ -28,16 +28,16 @@ static int target_function(void)
static int build_id_cache__add_file(const char *filename)
{
char sbuild_id[SBUILD_ID_SIZE];
- u8 build_id[BUILD_ID_SIZE];
+ struct build_id bid;
int err;
- err = filename__read_build_id(filename, &build_id, sizeof(build_id));
+ err = filename__read_build_id(filename, &bid);
if (err < 0) {
pr_debug("Failed to read build id of %s\n", filename);
return err;
}
- build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
+ build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
err = build_id_cache__add_s(sbuild_id, filename, NULL, false, false);
if (err < 0)
pr_debug("Failed to add build id cache of %s\n", filename);
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 7da13ddb0d50..62b258aaa128 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -130,16 +130,14 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
{
- u8 build_id[BUILD_ID_SIZE];
+ struct build_id bid;
int ret;
- ret = filename__read_build_id(pathname, build_id, sizeof(build_id));
+ ret = filename__read_build_id(pathname, &bid);
if (ret < 0)
return ret;
- else if (ret != sizeof(build_id))
- return -EINVAL;
- return build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
+ return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
}
/* asnprintf consolidates asprintf and snprintf */
diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index e3af46e818f1..87161e431830 100644
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -73,8 +73,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
continue;
}
nsinfo__mountns_enter(pos->nsinfo, &nsc);
- if (filename__read_build_id(pos->long_name, pos->bid.data,
- sizeof(pos->bid.data)) > 0) {
+ if (filename__read_build_id(pos->long_name, &pos->bid) > 0) {
have_build_id = true;
pos->has_build_id = true;
}
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 94a156df22d5..61d7c444e6f5 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -534,8 +534,9 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
#ifdef HAVE_LIBBFD_BUILDID_SUPPORT
-int filename__read_build_id(const char *filename, void *bf, size_t size)
+int filename__read_build_id(const char *filename, struct build_id *bid)
{
+ size_t size = sizeof(bid->data);
int err = -1;
bfd *abfd;
@@ -551,9 +552,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
if (!abfd->build_id || abfd->build_id->size > size)
goto out_close;
- memcpy(bf, abfd->build_id->data, abfd->build_id->size);
- memset(bf + abfd->build_id->size, 0, size - abfd->build_id->size);
- err = abfd->build_id->size;
+ memcpy(bid->data, abfd->build_id->data, abfd->build_id->size);
+ memset(bid->data + abfd->build_id->size, 0, size - abfd->build_id->size);
+ err = bid->size = abfd->build_id->size;
out_close:
bfd_close(abfd);
@@ -562,8 +563,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
#else // HAVE_LIBBFD_BUILDID_SUPPORT
-int filename__read_build_id(const char *filename, void *bf, size_t size)
+int filename__read_build_id(const char *filename, struct build_id *bid)
{
+ size_t size = sizeof(bid->data);
int fd, err = -1;
Elf *elf;
@@ -580,7 +582,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
goto out_close;
}
- err = elf_read_build_id(elf, bf, size);
+ err = elf_read_build_id(elf, bid->data, size);
+ if (err > 0)
+ bid->size = err;
elf_end(elf);
out_close:
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index d6e99af263ec..5173331ee6e4 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -31,9 +31,10 @@ static bool check_need_swap(int file_endian)
#define NT_GNU_BUILD_ID 3
-static int read_build_id(void *note_data, size_t note_len, void *bf,
- size_t size, bool need_swap)
+static int read_build_id(void *note_data, size_t note_len, struct build_id *bid,
+ bool need_swap)
{
+ size_t size = sizeof(bid->data);
struct {
u32 n_namesz;
u32 n_descsz;
@@ -63,8 +64,9 @@ static int read_build_id(void *note_data, size_t note_len, void *bf,
nhdr->n_namesz == sizeof("GNU")) {
if (memcmp(name, "GNU", sizeof("GNU")) == 0) {
size_t sz = min(size, descsz);
- memcpy(bf, ptr, sz);
- memset(bf + sz, 0, size - sz);
+ memcpy(bid->data, ptr, sz);
+ memset(bid->data + sz, 0, size - sz);
+ bid->size = sz;
return 0;
}
}
@@ -84,7 +86,7 @@ int filename__read_debuglink(const char *filename __maybe_unused,
/*
* Just try PT_NOTE header otherwise fails
*/
-int filename__read_build_id(const char *filename, void *bf, size_t size)
+int filename__read_build_id(const char *filename, struct build_id *bid)
{
FILE *fp;
int ret = -1;
@@ -156,9 +158,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
if (fread(buf, buf_size, 1, fp) != 1)
goto out_free;
- ret = read_build_id(buf, buf_size, bf, size, need_swap);
+ ret = read_build_id(buf, buf_size, bid, need_swap);
if (ret == 0)
- ret = size;
+ ret = bid->size;
break;
}
} else {
@@ -207,9 +209,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
if (fread(buf, buf_size, 1, fp) != 1)
goto out_free;
- ret = read_build_id(buf, buf_size, bf, size, need_swap);
+ ret = read_build_id(buf, buf_size, bid, need_swap);
if (ret == 0)
- ret = size;
+ ret = bid->size;
break;
}
}
@@ -220,8 +222,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
return ret;
}
-int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
+int sysfs__read_build_id(const char *filename, void *build_id, size_t size __maybe_unused)
{
+ struct build_id bid;
int fd;
int ret = -1;
struct stat stbuf;
@@ -243,7 +246,9 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
if (read(fd, buf, buf_size) != (ssize_t) buf_size)
goto out_free;
- ret = read_build_id(buf, buf_size, build_id, size, false);
+ ret = read_build_id(buf, buf_size, &bid, false);
+ if (ret > 0)
+ memcpy(build_id, bid.data, bid.size);
out_free:
free(buf);
out:
@@ -339,16 +344,15 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
struct symsrc *runtime_ss __maybe_unused,
int kmodule __maybe_unused)
{
- unsigned char build_id[BUILD_ID_SIZE];
+ struct build_id bid;
int ret;
ret = fd__is_64_bit(ss->fd);
if (ret >= 0)
dso->is_64_bit = ret;
- if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
- dso__set_build_id(dso, build_id);
- }
+ if (filename__read_build_id(ss->name, &bid) > 0)
+ dso__set_build_id(dso, bid.data);
return 0;
}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index c772c8c70db5..4c43bb959fee 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1755,7 +1755,7 @@ int dso__load(struct dso *dso, struct map *map)
struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
bool kmod;
bool perfmap;
- unsigned char build_id[BUILD_ID_SIZE];
+ struct build_id bid;
struct nscookie nsc;
char newmapname[PATH_MAX];
const char *map_path = dso->long_name;
@@ -1817,8 +1817,8 @@ int dso__load(struct dso *dso, struct map *map)
if (!dso->has_build_id &&
is_regular_file(dso->long_name)) {
__symbol__join_symfs(name, PATH_MAX, dso->long_name);
- if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0)
- dso__set_build_id(dso, build_id);
+ if (filename__read_build_id(name, &bid) > 0)
+ dso__set_build_id(dso, bid.data);
}
/*
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 11fe71f46d14..98908fa3f796 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -23,6 +23,7 @@ struct dso;
struct map;
struct maps;
struct option;
+struct build_id;
/*
* libelf 0.8.x and earlier do not support ELF_C_READ_MMAP;
@@ -142,7 +143,7 @@ struct symbol *dso__next_symbol(struct symbol *sym);
enum dso_type dso__type_fd(int fd);
-int filename__read_build_id(const char *filename, void *bf, size_t size);
+int filename__read_build_id(const char *filename, struct build_id *id);
int sysfs__read_build_id(const char *filename, void *bf, size_t size);
int modules__parse(const char *filename, void *arg,
int (*process_module)(void *arg, const char *name,
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 4/9] perf tools: Pass build id object to sysfs__read_build_id
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
` (2 preceding siblings ...)
2020-09-30 17:15 ` [PATCH 3/9] perf tools: Pass build_id object to filename__read_build_id Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 5:20 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 5/9] perf tools: Pass build_id object to build_id__sprintf Jiri Olsa
` (4 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Passing build id object to sysfs__read_build_id function,
so it can populate the size of the build_id object.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/build-id.c | 6 +++---
tools/perf/util/dso.c | 6 ++----
tools/perf/util/symbol-elf.c | 11 +++++------
tools/perf/util/symbol-minimal.c | 7 ++-----
tools/perf/util/symbol.c | 7 +++----
tools/perf/util/symbol.h | 2 +-
6 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 62b258aaa128..1c332e78bbc3 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -113,7 +113,7 @@ int build_id__sprintf(const u8 *build_id, int len, char *bf)
int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
{
char notes[PATH_MAX];
- u8 build_id[BUILD_ID_SIZE];
+ struct build_id bid;
int ret;
if (!root_dir)
@@ -121,11 +121,11 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
scnprintf(notes, sizeof(notes), "%s/sys/kernel/notes", root_dir);
- ret = sysfs__read_build_id(notes, build_id, sizeof(build_id));
+ ret = sysfs__read_build_id(notes, &bid);
if (ret < 0)
return ret;
- return build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
+ return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
}
int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index d2c1ed08c879..e87fa9a71d9f 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1346,8 +1346,7 @@ void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
if (machine__is_default_guest(machine))
return;
sprintf(path, "%s/sys/kernel/notes", machine->root_dir);
- if (sysfs__read_build_id(path, dso->bid.data,
- sizeof(dso->bid.data)) == 0)
+ if (sysfs__read_build_id(path, &dso->bid) == 0)
dso->has_build_id = true;
}
@@ -1365,8 +1364,7 @@ int dso__kernel_module_get_build_id(struct dso *dso,
"%s/sys/module/%.*s/notes/.note.gnu.build-id",
root_dir, (int)strlen(name) - 1, name);
- if (sysfs__read_build_id(filename, dso->bid.data,
- sizeof(dso->bid.data)) == 0)
+ if (sysfs__read_build_id(filename, &dso->bid) == 0)
dso->has_build_id = true;
return 0;
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 61d7c444e6f5..97a55f162ea5 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -595,13 +595,11 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
#endif // HAVE_LIBBFD_BUILDID_SUPPORT
-int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
+int sysfs__read_build_id(const char *filename, struct build_id *bid)
{
+ size_t size = sizeof(bid->data);
int fd, err = -1;
- if (size < BUILD_ID_SIZE)
- goto out;
-
fd = open(filename, O_RDONLY);
if (fd < 0)
goto out;
@@ -622,8 +620,9 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
break;
if (memcmp(bf, "GNU", sizeof("GNU")) == 0) {
size_t sz = min(descsz, size);
- if (read(fd, build_id, sz) == (ssize_t)sz) {
- memset(build_id + sz, 0, size - sz);
+ if (read(fd, bid->data, sz) == (ssize_t)sz) {
+ memset(bid->data + sz, 0, size - sz);
+ bid->size = sz;
err = 0;
break;
}
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 5173331ee6e4..dba6b9e5d64e 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -222,9 +222,8 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
return ret;
}
-int sysfs__read_build_id(const char *filename, void *build_id, size_t size __maybe_unused)
+int sysfs__read_build_id(const char *filename, struct build_id *bid)
{
- struct build_id bid;
int fd;
int ret = -1;
struct stat stbuf;
@@ -246,9 +245,7 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size __may
if (read(fd, buf, buf_size) != (ssize_t) buf_size)
goto out_free;
- ret = read_build_id(buf, buf_size, &bid, false);
- if (ret > 0)
- memcpy(build_id, bid.data, bid.size);
+ ret = read_build_id(buf, buf_size, bid, false);
out_free:
free(buf);
out:
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4c43bb959fee..dd1cf91c62fb 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2122,7 +2122,7 @@ static bool filename__readable(const char *file)
static char *dso__find_kallsyms(struct dso *dso, struct map *map)
{
- u8 host_build_id[BUILD_ID_SIZE];
+ struct build_id bid;
char sbuild_id[SBUILD_ID_SIZE];
bool is_host = false;
char path[PATH_MAX];
@@ -2135,9 +2135,8 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
goto proc_kallsyms;
}
- if (sysfs__read_build_id("/sys/kernel/notes", host_build_id,
- sizeof(host_build_id)) == 0)
- is_host = dso__build_id_equal(dso, host_build_id);
+ if (sysfs__read_build_id("/sys/kernel/notes", &bid) == 0)
+ is_host = dso__build_id_equal(dso, bid.data);
/* Try a fast path for /proc/kallsyms if possible */
if (is_host) {
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 98908fa3f796..70b3874e7dd8 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -144,7 +144,7 @@ struct symbol *dso__next_symbol(struct symbol *sym);
enum dso_type dso__type_fd(int fd);
int filename__read_build_id(const char *filename, struct build_id *id);
-int sysfs__read_build_id(const char *filename, void *bf, size_t size);
+int sysfs__read_build_id(const char *filename, struct build_id *bid);
int modules__parse(const char *filename, void *arg,
int (*process_module)(void *arg, const char *name,
u64 start, u64 size));
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 5/9] perf tools: Pass build_id object to build_id__sprintf
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
` (3 preceding siblings ...)
2020-09-30 17:15 ` [PATCH 4/9] perf tools: Pass build id object to sysfs__read_build_id Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 5:24 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 6/9] perf tools: Pass build_id object to dso__set_build_id Jiri Olsa
` (3 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Passing build_id object to build_id__sprintf function,
so it can operate with the proper size of build id.
This will create proper md5 build id readable names,
like following:
a50e350e97c43b4708d09bcd85ebfff7
instead of:
a50e350e97c43b4708d09bcd85ebfff700000000
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/builtin-buildid-cache.c | 6 ++--
tools/perf/tests/sdt.c | 2 +-
tools/perf/util/annotate.c | 3 +-
tools/perf/util/build-id.c | 30 ++++++++++++-------
tools/perf/util/build-id.h | 3 +-
tools/perf/util/dso.c | 6 ++--
| 3 +-
tools/perf/util/map.c | 4 +--
tools/perf/util/probe-event.c | 9 ++++--
tools/perf/util/probe-finder.c | 8 +++--
.../scripting-engines/trace-event-python.c | 2 +-
tools/perf/util/symbol.c | 2 +-
12 files changed, 43 insertions(+), 35 deletions(-)
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index c3cb168d546d..a25411926e48 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -186,7 +186,7 @@ static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
return -1;
}
- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
err = build_id_cache__add_s(sbuild_id, filename, nsi,
false, false);
pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
@@ -210,7 +210,7 @@ static int build_id_cache__remove_file(const char *filename, struct nsinfo *nsi)
return -1;
}
- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
err = build_id_cache__remove_s(sbuild_id);
pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
err ? "FAIL" : "Ok");
@@ -314,7 +314,7 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
}
err = 0;
- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
if (build_id_cache__cached(sbuild_id))
err = build_id_cache__remove_s(sbuild_id);
diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
index 3ef37f739203..ed76c693f65e 100644
--- a/tools/perf/tests/sdt.c
+++ b/tools/perf/tests/sdt.c
@@ -37,7 +37,7 @@ static int build_id_cache__add_file(const char *filename)
return err;
}
- build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ build_id__sprintf(&bid, sbuild_id);
err = build_id_cache__add_s(sbuild_id, filename, NULL, false, false);
if (err < 0)
pr_debug("Failed to add build id cache of %s\n", filename);
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index a016e1bd7b8d..6c8575e182ed 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1578,8 +1578,7 @@ int symbol__strerror_disassemble(struct map_symbol *ms, int errnum, char *buf, s
char *build_id_msg = NULL;
if (dso->has_build_id) {
- build_id__sprintf(dso->bid.data,
- sizeof(dso->bid.data), bf + 15);
+ build_id__sprintf(&dso->bid, bf + 15);
build_id_msg = bf;
}
scnprintf(buf, buflen,
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 1c332e78bbc3..b5648735f01f 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -37,6 +37,7 @@
#include <linux/ctype.h>
#include <linux/zalloc.h>
+#include <asm/bug.h>
static bool no_buildid_cache;
@@ -95,13 +96,13 @@ struct perf_tool build_id__mark_dso_hit_ops = {
.ordered_events = true,
};
-int build_id__sprintf(const u8 *build_id, int len, char *bf)
+int build_id__sprintf(const struct build_id *build_id, char *bf)
{
char *bid = bf;
- const u8 *raw = build_id;
- int i;
+ const u8 *raw = build_id->data;
+ size_t i;
- for (i = 0; i < len; ++i) {
+ for (i = 0; i < build_id->size; ++i) {
sprintf(bid, "%02x", *raw);
++raw;
bid += 2;
@@ -125,7 +126,7 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
if (ret < 0)
return ret;
- return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ return build_id__sprintf(&bid, sbuild_id);
}
int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
@@ -137,7 +138,7 @@ int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
if (ret < 0)
return ret;
- return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
+ return build_id__sprintf(&bid, sbuild_id);
}
/* asnprintf consolidates asprintf and snprintf */
@@ -270,7 +271,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size,
if (!dso->has_build_id)
return NULL;
- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
if (!linkname)
return NULL;
@@ -767,13 +768,13 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
return err;
}
-static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
+static int build_id_cache__add_b(const struct build_id *bid,
const char *name, struct nsinfo *nsi,
bool is_kallsyms, bool is_vdso)
{
char sbuild_id[SBUILD_ID_SIZE];
- build_id__sprintf(build_id, build_id_size, sbuild_id);
+ build_id__sprintf(bid, sbuild_id);
return build_id_cache__add_s(sbuild_id, name, nsi, is_kallsyms,
is_vdso);
@@ -839,8 +840,8 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)
is_kallsyms = true;
name = machine->mmap_name;
}
- return build_id_cache__add_b(dso->bid.data, sizeof(dso->bid.data), name,
- dso->nsinfo, is_kallsyms, is_vdso);
+ return build_id_cache__add_b(&dso->bid, name, dso->nsinfo,
+ is_kallsyms, is_vdso);
}
static int __dsos__cache_build_ids(struct list_head *head,
@@ -900,3 +901,10 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits)
return ret;
}
+
+void build_id__init(struct build_id *bid, const u8 *data, size_t size)
+{
+ WARN_ON(size > BUILD_ID_SIZE);
+ memcpy(bid->data, data, size);
+ bid->size = size;
+}
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index cc7fc62303ee..5e08e991defc 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -19,7 +19,8 @@ extern struct perf_tool build_id__mark_dso_hit_ops;
struct dso;
struct feat_fd;
-int build_id__sprintf(const u8 *build_id, int len, char *bf);
+void build_id__init(struct build_id *bid, const u8 *data, size_t size);
+int build_id__sprintf(const struct build_id *build_id, char *bf);
int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id);
int filename__sprintf_build_id(const char *pathname, char *sbuild_id);
char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf,
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index e87fa9a71d9f..2f7f01ead9a1 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -172,9 +172,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
break;
}
- build_id__sprintf(dso->bid.data,
- sizeof(dso->bid.data),
- build_id_hex);
+ build_id__sprintf(&dso->bid, build_id_hex);
len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
snprintf(filename + len, size - len, "%.2s/%s.debug",
build_id_hex, build_id_hex + 2);
@@ -1374,7 +1372,7 @@ size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
{
char sbuild_id[SBUILD_ID_SIZE];
- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
return fprintf(fp, "%s", sbuild_id);
}
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index cde8f6eba479..fe220f01fc94 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2095,8 +2095,7 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
free(m.name);
}
- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data),
- sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
pr_debug("build id event received for %s: %s\n",
dso->long_name, sbuild_id);
dso__put(dso);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index d08540193822..596cc8e2c167 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -346,9 +346,7 @@ int map__load(struct map *map)
if (map->dso->has_build_id) {
char sbuild_id[SBUILD_ID_SIZE];
- build_id__sprintf(map->dso->bid.data,
- sizeof(map->dso->bid.data),
- sbuild_id);
+ build_id__sprintf(&map->dso->bid, sbuild_id);
pr_debug("%s with build id %s not found", name, sbuild_id);
} else
pr_debug("Failed to open %s", name);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2041ad849851..8eae2afff71a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -473,7 +473,7 @@ static struct debuginfo *open_from_debuginfod(struct dso *dso, struct nsinfo *ns
if (!c)
return NULL;
- build_id__sprintf(dso->bid.data, BUILD_ID_SIZE, sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
fd = debuginfod_find_debuginfo(c, (const unsigned char *)sbuild_id,
0, &path);
if (fd >= 0)
@@ -1005,6 +1005,7 @@ static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
static int __show_line_range(struct line_range *lr, const char *module,
bool user)
{
+ struct build_id bid;
int l = 1;
struct int_node *ln;
struct debuginfo *dinfo;
@@ -1025,8 +1026,10 @@ static int __show_line_range(struct line_range *lr, const char *module,
if (!ret)
ret = debuginfo__find_line_range(dinfo, lr);
}
- if (dinfo->build_id)
- build_id__sprintf(dinfo->build_id, BUILD_ID_SIZE, sbuild_id);
+ if (dinfo->build_id) {
+ build_id__init(&bid, dinfo->build_id, BUILD_ID_SIZE);
+ build_id__sprintf(&bid, sbuild_id);
+ }
debuginfo__delete(dinfo);
if (ret == 0 || ret == -ENOENT) {
pr_warning("Specified source line is not found.\n");
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 6eddf7be8293..2c4061035f77 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -949,6 +949,7 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
/* Find probe points from lazy pattern */
static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
{
+ struct build_id bid;
char sbuild_id[SBUILD_ID_SIZE] = "";
int ret = 0;
char *fpath;
@@ -957,9 +958,10 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
const char *comp_dir;
comp_dir = cu_get_comp_dir(&pf->cu_die);
- if (pf->dbg->build_id)
- build_id__sprintf(pf->dbg->build_id,
- BUILD_ID_SIZE, sbuild_id);
+ if (pf->dbg->build_id) {
+ build_id__init(&bid, pf->dbg->build_id, BUILD_ID_SIZE);
+ build_id__sprintf(&bid, sbuild_id);
+ }
ret = find_source_path(pf->fname, sbuild_id, comp_dir, &fpath);
if (ret < 0) {
pr_warning("Failed to find source file path.\n");
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index db3b1c275d8f..7cbd024e3e63 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -1064,7 +1064,7 @@ static int python_export_dso(struct db_export *dbe, struct dso *dso,
char sbuild_id[SBUILD_ID_SIZE];
PyObject *t;
- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
t = tuple_new(5);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index dd1cf91c62fb..369cbad09f0d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2152,7 +2152,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
goto proc_kallsyms;
}
- build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
+ build_id__sprintf(&dso->bid, sbuild_id);
/* Find kallsyms in build-id cache with kcore */
scnprintf(path, sizeof(path), "%s/%s/%s",
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 6/9] perf tools: Pass build_id object to dso__set_build_id
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
` (4 preceding siblings ...)
2020-09-30 17:15 ` [PATCH 5/9] perf tools: Pass build_id object to build_id__sprintf Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 5:25 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 7/9] perf tools: Pass build_id object to dso__build_id_equal Jiri Olsa
` (2 subsequent siblings)
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Passing build_id object to dso__set_build_id, so it's easier
to initialize dos's build id object.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/dso.c | 4 ++--
tools/perf/util/dso.h | 2 +-
| 4 +++-
tools/perf/util/symbol-minimal.c | 2 +-
tools/perf/util/symbol.c | 2 +-
5 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 2f7f01ead9a1..4415ce83150b 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1326,9 +1326,9 @@ void dso__put(struct dso *dso)
dso__delete(dso);
}
-void dso__set_build_id(struct dso *dso, void *build_id)
+void dso__set_build_id(struct dso *dso, struct build_id *bid)
{
- memcpy(dso->bid.data, build_id, sizeof(dso->bid.data));
+ dso->bid = *bid;
dso->has_build_id = 1;
}
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index eac004210b47..5a5678dbdaa5 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -260,7 +260,7 @@ bool dso__sorted_by_name(const struct dso *dso);
void dso__set_sorted_by_name(struct dso *dso);
void dso__sort_by_name(struct dso *dso);
-void dso__set_build_id(struct dso *dso, void *build_id);
+void dso__set_build_id(struct dso *dso, struct build_id *bid);
bool dso__build_id_equal(const struct dso *dso, u8 *build_id);
void dso__read_running_kernel_build_id(struct dso *dso,
struct machine *machine);
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index fe220f01fc94..21243adbb9fd 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2082,8 +2082,10 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
dso = machine__findnew_dso(machine, filename);
if (dso != NULL) {
char sbuild_id[SBUILD_ID_SIZE];
+ struct build_id bid;
- dso__set_build_id(dso, &bev->build_id);
+ build_id__init(&bid, bev->build_id, BUILD_ID_SIZE);
+ dso__set_build_id(dso, &bid);
if (dso_space != DSO_SPACE__USER) {
struct kmod_path m = { .name = NULL, };
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index dba6b9e5d64e..f9eb0bee7f15 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -349,7 +349,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
dso->is_64_bit = ret;
if (filename__read_build_id(ss->name, &bid) > 0)
- dso__set_build_id(dso, bid.data);
+ dso__set_build_id(dso, &bid);
return 0;
}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 369cbad09f0d..976632d0baa0 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1818,7 +1818,7 @@ int dso__load(struct dso *dso, struct map *map)
is_regular_file(dso->long_name)) {
__symbol__join_symfs(name, PATH_MAX, dso->long_name);
if (filename__read_build_id(name, &bid) > 0)
- dso__set_build_id(dso, bid.data);
+ dso__set_build_id(dso, &bid);
}
/*
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 7/9] perf tools: Pass build_id object to dso__build_id_equal
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
` (5 preceding siblings ...)
2020-09-30 17:15 ` [PATCH 6/9] perf tools: Pass build_id object to dso__set_build_id Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 5:25 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 8/9] perf tools: Add size to struct perf_record_header_build_id Jiri Olsa
2020-09-30 17:15 ` [PATCH 9/9] perf tools: Align buildid list output for short build ids Jiri Olsa
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Passing build_id object to dso__build_id_equal, so we can
properly check build id with different size than sha1.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/dso.c | 5 +++--
tools/perf/util/dso.h | 2 +-
tools/perf/util/symbol-elf.c | 8 ++++++--
tools/perf/util/symbol.c | 2 +-
4 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 4415ce83150b..ca965845b35e 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1332,9 +1332,10 @@ void dso__set_build_id(struct dso *dso, struct build_id *bid)
dso->has_build_id = 1;
}
-bool dso__build_id_equal(const struct dso *dso, u8 *build_id)
+bool dso__build_id_equal(const struct dso *dso, struct build_id *bid)
{
- return memcmp(dso->bid.data, build_id, sizeof(dso->bid.data)) == 0;
+ return dso->bid.size == bid->size &&
+ memcmp(dso->bid.data, bid->data, dso->bid.size) == 0;
}
void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 5a5678dbdaa5..f926c96bf230 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -261,7 +261,7 @@ void dso__set_sorted_by_name(struct dso *dso);
void dso__sort_by_name(struct dso *dso);
void dso__set_build_id(struct dso *dso, struct build_id *bid);
-bool dso__build_id_equal(const struct dso *dso, u8 *build_id);
+bool dso__build_id_equal(const struct dso *dso, struct build_id *bid);
void dso__read_running_kernel_build_id(struct dso *dso,
struct machine *machine);
int dso__kernel_module_get_build_id(struct dso *dso, const char *root_dir);
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 97a55f162ea5..44dd86a4f25f 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -834,13 +834,17 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
/* Always reject images with a mismatched build-id: */
if (dso->has_build_id && !symbol_conf.ignore_vmlinux_buildid) {
u8 build_id[BUILD_ID_SIZE];
+ struct build_id bid;
+ int size;
- if (elf_read_build_id(elf, build_id, BUILD_ID_SIZE) < 0) {
+ size = elf_read_build_id(elf, build_id, BUILD_ID_SIZE);
+ if (size <= 0) {
dso->load_errno = DSO_LOAD_ERRNO__CANNOT_READ_BUILDID;
goto out_elf_end;
}
- if (!dso__build_id_equal(dso, build_id)) {
+ build_id__init(&bid, build_id, size);
+ if (!dso__build_id_equal(dso, &bid)) {
pr_debug("%s: build id mismatch for %s.\n", __func__, name);
dso->load_errno = DSO_LOAD_ERRNO__MISMATCHING_BUILDID;
goto out_elf_end;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 976632d0baa0..6138866665df 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2136,7 +2136,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
}
if (sysfs__read_build_id("/sys/kernel/notes", &bid) == 0)
- is_host = dso__build_id_equal(dso, bid.data);
+ is_host = dso__build_id_equal(dso, &bid);
/* Try a fast path for /proc/kallsyms if possible */
if (is_host) {
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 8/9] perf tools: Add size to struct perf_record_header_build_id
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
` (6 preceding siblings ...)
2020-09-30 17:15 ` [PATCH 7/9] perf tools: Pass build_id object to dso__build_id_equal Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 5:31 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 9/9] perf tools: Align buildid list output for short build ids Jiri Olsa
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
We do not store size with build ids in perf data,
but there's enough space to do it. Adding misc bit
PERF_RECORD_MISC_BUILD_ID_SIZE to mark build id event
with size.
With this fix the dso with md5 build id will have correct
build id data and will be usable for debuginfod processing
if needed (coming in following patches).
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/lib/perf/include/perf/event.h | 12 +++++++++++-
tools/perf/util/build-id.c | 8 +++++---
| 10 +++++++---
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
index a6dbba6b9073..988c539bedb6 100644
--- a/tools/lib/perf/include/perf/event.h
+++ b/tools/lib/perf/include/perf/event.h
@@ -201,10 +201,20 @@ struct perf_record_header_tracing_data {
__u32 size;
};
+#define PERF_RECORD_MISC_BUILD_ID_SIZE (1 << 15)
+
struct perf_record_header_build_id {
struct perf_event_header header;
pid_t pid;
- __u8 build_id[24];
+ union {
+ __u8 build_id[24];
+ struct {
+ __u8 data[20];
+ __u8 size;
+ __u8 reserved1__;
+ __u16 reserved2__;
+ };
+ };
char filename[];
};
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index b5648735f01f..8763772f1095 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -296,7 +296,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size,
continue; \
else
-static int write_buildid(const char *name, size_t name_len, u8 *build_id,
+static int write_buildid(const char *name, size_t name_len, struct build_id *bid,
pid_t pid, u16 misc, struct feat_fd *fd)
{
int err;
@@ -307,7 +307,9 @@ static int write_buildid(const char *name, size_t name_len, u8 *build_id,
len = PERF_ALIGN(len, NAME_ALIGN);
memset(&b, 0, sizeof(b));
- memcpy(&b.build_id, build_id, BUILD_ID_SIZE);
+ memcpy(&b.data, bid->data, bid->size);
+ b.size = (u8) bid->size;
+ misc |= PERF_RECORD_MISC_BUILD_ID_SIZE;
b.pid = pid;
b.header.misc = misc;
b.header.size = sizeof(b) + len;
@@ -354,7 +356,7 @@ static int machine__write_buildid_table(struct machine *machine,
in_kernel = pos->kernel ||
is_kernel_module(name,
PERF_RECORD_MISC_CPUMODE_UNKNOWN);
- err = write_buildid(name, name_len, pos->bid.data, machine->pid,
+ err = write_buildid(name, name_len, &pos->bid, machine->pid,
in_kernel ? kmisc : umisc, fd);
if (err)
break;
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 21243adbb9fd..8da3886f10a8 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2083,8 +2083,12 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
if (dso != NULL) {
char sbuild_id[SBUILD_ID_SIZE];
struct build_id bid;
+ size_t size = BUILD_ID_SIZE;
- build_id__init(&bid, bev->build_id, BUILD_ID_SIZE);
+ if (bev->header.misc & PERF_RECORD_MISC_BUILD_ID_SIZE)
+ size = bev->size;
+
+ build_id__init(&bid, bev->data, size);
dso__set_build_id(dso, &bid);
if (dso_space != DSO_SPACE__USER) {
@@ -2098,8 +2102,8 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
}
build_id__sprintf(&dso->bid, sbuild_id);
- pr_debug("build id event received for %s: %s\n",
- dso->long_name, sbuild_id);
+ pr_debug("build id event received for %s: %s [%lu]\n",
+ dso->long_name, sbuild_id, size);
dso__put(dso);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 9/9] perf tools: Align buildid list output for short build ids
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
` (7 preceding siblings ...)
2020-09-30 17:15 ` [PATCH 8/9] perf tools: Add size to struct perf_record_header_build_id Jiri Olsa
@ 2020-09-30 17:15 ` Jiri Olsa
2020-10-01 5:35 ` Ian Rogers
8 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-09-30 17:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
With shorter md5 build ids we need to align their
paths properly with other build ids:
$ perf buildid-list
17f4e448cc746582ea1881528deb549f7fdb3fd5 [kernel.kallsyms]
a50e350e97c43b4708d09bcd85ebfff7 .../tools/perf/buildid-ex-md5
1805c738c8f3ec0f47b7ea09080c28f34d18a82b /usr/lib64/ld-2.31.so
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/dso.c | 2 +-
tools/perf/util/dso.h | 1 -
tools/perf/util/dsos.c | 6 ++++--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index ca965845b35e..55c11e854fe4 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1369,7 +1369,7 @@ int dso__kernel_module_get_build_id(struct dso *dso,
return 0;
}
-size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
+static size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
{
char sbuild_id[SBUILD_ID_SIZE];
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index f926c96bf230..d8cb4f5680a4 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -362,7 +362,6 @@ struct dso *machine__findnew_kernel(struct machine *machine, const char *name,
void dso__reset_find_symbol_cache(struct dso *dso);
-size_t dso__fprintf_buildid(struct dso *dso, FILE *fp);
size_t dso__fprintf_symbols_by_name(struct dso *dso, FILE *fp);
size_t dso__fprintf(struct dso *dso, FILE *fp);
diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index 87161e431830..183a81d5b2f9 100644
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -287,10 +287,12 @@ size_t __dsos__fprintf_buildid(struct list_head *head, FILE *fp,
size_t ret = 0;
list_for_each_entry(pos, head, node) {
+ char sbuild_id[SBUILD_ID_SIZE];
+
if (skip && skip(pos, parm))
continue;
- ret += dso__fprintf_buildid(pos, fp);
- ret += fprintf(fp, " %s\n", pos->long_name);
+ build_id__sprintf(&pos->bid, sbuild_id);
+ ret += fprintf(fp, "%-40s %s\n", sbuild_id, pos->long_name);
}
return ret;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] perf tools: Add build id shell test
2020-09-30 17:15 ` [PATCH 1/9] perf tools: Add build id shell test Jiri Olsa
@ 2020-10-01 2:00 ` Ian Rogers
2020-10-01 9:25 ` Jiri Olsa
2020-10-01 19:05 ` [PATCHv2 " Jiri Olsa
2020-10-13 16:13 ` [PATCH " Arnaldo Carvalho de Melo
2 siblings, 1 reply; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 2:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding test for build id cache that adds binary
> with sha1 and md5 build ids and verifies it's
> added properly.
>
> The test updates build id cache with perf record
> and perf buildid-cache -a.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
This is great! If I build perf and test from the build directory the
test gets run. If I build using O=/tmp/perf and run from that
directory then ./tests/shell isn't found and the test doesn't run.
Similarly the install directory doesn't contain the executables and so
the test is skipped. Is there any way to get the test running in these
other scenarios?
Thanks,
Ian
> ---
> tools/perf/Makefile.perf | 14 +++++
> tools/perf/tests/shell/buildid.sh | 90 +++++++++++++++++++++++++++++++
> 2 files changed, 104 insertions(+)
> create mode 100755 tools/perf/tests/shell/buildid.sh
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 920d8afb9238..b2aeefa64e92 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
> #
> # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
> #
> +# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
> +#
>
> # As per kernel Makefile, avoid funny character set dependencies
> unexport LC_ALL
> @@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
> PROGRAMS += $(OUTPUT)perf-read-vdsox32
> endif
>
> +ifndef NO_BUILDID_EX
> +PROGRAMS += $(OUTPUT)buildid-ex-sha1
> +PROGRAMS += $(OUTPUT)buildid-ex-md5
> +endif
> +
> LIBJVMTI = libperf-jvmti.so
>
> ifndef NO_JVMTI
> @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> endif
>
> +ifndef NO_BUILDID_EX
> +$(OUTPUT)buildid-ex-sha1:
> + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> +$(OUTPUT)buildid-ex-md5:
> + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> +endif
> +
> ifndef NO_JVMTI
> LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
>
> diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> new file mode 100755
> index 000000000000..57fcd28bc4bd
> --- /dev/null
> +++ b/tools/perf/tests/shell/buildid.sh
> @@ -0,0 +1,90 @@
> +#!/bin/sh
> +# build id cache operations
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# skip if there are no test binaries
> +if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
> + echo "failed: no test binaries"
> + exit 2
> +fi
> +
> +# skip if there's no readelf
> +if [ ! -x `which readelf` ]; then
> + echo "failed: no readelf, install binutils"
> + exit 2
> +fi
> +
> +check()
> +{
> + id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> +
> + echo "build id: ${id}"
> +
> + link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> + echo "link: ${link}"
> +
> + if [ ! -h $link ]; then
> + echo "failed: link ${link} does not exist"
> + exit 1
> + fi
> +
> + file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
> + echo "file: ${file}"
> +
> + if [ ! -x $file ]; then
> + echo "failed: file ${file} does not exist"
> + exit 1
> + fi
> +
> + diff ${file} ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: ${file} do not match"
> + exit 1
> + fi
> +
> + echo "OK for ${1}"
> +}
> +
> +test_add()
> +{
> + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> + perf="perf --buildid-dir ${build_id_dir}"
> +
> + ${perf} buildid-cache -v -a ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: add ${1} to build id cache"
> + exit 1
> + fi
> +
> + check ${1}
> +
> + rm -rf ${build_id_dir}
> +}
> +
> +test_record()
> +{
> + data=$(mktemp /tmp/perf.data.XXX)
> + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> + perf="perf --buildid-dir ${build_id_dir}"
> +
> + ${perf} record --buildid-all -o ${data} ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: record ${1}"
> + exit 1
> + fi
> +
> + check ${1}
> +
> + rm -rf ${build_id_dir}
> + rm -rf ${data}
> +}
> +
> +# add binaries manual via perf buildid-cache -a
> +test_add buildid-ex-sha1
> +test_add buildid-ex-md5
> +
> +# add binaries via perf record post processing
> +test_record buildid-ex-sha1
> +test_record buildid-ex-md5
> +
> +exit ${err}
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/9] perf tools: Use build_id object in dso
2020-09-30 17:15 ` [PATCH 2/9] perf tools: Use build_id object in dso Jiri Olsa
@ 2020-10-01 2:01 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 2:01 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Replace build_id byte array with struct build_id
> object and all the code that references it.
>
> The objective is to carry size together with build
> id array, so it's better to keep both together.
>
> This is preparatory change for following patches,
> and there's no functional change.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
This reads a little funny but makes sense in the context of the rest
of the patch set.
Thanks,
Ian
> ---
> tools/perf/builtin-buildid-cache.c | 2 +-
> tools/perf/builtin-inject.c | 4 ++--
> tools/perf/util/annotate.c | 4 ++--
> tools/perf/util/build-id.c | 6 +++---
> tools/perf/util/build-id.h | 5 +++++
> tools/perf/util/dso.c | 18 +++++++++---------
> tools/perf/util/dso.h | 2 +-
> tools/perf/util/dsos.c | 4 ++--
> tools/perf/util/header.c | 2 +-
> tools/perf/util/map.c | 4 ++--
> tools/perf/util/probe-event.c | 2 +-
> .../scripting-engines/trace-event-python.c | 2 +-
> tools/perf/util/symbol.c | 2 +-
> tools/perf/util/synthetic-events.c | 2 +-
> 14 files changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index 39efa51d7fb3..a523c629f321 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -284,7 +284,7 @@ static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
>
> pr_warning("Problems with %s file, consider removing it from the cache\n",
> filename);
> - } else if (memcmp(dso->build_id, build_id, sizeof(dso->build_id))) {
> + } else if (memcmp(dso->bid.data, build_id, sizeof(dso->bid.data))) {
> pr_warning("Problems with %s file, consider removing it from the cache\n",
> filename);
> }
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 6d2f410d773a..5cf32d8e3aa6 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -408,8 +408,8 @@ static int dso__read_build_id(struct dso *dso)
> if (dso->has_build_id)
> return 0;
>
> - if (filename__read_build_id(dso->long_name, dso->build_id,
> - sizeof(dso->build_id)) > 0) {
> + if (filename__read_build_id(dso->long_name, dso->bid.data,
> + sizeof(dso->bid.data)) > 0) {
> dso->has_build_id = true;
> return 0;
> }
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index fc17af7ba845..a016e1bd7b8d 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1578,8 +1578,8 @@ int symbol__strerror_disassemble(struct map_symbol *ms, int errnum, char *buf, s
> char *build_id_msg = NULL;
>
> if (dso->has_build_id) {
> - build_id__sprintf(dso->build_id,
> - sizeof(dso->build_id), bf + 15);
> + build_id__sprintf(dso->bid.data,
> + sizeof(dso->bid.data), bf + 15);
> build_id_msg = bf;
> }
> scnprintf(buf, buflen,
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 31207b6e2066..7da13ddb0d50 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -272,7 +272,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size,
> if (!dso->has_build_id)
> return NULL;
>
> - build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
> + build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
> linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
> if (!linkname)
> return NULL;
> @@ -355,7 +355,7 @@ static int machine__write_buildid_table(struct machine *machine,
> in_kernel = pos->kernel ||
> is_kernel_module(name,
> PERF_RECORD_MISC_CPUMODE_UNKNOWN);
> - err = write_buildid(name, name_len, pos->build_id, machine->pid,
> + err = write_buildid(name, name_len, pos->bid.data, machine->pid,
> in_kernel ? kmisc : umisc, fd);
> if (err)
> break;
> @@ -841,7 +841,7 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)
> is_kallsyms = true;
> name = machine->mmap_name;
> }
> - return build_id_cache__add_b(dso->build_id, sizeof(dso->build_id), name,
> + return build_id_cache__add_b(dso->bid.data, sizeof(dso->bid.data), name,
> dso->nsinfo, is_kallsyms, is_vdso);
> }
>
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index aad419bb165c..cc7fc62303ee 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -8,6 +8,11 @@
> #include "tool.h"
> #include <linux/types.h>
>
> +struct build_id {
> + u8 data[BUILD_ID_SIZE];
> + size_t size;
> +};
> +
> struct nsinfo;
>
> extern struct perf_tool build_id__mark_dso_hit_ops;
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 5a3b4755f0b3..d2c1ed08c879 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -172,8 +172,8 @@ int dso__read_binary_type_filename(const struct dso *dso,
> break;
> }
>
> - build_id__sprintf(dso->build_id,
> - sizeof(dso->build_id),
> + build_id__sprintf(dso->bid.data,
> + sizeof(dso->bid.data),
> build_id_hex);
> len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
> snprintf(filename + len, size - len, "%.2s/%s.debug",
> @@ -1330,13 +1330,13 @@ void dso__put(struct dso *dso)
>
> void dso__set_build_id(struct dso *dso, void *build_id)
> {
> - memcpy(dso->build_id, build_id, sizeof(dso->build_id));
> + memcpy(dso->bid.data, build_id, sizeof(dso->bid.data));
> dso->has_build_id = 1;
> }
>
> bool dso__build_id_equal(const struct dso *dso, u8 *build_id)
> {
> - return memcmp(dso->build_id, build_id, sizeof(dso->build_id)) == 0;
> + return memcmp(dso->bid.data, build_id, sizeof(dso->bid.data)) == 0;
> }
>
> void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
> @@ -1346,8 +1346,8 @@ void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
> if (machine__is_default_guest(machine))
> return;
> sprintf(path, "%s/sys/kernel/notes", machine->root_dir);
> - if (sysfs__read_build_id(path, dso->build_id,
> - sizeof(dso->build_id)) == 0)
> + if (sysfs__read_build_id(path, dso->bid.data,
> + sizeof(dso->bid.data)) == 0)
> dso->has_build_id = true;
> }
>
> @@ -1365,8 +1365,8 @@ int dso__kernel_module_get_build_id(struct dso *dso,
> "%s/sys/module/%.*s/notes/.note.gnu.build-id",
> root_dir, (int)strlen(name) - 1, name);
>
> - if (sysfs__read_build_id(filename, dso->build_id,
> - sizeof(dso->build_id)) == 0)
> + if (sysfs__read_build_id(filename, dso->bid.data,
> + sizeof(dso->bid.data)) == 0)
> dso->has_build_id = true;
>
> return 0;
> @@ -1376,7 +1376,7 @@ size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
> {
> char sbuild_id[SBUILD_ID_SIZE];
>
> - build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
> + build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
> return fprintf(fp, "%s", sbuild_id);
> }
>
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 8ad17f395a19..eac004210b47 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -176,7 +176,7 @@ struct dso {
> bool sorted_by_name;
> bool loaded;
> u8 rel;
> - u8 build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> u64 text_offset;
> const char *short_name;
> const char *long_name;
> diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
> index 939471731ea6..e3af46e818f1 100644
> --- a/tools/perf/util/dsos.c
> +++ b/tools/perf/util/dsos.c
> @@ -73,8 +73,8 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
> continue;
> }
> nsinfo__mountns_enter(pos->nsinfo, &nsc);
> - if (filename__read_build_id(pos->long_name, pos->build_id,
> - sizeof(pos->build_id)) > 0) {
> + if (filename__read_build_id(pos->long_name, pos->bid.data,
> + sizeof(pos->bid.data)) > 0) {
> have_build_id = true;
> pos->has_build_id = true;
> }
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 9cf4efdcbbbd..cde8f6eba479 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2095,7 +2095,7 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
> free(m.name);
> }
>
> - build_id__sprintf(dso->build_id, sizeof(dso->build_id),
> + build_id__sprintf(dso->bid.data, sizeof(dso->bid.data),
> sbuild_id);
> pr_debug("build id event received for %s: %s\n",
> dso->long_name, sbuild_id);
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index cc0faf8f1321..d08540193822 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -346,8 +346,8 @@ int map__load(struct map *map)
> if (map->dso->has_build_id) {
> char sbuild_id[SBUILD_ID_SIZE];
>
> - build_id__sprintf(map->dso->build_id,
> - sizeof(map->dso->build_id),
> + build_id__sprintf(map->dso->bid.data,
> + sizeof(map->dso->bid.data),
> sbuild_id);
> pr_debug("%s with build id %s not found", name, sbuild_id);
> } else
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 3a1b58a92673..2041ad849851 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -473,7 +473,7 @@ static struct debuginfo *open_from_debuginfod(struct dso *dso, struct nsinfo *ns
> if (!c)
> return NULL;
>
> - build_id__sprintf(dso->build_id, BUILD_ID_SIZE, sbuild_id);
> + build_id__sprintf(dso->bid.data, BUILD_ID_SIZE, sbuild_id);
> fd = debuginfod_find_debuginfo(c, (const unsigned char *)sbuild_id,
> 0, &path);
> if (fd >= 0)
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 739516fdf6e3..db3b1c275d8f 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1064,7 +1064,7 @@ static int python_export_dso(struct db_export *dbe, struct dso *dso,
> char sbuild_id[SBUILD_ID_SIZE];
> PyObject *t;
>
> - build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
> + build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
>
> t = tuple_new(5);
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 5ddf76fb691c..c772c8c70db5 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2153,7 +2153,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
> goto proc_kallsyms;
> }
>
> - build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
> + build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
>
> /* Find kallsyms in build-id cache with kcore */
> scnprintf(path, sizeof(path), "%s/%s/%s",
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 3ca5d9399680..8a23391558cf 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -1961,7 +1961,7 @@ int perf_event__synthesize_build_id(struct perf_tool *tool, struct dso *pos, u16
>
> len = pos->long_name_len + 1;
> len = PERF_ALIGN(len, NAME_ALIGN);
> - memcpy(&ev.build_id.build_id, pos->build_id, sizeof(pos->build_id));
> + memcpy(&ev.build_id.build_id, pos->bid.data, sizeof(pos->bid.data));
> ev.build_id.header.type = PERF_RECORD_HEADER_BUILD_ID;
> ev.build_id.header.misc = misc;
> ev.build_id.pid = machine->pid;
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 3/9] perf tools: Pass build_id object to filename__read_build_id
2020-09-30 17:15 ` [PATCH 3/9] perf tools: Pass build_id object to filename__read_build_id Jiri Olsa
@ 2020-10-01 5:17 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 5:17 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Passing build_id object to filename__read_build_id function,
> so it can populate the size of the build_id object.
>
> Changing filename__read_build_id code for both elf/non-elf
> code.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/builtin-buildid-cache.c | 25 +++++++++++-----------
> tools/perf/builtin-inject.c | 3 +--
> tools/perf/tests/pe-file-parsing.c | 10 ++++-----
> tools/perf/tests/sdt.c | 6 +++---
> tools/perf/util/build-id.c | 8 +++----
> tools/perf/util/dsos.c | 3 +--
> tools/perf/util/symbol-elf.c | 16 ++++++++------
> tools/perf/util/symbol-minimal.c | 34 +++++++++++++++++-------------
> tools/perf/util/symbol.c | 6 +++---
> tools/perf/util/symbol.h | 3 ++-
> 10 files changed, 59 insertions(+), 55 deletions(-)
>
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index a523c629f321..c3cb168d546d 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -174,19 +174,19 @@ static int build_id_cache__add_kcore(const char *filename, bool force)
> static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
> {
> char sbuild_id[SBUILD_ID_SIZE];
> - u8 build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> int err;
> struct nscookie nsc;
>
> nsinfo__mountns_enter(nsi, &nsc);
> - err = filename__read_build_id(filename, &build_id, sizeof(build_id));
> + err = filename__read_build_id(filename, &bid);
> nsinfo__mountns_exit(&nsc);
> if (err < 0) {
> pr_debug("Couldn't read a build-id in %s\n", filename);
> return -1;
> }
>
> - build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> + build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> err = build_id_cache__add_s(sbuild_id, filename, nsi,
> false, false);
> pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
> @@ -196,21 +196,21 @@ static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
>
> static int build_id_cache__remove_file(const char *filename, struct nsinfo *nsi)
> {
> - u8 build_id[BUILD_ID_SIZE];
> char sbuild_id[SBUILD_ID_SIZE];
> + struct build_id bid;
> struct nscookie nsc;
>
> int err;
>
> nsinfo__mountns_enter(nsi, &nsc);
> - err = filename__read_build_id(filename, &build_id, sizeof(build_id));
> + err = filename__read_build_id(filename, &bid);
> nsinfo__mountns_exit(&nsc);
> if (err < 0) {
> pr_debug("Couldn't read a build-id in %s\n", filename);
> return -1;
> }
>
> - build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> + build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> err = build_id_cache__remove_s(sbuild_id);
> pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
> err ? "FAIL" : "Ok");
> @@ -274,17 +274,16 @@ static int build_id_cache__purge_all(void)
> static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
> {
> char filename[PATH_MAX];
> - u8 build_id[BUILD_ID_SIZE];
> + struct build_id bid;
>
> if (dso__build_id_filename(dso, filename, sizeof(filename), false) &&
> - filename__read_build_id(filename, build_id,
> - sizeof(build_id)) != sizeof(build_id)) {
> + filename__read_build_id(filename, &bid) == -1) {
> if (errno == ENOENT)
> return false;
>
> pr_warning("Problems with %s file, consider removing it from the cache\n",
> filename);
> - } else if (memcmp(dso->bid.data, build_id, sizeof(dso->bid.data))) {
> + } else if (memcmp(dso->bid.data, bid.data, bid.size)) {
> pr_warning("Problems with %s file, consider removing it from the cache\n",
> filename);
> }
> @@ -300,14 +299,14 @@ static int build_id_cache__fprintf_missing(struct perf_session *session, FILE *f
>
> static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
> {
> - u8 build_id[BUILD_ID_SIZE];
> char sbuild_id[SBUILD_ID_SIZE];
> + struct build_id bid;
> struct nscookie nsc;
>
> int err;
>
> nsinfo__mountns_enter(nsi, &nsc);
> - err = filename__read_build_id(filename, &build_id, sizeof(build_id));
> + err = filename__read_build_id(filename, &bid);
> nsinfo__mountns_exit(&nsc);
> if (err < 0) {
> pr_debug("Couldn't read a build-id in %s\n", filename);
> @@ -315,7 +314,7 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
> }
> err = 0;
>
> - build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> + build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> if (build_id_cache__cached(sbuild_id))
> err = build_id_cache__remove_s(sbuild_id);
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 5cf32d8e3aa6..a7a5ac5ea9d5 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -408,8 +408,7 @@ static int dso__read_build_id(struct dso *dso)
> if (dso->has_build_id)
> return 0;
>
> - if (filename__read_build_id(dso->long_name, dso->bid.data,
> - sizeof(dso->bid.data)) > 0) {
> + if (filename__read_build_id(dso->long_name, &dso->bid) > 0) {
> dso->has_build_id = true;
> return 0;
> }
> diff --git a/tools/perf/tests/pe-file-parsing.c b/tools/perf/tests/pe-file-parsing.c
> index 19eae3e8e229..58b90c42eb38 100644
> --- a/tools/perf/tests/pe-file-parsing.c
> +++ b/tools/perf/tests/pe-file-parsing.c
> @@ -24,7 +24,7 @@ static int run_dir(const char *d)
> {
> char filename[PATH_MAX];
> char debugfile[PATH_MAX];
> - char build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> char debuglink[PATH_MAX];
> char expect_build_id[] = {
> 0x5a, 0x0f, 0xd8, 0x82, 0xb5, 0x30, 0x84, 0x22,
> @@ -36,10 +36,10 @@ static int run_dir(const char *d)
> int ret;
>
> scnprintf(filename, PATH_MAX, "%s/pe-file.exe", d);
> - ret = filename__read_build_id(filename, build_id, BUILD_ID_SIZE);
> + ret = filename__read_build_id(filename, &bid);
> TEST_ASSERT_VAL("Failed to read build_id",
> ret == sizeof(expect_build_id));
> - TEST_ASSERT_VAL("Wrong build_id", !memcmp(build_id, expect_build_id,
> + TEST_ASSERT_VAL("Wrong build_id", !memcmp(bid.data, expect_build_id,
> sizeof(expect_build_id)));
>
> ret = filename__read_debuglink(filename, debuglink, PATH_MAX);
> @@ -48,10 +48,10 @@ static int run_dir(const char *d)
> !strcmp(debuglink, expect_debuglink));
>
> scnprintf(debugfile, PATH_MAX, "%s/%s", d, debuglink);
> - ret = filename__read_build_id(debugfile, build_id, BUILD_ID_SIZE);
> + ret = filename__read_build_id(debugfile, &bid);
> TEST_ASSERT_VAL("Failed to read debug file build_id",
> ret == sizeof(expect_build_id));
> - TEST_ASSERT_VAL("Wrong build_id", !memcmp(build_id, expect_build_id,
> + TEST_ASSERT_VAL("Wrong build_id", !memcmp(bid.data, expect_build_id,
> sizeof(expect_build_id)));
>
> dso = dso__new(filename);
> diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
> index 60f0e9ee04fb..3ef37f739203 100644
> --- a/tools/perf/tests/sdt.c
> +++ b/tools/perf/tests/sdt.c
> @@ -28,16 +28,16 @@ static int target_function(void)
> static int build_id_cache__add_file(const char *filename)
> {
> char sbuild_id[SBUILD_ID_SIZE];
> - u8 build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> int err;
>
> - err = filename__read_build_id(filename, &build_id, sizeof(build_id));
> + err = filename__read_build_id(filename, &bid);
> if (err < 0) {
> pr_debug("Failed to read build id of %s\n", filename);
> return err;
> }
>
> - build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> + build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> err = build_id_cache__add_s(sbuild_id, filename, NULL, false, false);
> if (err < 0)
> pr_debug("Failed to add build id cache of %s\n", filename);
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 7da13ddb0d50..62b258aaa128 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -130,16 +130,14 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
>
> int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
> {
> - u8 build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> int ret;
>
> - ret = filename__read_build_id(pathname, build_id, sizeof(build_id));
> + ret = filename__read_build_id(pathname, &bid);
> if (ret < 0)
> return ret;
> - else if (ret != sizeof(build_id))
> - return -EINVAL;
>
> - return build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> + return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> }
>
> /* asnprintf consolidates asprintf and snprintf */
> diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
> index e3af46e818f1..87161e431830 100644
> --- a/tools/perf/util/dsos.c
> +++ b/tools/perf/util/dsos.c
> @@ -73,8 +73,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits)
> continue;
> }
> nsinfo__mountns_enter(pos->nsinfo, &nsc);
> - if (filename__read_build_id(pos->long_name, pos->bid.data,
> - sizeof(pos->bid.data)) > 0) {
> + if (filename__read_build_id(pos->long_name, &pos->bid) > 0) {
> have_build_id = true;
> pos->has_build_id = true;
> }
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 94a156df22d5..61d7c444e6f5 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -534,8 +534,9 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
>
> #ifdef HAVE_LIBBFD_BUILDID_SUPPORT
>
> -int filename__read_build_id(const char *filename, void *bf, size_t size)
> +int filename__read_build_id(const char *filename, struct build_id *bid)
> {
> + size_t size = sizeof(bid->data);
> int err = -1;
> bfd *abfd;
>
> @@ -551,9 +552,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
> if (!abfd->build_id || abfd->build_id->size > size)
> goto out_close;
>
> - memcpy(bf, abfd->build_id->data, abfd->build_id->size);
> - memset(bf + abfd->build_id->size, 0, size - abfd->build_id->size);
> - err = abfd->build_id->size;
> + memcpy(bid->data, abfd->build_id->data, abfd->build_id->size);
> + memset(bid->data + abfd->build_id->size, 0, size - abfd->build_id->size);
> + err = bid->size = abfd->build_id->size;
>
> out_close:
> bfd_close(abfd);
> @@ -562,8 +563,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
>
> #else // HAVE_LIBBFD_BUILDID_SUPPORT
>
> -int filename__read_build_id(const char *filename, void *bf, size_t size)
> +int filename__read_build_id(const char *filename, struct build_id *bid)
> {
> + size_t size = sizeof(bid->data);
> int fd, err = -1;
> Elf *elf;
>
> @@ -580,7 +582,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
> goto out_close;
> }
>
> - err = elf_read_build_id(elf, bf, size);
> + err = elf_read_build_id(elf, bid->data, size);
> + if (err > 0)
> + bid->size = err;
>
> elf_end(elf);
> out_close:
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index d6e99af263ec..5173331ee6e4 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -31,9 +31,10 @@ static bool check_need_swap(int file_endian)
>
> #define NT_GNU_BUILD_ID 3
>
> -static int read_build_id(void *note_data, size_t note_len, void *bf,
> - size_t size, bool need_swap)
> +static int read_build_id(void *note_data, size_t note_len, struct build_id *bid,
> + bool need_swap)
> {
> + size_t size = sizeof(bid->data);
> struct {
> u32 n_namesz;
> u32 n_descsz;
> @@ -63,8 +64,9 @@ static int read_build_id(void *note_data, size_t note_len, void *bf,
> nhdr->n_namesz == sizeof("GNU")) {
> if (memcmp(name, "GNU", sizeof("GNU")) == 0) {
> size_t sz = min(size, descsz);
> - memcpy(bf, ptr, sz);
> - memset(bf + sz, 0, size - sz);
> + memcpy(bid->data, ptr, sz);
> + memset(bid->data + sz, 0, size - sz);
> + bid->size = sz;
> return 0;
> }
> }
> @@ -84,7 +86,7 @@ int filename__read_debuglink(const char *filename __maybe_unused,
> /*
> * Just try PT_NOTE header otherwise fails
> */
> -int filename__read_build_id(const char *filename, void *bf, size_t size)
> +int filename__read_build_id(const char *filename, struct build_id *bid)
> {
> FILE *fp;
> int ret = -1;
> @@ -156,9 +158,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
> if (fread(buf, buf_size, 1, fp) != 1)
> goto out_free;
>
> - ret = read_build_id(buf, buf_size, bf, size, need_swap);
> + ret = read_build_id(buf, buf_size, bid, need_swap);
> if (ret == 0)
> - ret = size;
> + ret = bid->size;
> break;
> }
> } else {
> @@ -207,9 +209,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
> if (fread(buf, buf_size, 1, fp) != 1)
> goto out_free;
>
> - ret = read_build_id(buf, buf_size, bf, size, need_swap);
> + ret = read_build_id(buf, buf_size, bid, need_swap);
> if (ret == 0)
> - ret = size;
> + ret = bid->size;
> break;
> }
> }
> @@ -220,8 +222,9 @@ int filename__read_build_id(const char *filename, void *bf, size_t size)
> return ret;
> }
>
> -int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
> +int sysfs__read_build_id(const char *filename, void *build_id, size_t size __maybe_unused)
> {
> + struct build_id bid;
> int fd;
> int ret = -1;
> struct stat stbuf;
> @@ -243,7 +246,9 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
> if (read(fd, buf, buf_size) != (ssize_t) buf_size)
> goto out_free;
>
> - ret = read_build_id(buf, buf_size, build_id, size, false);
> + ret = read_build_id(buf, buf_size, &bid, false);
> + if (ret > 0)
> + memcpy(build_id, bid.data, bid.size);
> out_free:
> free(buf);
> out:
> @@ -339,16 +344,15 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
> struct symsrc *runtime_ss __maybe_unused,
> int kmodule __maybe_unused)
> {
> - unsigned char build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> int ret;
>
> ret = fd__is_64_bit(ss->fd);
> if (ret >= 0)
> dso->is_64_bit = ret;
>
> - if (filename__read_build_id(ss->name, build_id, BUILD_ID_SIZE) > 0) {
> - dso__set_build_id(dso, build_id);
> - }
> + if (filename__read_build_id(ss->name, &bid) > 0)
> + dso__set_build_id(dso, bid.data);
> return 0;
> }
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index c772c8c70db5..4c43bb959fee 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1755,7 +1755,7 @@ int dso__load(struct dso *dso, struct map *map)
> struct symsrc *syms_ss = NULL, *runtime_ss = NULL;
> bool kmod;
> bool perfmap;
> - unsigned char build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> struct nscookie nsc;
> char newmapname[PATH_MAX];
> const char *map_path = dso->long_name;
> @@ -1817,8 +1817,8 @@ int dso__load(struct dso *dso, struct map *map)
> if (!dso->has_build_id &&
> is_regular_file(dso->long_name)) {
> __symbol__join_symfs(name, PATH_MAX, dso->long_name);
> - if (filename__read_build_id(name, build_id, BUILD_ID_SIZE) > 0)
> - dso__set_build_id(dso, build_id);
> + if (filename__read_build_id(name, &bid) > 0)
> + dso__set_build_id(dso, bid.data);
> }
>
> /*
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 11fe71f46d14..98908fa3f796 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -23,6 +23,7 @@ struct dso;
> struct map;
> struct maps;
> struct option;
> +struct build_id;
>
> /*
> * libelf 0.8.x and earlier do not support ELF_C_READ_MMAP;
> @@ -142,7 +143,7 @@ struct symbol *dso__next_symbol(struct symbol *sym);
>
> enum dso_type dso__type_fd(int fd);
>
> -int filename__read_build_id(const char *filename, void *bf, size_t size);
> +int filename__read_build_id(const char *filename, struct build_id *id);
> int sysfs__read_build_id(const char *filename, void *bf, size_t size);
> int modules__parse(const char *filename, void *arg,
> int (*process_module)(void *arg, const char *name,
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/9] perf tools: Pass build id object to sysfs__read_build_id
2020-09-30 17:15 ` [PATCH 4/9] perf tools: Pass build id object to sysfs__read_build_id Jiri Olsa
@ 2020-10-01 5:20 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 5:20 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Passing build id object to sysfs__read_build_id function,
> so it can populate the size of the build_id object.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/util/build-id.c | 6 +++---
> tools/perf/util/dso.c | 6 ++----
> tools/perf/util/symbol-elf.c | 11 +++++------
> tools/perf/util/symbol-minimal.c | 7 ++-----
> tools/perf/util/symbol.c | 7 +++----
> tools/perf/util/symbol.h | 2 +-
> 6 files changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 62b258aaa128..1c332e78bbc3 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -113,7 +113,7 @@ int build_id__sprintf(const u8 *build_id, int len, char *bf)
> int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
> {
> char notes[PATH_MAX];
> - u8 build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> int ret;
>
> if (!root_dir)
> @@ -121,11 +121,11 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
>
> scnprintf(notes, sizeof(notes), "%s/sys/kernel/notes", root_dir);
>
> - ret = sysfs__read_build_id(notes, build_id, sizeof(build_id));
> + ret = sysfs__read_build_id(notes, &bid);
> if (ret < 0)
> return ret;
>
> - return build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> + return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> }
>
> int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index d2c1ed08c879..e87fa9a71d9f 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1346,8 +1346,7 @@ void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
> if (machine__is_default_guest(machine))
> return;
> sprintf(path, "%s/sys/kernel/notes", machine->root_dir);
> - if (sysfs__read_build_id(path, dso->bid.data,
> - sizeof(dso->bid.data)) == 0)
> + if (sysfs__read_build_id(path, &dso->bid) == 0)
> dso->has_build_id = true;
> }
>
> @@ -1365,8 +1364,7 @@ int dso__kernel_module_get_build_id(struct dso *dso,
> "%s/sys/module/%.*s/notes/.note.gnu.build-id",
> root_dir, (int)strlen(name) - 1, name);
>
> - if (sysfs__read_build_id(filename, dso->bid.data,
> - sizeof(dso->bid.data)) == 0)
> + if (sysfs__read_build_id(filename, &dso->bid) == 0)
> dso->has_build_id = true;
>
> return 0;
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 61d7c444e6f5..97a55f162ea5 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -595,13 +595,11 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
>
> #endif // HAVE_LIBBFD_BUILDID_SUPPORT
>
> -int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
> +int sysfs__read_build_id(const char *filename, struct build_id *bid)
> {
> + size_t size = sizeof(bid->data);
> int fd, err = -1;
>
> - if (size < BUILD_ID_SIZE)
> - goto out;
> -
> fd = open(filename, O_RDONLY);
> if (fd < 0)
> goto out;
> @@ -622,8 +620,9 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
> break;
> if (memcmp(bf, "GNU", sizeof("GNU")) == 0) {
> size_t sz = min(descsz, size);
> - if (read(fd, build_id, sz) == (ssize_t)sz) {
> - memset(build_id + sz, 0, size - sz);
> + if (read(fd, bid->data, sz) == (ssize_t)sz) {
> + memset(bid->data + sz, 0, size - sz);
> + bid->size = sz;
> err = 0;
> break;
> }
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 5173331ee6e4..dba6b9e5d64e 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -222,9 +222,8 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
> return ret;
> }
>
> -int sysfs__read_build_id(const char *filename, void *build_id, size_t size __maybe_unused)
> +int sysfs__read_build_id(const char *filename, struct build_id *bid)
> {
> - struct build_id bid;
> int fd;
> int ret = -1;
> struct stat stbuf;
> @@ -246,9 +245,7 @@ int sysfs__read_build_id(const char *filename, void *build_id, size_t size __may
> if (read(fd, buf, buf_size) != (ssize_t) buf_size)
> goto out_free;
>
> - ret = read_build_id(buf, buf_size, &bid, false);
> - if (ret > 0)
> - memcpy(build_id, bid.data, bid.size);
> + ret = read_build_id(buf, buf_size, bid, false);
> out_free:
> free(buf);
> out:
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 4c43bb959fee..dd1cf91c62fb 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2122,7 +2122,7 @@ static bool filename__readable(const char *file)
>
> static char *dso__find_kallsyms(struct dso *dso, struct map *map)
> {
> - u8 host_build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> char sbuild_id[SBUILD_ID_SIZE];
> bool is_host = false;
> char path[PATH_MAX];
> @@ -2135,9 +2135,8 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
> goto proc_kallsyms;
> }
>
> - if (sysfs__read_build_id("/sys/kernel/notes", host_build_id,
> - sizeof(host_build_id)) == 0)
> - is_host = dso__build_id_equal(dso, host_build_id);
> + if (sysfs__read_build_id("/sys/kernel/notes", &bid) == 0)
> + is_host = dso__build_id_equal(dso, bid.data);
>
> /* Try a fast path for /proc/kallsyms if possible */
> if (is_host) {
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 98908fa3f796..70b3874e7dd8 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -144,7 +144,7 @@ struct symbol *dso__next_symbol(struct symbol *sym);
> enum dso_type dso__type_fd(int fd);
>
> int filename__read_build_id(const char *filename, struct build_id *id);
> -int sysfs__read_build_id(const char *filename, void *bf, size_t size);
> +int sysfs__read_build_id(const char *filename, struct build_id *bid);
> int modules__parse(const char *filename, void *arg,
> int (*process_module)(void *arg, const char *name,
> u64 start, u64 size));
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 5/9] perf tools: Pass build_id object to build_id__sprintf
2020-09-30 17:15 ` [PATCH 5/9] perf tools: Pass build_id object to build_id__sprintf Jiri Olsa
@ 2020-10-01 5:24 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 5:24 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Passing build_id object to build_id__sprintf function,
> so it can operate with the proper size of build id.
>
> This will create proper md5 build id readable names,
> like following:
> a50e350e97c43b4708d09bcd85ebfff7
>
> instead of:
> a50e350e97c43b4708d09bcd85ebfff700000000
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/builtin-buildid-cache.c | 6 ++--
> tools/perf/tests/sdt.c | 2 +-
> tools/perf/util/annotate.c | 3 +-
> tools/perf/util/build-id.c | 30 ++++++++++++-------
> tools/perf/util/build-id.h | 3 +-
> tools/perf/util/dso.c | 6 ++--
> tools/perf/util/header.c | 3 +-
> tools/perf/util/map.c | 4 +--
> tools/perf/util/probe-event.c | 9 ++++--
> tools/perf/util/probe-finder.c | 8 +++--
> .../scripting-engines/trace-event-python.c | 2 +-
> tools/perf/util/symbol.c | 2 +-
> 12 files changed, 43 insertions(+), 35 deletions(-)
>
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index c3cb168d546d..a25411926e48 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -186,7 +186,7 @@ static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
> return -1;
> }
>
> - build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> + build_id__sprintf(&bid, sbuild_id);
> err = build_id_cache__add_s(sbuild_id, filename, nsi,
> false, false);
> pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
> @@ -210,7 +210,7 @@ static int build_id_cache__remove_file(const char *filename, struct nsinfo *nsi)
> return -1;
> }
>
> - build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> + build_id__sprintf(&bid, sbuild_id);
> err = build_id_cache__remove_s(sbuild_id);
> pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
> err ? "FAIL" : "Ok");
> @@ -314,7 +314,7 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
> }
> err = 0;
>
> - build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> + build_id__sprintf(&bid, sbuild_id);
> if (build_id_cache__cached(sbuild_id))
> err = build_id_cache__remove_s(sbuild_id);
>
> diff --git a/tools/perf/tests/sdt.c b/tools/perf/tests/sdt.c
> index 3ef37f739203..ed76c693f65e 100644
> --- a/tools/perf/tests/sdt.c
> +++ b/tools/perf/tests/sdt.c
> @@ -37,7 +37,7 @@ static int build_id_cache__add_file(const char *filename)
> return err;
> }
>
> - build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> + build_id__sprintf(&bid, sbuild_id);
> err = build_id_cache__add_s(sbuild_id, filename, NULL, false, false);
> if (err < 0)
> pr_debug("Failed to add build id cache of %s\n", filename);
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index a016e1bd7b8d..6c8575e182ed 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -1578,8 +1578,7 @@ int symbol__strerror_disassemble(struct map_symbol *ms, int errnum, char *buf, s
> char *build_id_msg = NULL;
>
> if (dso->has_build_id) {
> - build_id__sprintf(dso->bid.data,
> - sizeof(dso->bid.data), bf + 15);
> + build_id__sprintf(&dso->bid, bf + 15);
> build_id_msg = bf;
> }
> scnprintf(buf, buflen,
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 1c332e78bbc3..b5648735f01f 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -37,6 +37,7 @@
>
> #include <linux/ctype.h>
> #include <linux/zalloc.h>
> +#include <asm/bug.h>
>
> static bool no_buildid_cache;
>
> @@ -95,13 +96,13 @@ struct perf_tool build_id__mark_dso_hit_ops = {
> .ordered_events = true,
> };
>
> -int build_id__sprintf(const u8 *build_id, int len, char *bf)
> +int build_id__sprintf(const struct build_id *build_id, char *bf)
> {
> char *bid = bf;
> - const u8 *raw = build_id;
> - int i;
> + const u8 *raw = build_id->data;
> + size_t i;
>
> - for (i = 0; i < len; ++i) {
> + for (i = 0; i < build_id->size; ++i) {
> sprintf(bid, "%02x", *raw);
> ++raw;
> bid += 2;
> @@ -125,7 +126,7 @@ int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
> if (ret < 0)
> return ret;
>
> - return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> + return build_id__sprintf(&bid, sbuild_id);
> }
>
> int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
> @@ -137,7 +138,7 @@ int filename__sprintf_build_id(const char *pathname, char *sbuild_id)
> if (ret < 0)
> return ret;
>
> - return build_id__sprintf(bid.data, sizeof(bid.data), sbuild_id);
> + return build_id__sprintf(&bid, sbuild_id);
> }
>
> /* asnprintf consolidates asprintf and snprintf */
> @@ -270,7 +271,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size,
> if (!dso->has_build_id)
> return NULL;
>
> - build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
> + build_id__sprintf(&dso->bid, sbuild_id);
> linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
> if (!linkname)
> return NULL;
> @@ -767,13 +768,13 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
> return err;
> }
>
> -static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
> +static int build_id_cache__add_b(const struct build_id *bid,
> const char *name, struct nsinfo *nsi,
> bool is_kallsyms, bool is_vdso)
> {
> char sbuild_id[SBUILD_ID_SIZE];
>
> - build_id__sprintf(build_id, build_id_size, sbuild_id);
> + build_id__sprintf(bid, sbuild_id);
>
> return build_id_cache__add_s(sbuild_id, name, nsi, is_kallsyms,
> is_vdso);
> @@ -839,8 +840,8 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)
> is_kallsyms = true;
> name = machine->mmap_name;
> }
> - return build_id_cache__add_b(dso->bid.data, sizeof(dso->bid.data), name,
> - dso->nsinfo, is_kallsyms, is_vdso);
> + return build_id_cache__add_b(&dso->bid, name, dso->nsinfo,
> + is_kallsyms, is_vdso);
> }
>
> static int __dsos__cache_build_ids(struct list_head *head,
> @@ -900,3 +901,10 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits)
>
> return ret;
> }
> +
> +void build_id__init(struct build_id *bid, const u8 *data, size_t size)
> +{
> + WARN_ON(size > BUILD_ID_SIZE);
> + memcpy(bid->data, data, size);
> + bid->size = size;
> +}
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index cc7fc62303ee..5e08e991defc 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -19,7 +19,8 @@ extern struct perf_tool build_id__mark_dso_hit_ops;
> struct dso;
> struct feat_fd;
>
> -int build_id__sprintf(const u8 *build_id, int len, char *bf);
> +void build_id__init(struct build_id *bid, const u8 *data, size_t size);
> +int build_id__sprintf(const struct build_id *build_id, char *bf);
Nit: perhaps bf should be on the left for consistency with regular sprintf.
> int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id);
> int filename__sprintf_build_id(const char *pathname, char *sbuild_id);
> char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf,
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index e87fa9a71d9f..2f7f01ead9a1 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -172,9 +172,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
> break;
> }
>
> - build_id__sprintf(dso->bid.data,
> - sizeof(dso->bid.data),
> - build_id_hex);
> + build_id__sprintf(&dso->bid, build_id_hex);
> len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
> snprintf(filename + len, size - len, "%.2s/%s.debug",
> build_id_hex, build_id_hex + 2);
> @@ -1374,7 +1372,7 @@ size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
> {
> char sbuild_id[SBUILD_ID_SIZE];
>
> - build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
> + build_id__sprintf(&dso->bid, sbuild_id);
> return fprintf(fp, "%s", sbuild_id);
> }
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index cde8f6eba479..fe220f01fc94 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2095,8 +2095,7 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
> free(m.name);
> }
>
> - build_id__sprintf(dso->bid.data, sizeof(dso->bid.data),
> - sbuild_id);
> + build_id__sprintf(&dso->bid, sbuild_id);
> pr_debug("build id event received for %s: %s\n",
> dso->long_name, sbuild_id);
> dso__put(dso);
> diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
> index d08540193822..596cc8e2c167 100644
> --- a/tools/perf/util/map.c
> +++ b/tools/perf/util/map.c
> @@ -346,9 +346,7 @@ int map__load(struct map *map)
> if (map->dso->has_build_id) {
> char sbuild_id[SBUILD_ID_SIZE];
>
> - build_id__sprintf(map->dso->bid.data,
> - sizeof(map->dso->bid.data),
> - sbuild_id);
> + build_id__sprintf(&map->dso->bid, sbuild_id);
> pr_debug("%s with build id %s not found", name, sbuild_id);
> } else
> pr_debug("Failed to open %s", name);
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 2041ad849851..8eae2afff71a 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -473,7 +473,7 @@ static struct debuginfo *open_from_debuginfod(struct dso *dso, struct nsinfo *ns
> if (!c)
> return NULL;
>
> - build_id__sprintf(dso->bid.data, BUILD_ID_SIZE, sbuild_id);
> + build_id__sprintf(&dso->bid, sbuild_id);
> fd = debuginfod_find_debuginfo(c, (const unsigned char *)sbuild_id,
> 0, &path);
> if (fd >= 0)
> @@ -1005,6 +1005,7 @@ static int _show_one_line(FILE *fp, int l, bool skip, bool show_num)
> static int __show_line_range(struct line_range *lr, const char *module,
> bool user)
> {
> + struct build_id bid;
> int l = 1;
> struct int_node *ln;
> struct debuginfo *dinfo;
> @@ -1025,8 +1026,10 @@ static int __show_line_range(struct line_range *lr, const char *module,
> if (!ret)
> ret = debuginfo__find_line_range(dinfo, lr);
> }
> - if (dinfo->build_id)
> - build_id__sprintf(dinfo->build_id, BUILD_ID_SIZE, sbuild_id);
> + if (dinfo->build_id) {
> + build_id__init(&bid, dinfo->build_id, BUILD_ID_SIZE);
> + build_id__sprintf(&bid, sbuild_id);
> + }
> debuginfo__delete(dinfo);
> if (ret == 0 || ret == -ENOENT) {
> pr_warning("Specified source line is not found.\n");
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 6eddf7be8293..2c4061035f77 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -949,6 +949,7 @@ static int probe_point_lazy_walker(const char *fname, int lineno,
> /* Find probe points from lazy pattern */
> static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
> {
> + struct build_id bid;
> char sbuild_id[SBUILD_ID_SIZE] = "";
> int ret = 0;
> char *fpath;
> @@ -957,9 +958,10 @@ static int find_probe_point_lazy(Dwarf_Die *sp_die, struct probe_finder *pf)
> const char *comp_dir;
>
> comp_dir = cu_get_comp_dir(&pf->cu_die);
> - if (pf->dbg->build_id)
> - build_id__sprintf(pf->dbg->build_id,
> - BUILD_ID_SIZE, sbuild_id);
> + if (pf->dbg->build_id) {
> + build_id__init(&bid, pf->dbg->build_id, BUILD_ID_SIZE);
> + build_id__sprintf(&bid, sbuild_id);
> + }
> ret = find_source_path(pf->fname, sbuild_id, comp_dir, &fpath);
> if (ret < 0) {
> pr_warning("Failed to find source file path.\n");
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index db3b1c275d8f..7cbd024e3e63 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -1064,7 +1064,7 @@ static int python_export_dso(struct db_export *dbe, struct dso *dso,
> char sbuild_id[SBUILD_ID_SIZE];
> PyObject *t;
>
> - build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
> + build_id__sprintf(&dso->bid, sbuild_id);
>
> t = tuple_new(5);
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index dd1cf91c62fb..369cbad09f0d 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2152,7 +2152,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
> goto proc_kallsyms;
> }
>
> - build_id__sprintf(dso->bid.data, sizeof(dso->bid.data), sbuild_id);
> + build_id__sprintf(&dso->bid, sbuild_id);
>
> /* Find kallsyms in build-id cache with kcore */
> scnprintf(path, sizeof(path), "%s/%s/%s",
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 6/9] perf tools: Pass build_id object to dso__set_build_id
2020-09-30 17:15 ` [PATCH 6/9] perf tools: Pass build_id object to dso__set_build_id Jiri Olsa
@ 2020-10-01 5:25 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 5:25 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Passing build_id object to dso__set_build_id, so it's easier
> to initialize dos's build id object.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/util/dso.c | 4 ++--
> tools/perf/util/dso.h | 2 +-
> tools/perf/util/header.c | 4 +++-
> tools/perf/util/symbol-minimal.c | 2 +-
> tools/perf/util/symbol.c | 2 +-
> 5 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 2f7f01ead9a1..4415ce83150b 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1326,9 +1326,9 @@ void dso__put(struct dso *dso)
> dso__delete(dso);
> }
>
> -void dso__set_build_id(struct dso *dso, void *build_id)
> +void dso__set_build_id(struct dso *dso, struct build_id *bid)
> {
> - memcpy(dso->bid.data, build_id, sizeof(dso->bid.data));
> + dso->bid = *bid;
> dso->has_build_id = 1;
> }
>
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index eac004210b47..5a5678dbdaa5 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -260,7 +260,7 @@ bool dso__sorted_by_name(const struct dso *dso);
> void dso__set_sorted_by_name(struct dso *dso);
> void dso__sort_by_name(struct dso *dso);
>
> -void dso__set_build_id(struct dso *dso, void *build_id);
> +void dso__set_build_id(struct dso *dso, struct build_id *bid);
> bool dso__build_id_equal(const struct dso *dso, u8 *build_id);
> void dso__read_running_kernel_build_id(struct dso *dso,
> struct machine *machine);
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index fe220f01fc94..21243adbb9fd 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2082,8 +2082,10 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
> dso = machine__findnew_dso(machine, filename);
> if (dso != NULL) {
> char sbuild_id[SBUILD_ID_SIZE];
> + struct build_id bid;
>
> - dso__set_build_id(dso, &bev->build_id);
> + build_id__init(&bid, bev->build_id, BUILD_ID_SIZE);
> + dso__set_build_id(dso, &bid);
>
> if (dso_space != DSO_SPACE__USER) {
> struct kmod_path m = { .name = NULL, };
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index dba6b9e5d64e..f9eb0bee7f15 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -349,7 +349,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
> dso->is_64_bit = ret;
>
> if (filename__read_build_id(ss->name, &bid) > 0)
> - dso__set_build_id(dso, bid.data);
> + dso__set_build_id(dso, &bid);
> return 0;
> }
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 369cbad09f0d..976632d0baa0 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1818,7 +1818,7 @@ int dso__load(struct dso *dso, struct map *map)
> is_regular_file(dso->long_name)) {
> __symbol__join_symfs(name, PATH_MAX, dso->long_name);
> if (filename__read_build_id(name, &bid) > 0)
> - dso__set_build_id(dso, bid.data);
> + dso__set_build_id(dso, &bid);
> }
>
> /*
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 7/9] perf tools: Pass build_id object to dso__build_id_equal
2020-09-30 17:15 ` [PATCH 7/9] perf tools: Pass build_id object to dso__build_id_equal Jiri Olsa
@ 2020-10-01 5:25 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 5:25 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Passing build_id object to dso__build_id_equal, so we can
> properly check build id with different size than sha1.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/util/dso.c | 5 +++--
> tools/perf/util/dso.h | 2 +-
> tools/perf/util/symbol-elf.c | 8 ++++++--
> tools/perf/util/symbol.c | 2 +-
> 4 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 4415ce83150b..ca965845b35e 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1332,9 +1332,10 @@ void dso__set_build_id(struct dso *dso, struct build_id *bid)
> dso->has_build_id = 1;
> }
>
> -bool dso__build_id_equal(const struct dso *dso, u8 *build_id)
> +bool dso__build_id_equal(const struct dso *dso, struct build_id *bid)
> {
> - return memcmp(dso->bid.data, build_id, sizeof(dso->bid.data)) == 0;
> + return dso->bid.size == bid->size &&
> + memcmp(dso->bid.data, bid->data, dso->bid.size) == 0;
> }
>
> void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index 5a5678dbdaa5..f926c96bf230 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -261,7 +261,7 @@ void dso__set_sorted_by_name(struct dso *dso);
> void dso__sort_by_name(struct dso *dso);
>
> void dso__set_build_id(struct dso *dso, struct build_id *bid);
> -bool dso__build_id_equal(const struct dso *dso, u8 *build_id);
> +bool dso__build_id_equal(const struct dso *dso, struct build_id *bid);
> void dso__read_running_kernel_build_id(struct dso *dso,
> struct machine *machine);
> int dso__kernel_module_get_build_id(struct dso *dso, const char *root_dir);
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 97a55f162ea5..44dd86a4f25f 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -834,13 +834,17 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
> /* Always reject images with a mismatched build-id: */
> if (dso->has_build_id && !symbol_conf.ignore_vmlinux_buildid) {
> u8 build_id[BUILD_ID_SIZE];
> + struct build_id bid;
> + int size;
>
> - if (elf_read_build_id(elf, build_id, BUILD_ID_SIZE) < 0) {
> + size = elf_read_build_id(elf, build_id, BUILD_ID_SIZE);
> + if (size <= 0) {
> dso->load_errno = DSO_LOAD_ERRNO__CANNOT_READ_BUILDID;
> goto out_elf_end;
> }
>
> - if (!dso__build_id_equal(dso, build_id)) {
> + build_id__init(&bid, build_id, size);
> + if (!dso__build_id_equal(dso, &bid)) {
> pr_debug("%s: build id mismatch for %s.\n", __func__, name);
> dso->load_errno = DSO_LOAD_ERRNO__MISMATCHING_BUILDID;
> goto out_elf_end;
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 976632d0baa0..6138866665df 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2136,7 +2136,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
> }
>
> if (sysfs__read_build_id("/sys/kernel/notes", &bid) == 0)
> - is_host = dso__build_id_equal(dso, bid.data);
> + is_host = dso__build_id_equal(dso, &bid);
>
> /* Try a fast path for /proc/kallsyms if possible */
> if (is_host) {
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 8/9] perf tools: Add size to struct perf_record_header_build_id
2020-09-30 17:15 ` [PATCH 8/9] perf tools: Add size to struct perf_record_header_build_id Jiri Olsa
@ 2020-10-01 5:31 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 5:31 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> We do not store size with build ids in perf data,
> but there's enough space to do it. Adding misc bit
> PERF_RECORD_MISC_BUILD_ID_SIZE to mark build id event
> with size.
>
> With this fix the dso with md5 build id will have correct
> build id data and will be usable for debuginfod processing
> if needed (coming in following patches).
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/lib/perf/include/perf/event.h | 12 +++++++++++-
> tools/perf/util/build-id.c | 8 +++++---
> tools/perf/util/header.c | 10 +++++++---
> 3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
> index a6dbba6b9073..988c539bedb6 100644
> --- a/tools/lib/perf/include/perf/event.h
> +++ b/tools/lib/perf/include/perf/event.h
> @@ -201,10 +201,20 @@ struct perf_record_header_tracing_data {
> __u32 size;
> };
>
> +#define PERF_RECORD_MISC_BUILD_ID_SIZE (1 << 15)
> +
> struct perf_record_header_build_id {
> struct perf_event_header header;
> pid_t pid;
> - __u8 build_id[24];
> + union {
> + __u8 build_id[24];
> + struct {
> + __u8 data[20];
> + __u8 size;
> + __u8 reserved1__;
> + __u16 reserved2__;
> + };
> + };
> char filename[];
> };
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index b5648735f01f..8763772f1095 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -296,7 +296,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size,
> continue; \
> else
>
> -static int write_buildid(const char *name, size_t name_len, u8 *build_id,
> +static int write_buildid(const char *name, size_t name_len, struct build_id *bid,
> pid_t pid, u16 misc, struct feat_fd *fd)
> {
> int err;
> @@ -307,7 +307,9 @@ static int write_buildid(const char *name, size_t name_len, u8 *build_id,
> len = PERF_ALIGN(len, NAME_ALIGN);
>
> memset(&b, 0, sizeof(b));
> - memcpy(&b.build_id, build_id, BUILD_ID_SIZE);
> + memcpy(&b.data, bid->data, bid->size);
> + b.size = (u8) bid->size;
> + misc |= PERF_RECORD_MISC_BUILD_ID_SIZE;
> b.pid = pid;
> b.header.misc = misc;
> b.header.size = sizeof(b) + len;
> @@ -354,7 +356,7 @@ static int machine__write_buildid_table(struct machine *machine,
> in_kernel = pos->kernel ||
> is_kernel_module(name,
> PERF_RECORD_MISC_CPUMODE_UNKNOWN);
> - err = write_buildid(name, name_len, pos->bid.data, machine->pid,
> + err = write_buildid(name, name_len, &pos->bid, machine->pid,
> in_kernel ? kmisc : umisc, fd);
> if (err)
> break;
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 21243adbb9fd..8da3886f10a8 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2083,8 +2083,12 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
> if (dso != NULL) {
> char sbuild_id[SBUILD_ID_SIZE];
> struct build_id bid;
> + size_t size = BUILD_ID_SIZE;
>
> - build_id__init(&bid, bev->build_id, BUILD_ID_SIZE);
> + if (bev->header.misc & PERF_RECORD_MISC_BUILD_ID_SIZE)
> + size = bev->size;
> +
> + build_id__init(&bid, bev->data, size);
> dso__set_build_id(dso, &bid);
>
> if (dso_space != DSO_SPACE__USER) {
> @@ -2098,8 +2102,8 @@ static int __event_process_build_id(struct perf_record_header_build_id *bev,
> }
>
> build_id__sprintf(&dso->bid, sbuild_id);
> - pr_debug("build id event received for %s: %s\n",
> - dso->long_name, sbuild_id);
> + pr_debug("build id event received for %s: %s [%lu]\n",
> + dso->long_name, sbuild_id, size);
> dso__put(dso);
> }
>
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 9/9] perf tools: Align buildid list output for short build ids
2020-09-30 17:15 ` [PATCH 9/9] perf tools: Align buildid list output for short build ids Jiri Olsa
@ 2020-10-01 5:35 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 5:35 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan,
Stephane Eranian
On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> With shorter md5 build ids we need to align their
> paths properly with other build ids:
>
> $ perf buildid-list
> 17f4e448cc746582ea1881528deb549f7fdb3fd5 [kernel.kallsyms]
> a50e350e97c43b4708d09bcd85ebfff7 .../tools/perf/buildid-ex-md5
> 1805c738c8f3ec0f47b7ea09080c28f34d18a82b /usr/lib64/ld-2.31.so
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> ---
> tools/perf/util/dso.c | 2 +-
> tools/perf/util/dso.h | 1 -
> tools/perf/util/dsos.c | 6 ++++--
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index ca965845b35e..55c11e854fe4 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1369,7 +1369,7 @@ int dso__kernel_module_get_build_id(struct dso *dso,
> return 0;
> }
>
> -size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
> +static size_t dso__fprintf_buildid(struct dso *dso, FILE *fp)
> {
> char sbuild_id[SBUILD_ID_SIZE];
>
> diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
> index f926c96bf230..d8cb4f5680a4 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -362,7 +362,6 @@ struct dso *machine__findnew_kernel(struct machine *machine, const char *name,
>
> void dso__reset_find_symbol_cache(struct dso *dso);
>
> -size_t dso__fprintf_buildid(struct dso *dso, FILE *fp);
> size_t dso__fprintf_symbols_by_name(struct dso *dso, FILE *fp);
> size_t dso__fprintf(struct dso *dso, FILE *fp);
>
> diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
> index 87161e431830..183a81d5b2f9 100644
> --- a/tools/perf/util/dsos.c
> +++ b/tools/perf/util/dsos.c
> @@ -287,10 +287,12 @@ size_t __dsos__fprintf_buildid(struct list_head *head, FILE *fp,
> size_t ret = 0;
>
> list_for_each_entry(pos, head, node) {
> + char sbuild_id[SBUILD_ID_SIZE];
> +
> if (skip && skip(pos, parm))
> continue;
> - ret += dso__fprintf_buildid(pos, fp);
> - ret += fprintf(fp, " %s\n", pos->long_name);
> + build_id__sprintf(&pos->bid, sbuild_id);
> + ret += fprintf(fp, "%-40s %s\n", sbuild_id, pos->long_name);
> }
> return ret;
> }
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] perf tools: Add build id shell test
2020-10-01 2:00 ` Ian Rogers
@ 2020-10-01 9:25 ` Jiri Olsa
2020-10-01 10:25 ` Jiri Olsa
0 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-10-01 9:25 UTC (permalink / raw)
To: Ian Rogers
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Wed, Sep 30, 2020 at 07:00:05PM -0700, Ian Rogers wrote:
> On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding test for build id cache that adds binary
> > with sha1 and md5 build ids and verifies it's
> > added properly.
> >
> > The test updates build id cache with perf record
> > and perf buildid-cache -a.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
>
> Acked-by: Ian Rogers <irogers@google.com>
>
> This is great! If I build perf and test from the build directory the
> test gets run. If I build using O=/tmp/perf and run from that
> directory then ./tests/shell isn't found and the test doesn't run.
> Similarly the install directory doesn't contain the executables and so
> the test is skipped. Is there any way to get the test running in these
> other scenarios?
ok, if there's already some way to get the build path I did not see that
I'll check and add something if it's missing
thanks,
jirka
>
> Thanks,
> Ian
>
> > ---
> > tools/perf/Makefile.perf | 14 +++++
> > tools/perf/tests/shell/buildid.sh | 90 +++++++++++++++++++++++++++++++
> > 2 files changed, 104 insertions(+)
> > create mode 100755 tools/perf/tests/shell/buildid.sh
> >
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index 920d8afb9238..b2aeefa64e92 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
> > #
> > # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
> > #
> > +# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
> > +#
> >
> > # As per kernel Makefile, avoid funny character set dependencies
> > unexport LC_ALL
> > @@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
> > PROGRAMS += $(OUTPUT)perf-read-vdsox32
> > endif
> >
> > +ifndef NO_BUILDID_EX
> > +PROGRAMS += $(OUTPUT)buildid-ex-sha1
> > +PROGRAMS += $(OUTPUT)buildid-ex-md5
> > +endif
> > +
> > LIBJVMTI = libperf-jvmti.so
> >
> > ifndef NO_JVMTI
> > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> > endif
> >
> > +ifndef NO_BUILDID_EX
> > +$(OUTPUT)buildid-ex-sha1:
> > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> > +$(OUTPUT)buildid-ex-md5:
> > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> > +endif
> > +
> > ifndef NO_JVMTI
> > LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
> >
> > diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> > new file mode 100755
> > index 000000000000..57fcd28bc4bd
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/buildid.sh
> > @@ -0,0 +1,90 @@
> > +#!/bin/sh
> > +# build id cache operations
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# skip if there are no test binaries
> > +if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
> > + echo "failed: no test binaries"
> > + exit 2
> > +fi
> > +
> > +# skip if there's no readelf
> > +if [ ! -x `which readelf` ]; then
> > + echo "failed: no readelf, install binutils"
> > + exit 2
> > +fi
> > +
> > +check()
> > +{
> > + id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> > +
> > + echo "build id: ${id}"
> > +
> > + link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> > + echo "link: ${link}"
> > +
> > + if [ ! -h $link ]; then
> > + echo "failed: link ${link} does not exist"
> > + exit 1
> > + fi
> > +
> > + file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
> > + echo "file: ${file}"
> > +
> > + if [ ! -x $file ]; then
> > + echo "failed: file ${file} does not exist"
> > + exit 1
> > + fi
> > +
> > + diff ${file} ${1}
> > + if [ $? -ne 0 ]; then
> > + echo "failed: ${file} do not match"
> > + exit 1
> > + fi
> > +
> > + echo "OK for ${1}"
> > +}
> > +
> > +test_add()
> > +{
> > + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > + perf="perf --buildid-dir ${build_id_dir}"
> > +
> > + ${perf} buildid-cache -v -a ${1}
> > + if [ $? -ne 0 ]; then
> > + echo "failed: add ${1} to build id cache"
> > + exit 1
> > + fi
> > +
> > + check ${1}
> > +
> > + rm -rf ${build_id_dir}
> > +}
> > +
> > +test_record()
> > +{
> > + data=$(mktemp /tmp/perf.data.XXX)
> > + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > + perf="perf --buildid-dir ${build_id_dir}"
> > +
> > + ${perf} record --buildid-all -o ${data} ${1}
> > + if [ $? -ne 0 ]; then
> > + echo "failed: record ${1}"
> > + exit 1
> > + fi
> > +
> > + check ${1}
> > +
> > + rm -rf ${build_id_dir}
> > + rm -rf ${data}
> > +}
> > +
> > +# add binaries manual via perf buildid-cache -a
> > +test_add buildid-ex-sha1
> > +test_add buildid-ex-md5
> > +
> > +# add binaries via perf record post processing
> > +test_record buildid-ex-sha1
> > +test_record buildid-ex-md5
> > +
> > +exit ${err}
> > --
> > 2.26.2
> >
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] perf tools: Add build id shell test
2020-10-01 9:25 ` Jiri Olsa
@ 2020-10-01 10:25 ` Jiri Olsa
2020-10-01 16:58 ` Ian Rogers
0 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-10-01 10:25 UTC (permalink / raw)
To: Ian Rogers
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Thu, Oct 01, 2020 at 11:25:34AM +0200, Jiri Olsa wrote:
> On Wed, Sep 30, 2020 at 07:00:05PM -0700, Ian Rogers wrote:
> > On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding test for build id cache that adds binary
> > > with sha1 and md5 build ids and verifies it's
> > > added properly.
> > >
> > > The test updates build id cache with perf record
> > > and perf buildid-cache -a.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> >
> > Acked-by: Ian Rogers <irogers@google.com>
> >
> > This is great! If I build perf and test from the build directory the
> > test gets run. If I build using O=/tmp/perf and run from that
> > directory then ./tests/shell isn't found and the test doesn't run.
> > Similarly the install directory doesn't contain the executables and so
> > the test is skipped. Is there any way to get the test running in these
> > other scenarios?
>
> ok, if there's already some way to get the build path I did not see that
> I'll check and add something if it's missing
would the patch below work for you?
thanks,
jirka
---
diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
index 57fcd28bc4bd..dd9f9c306c34 100755
--- a/tools/perf/tests/shell/buildid.sh
+++ b/tools/perf/tests/shell/buildid.sh
@@ -2,12 +2,23 @@
# build id cache operations
# SPDX-License-Identifier: GPL-2.0
+ex_md5=buildid-ex-md5
+ex_sha1=buildid-ex-sha1
+
# skip if there are no test binaries
if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
- echo "failed: no test binaries"
- exit 2
+ ex_dir=$(dirname `which perf`)
+ ex_md5=${ex_dir}/buildid-ex-md5
+ ex_sha1=${ex_dir}/buildid-ex-sha1
+
+ if [ ! -x ${ex_sha1} -a ! -x ${ex_md5} ]; then
+ echo "failed: no test binaries"
+ exit 2
+ fi
fi
+echo "test binaries: ${ex_sha1} ${ex_md5}"
+
# skip if there's no readelf
if [ ! -x `which readelf` ]; then
echo "failed: no readelf, install binutils"
@@ -80,11 +91,11 @@ test_record()
}
# add binaries manual via perf buildid-cache -a
-test_add buildid-ex-sha1
-test_add buildid-ex-md5
+test_add ${ex_sha1}
+test_add ${ex_md5}
# add binaries via perf record post processing
-test_record buildid-ex-sha1
-test_record buildid-ex-md5
+test_record ${ex_sha1}
+test_record ${ex_md5}
exit ${err}
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] perf tools: Add build id shell test
2020-10-01 10:25 ` Jiri Olsa
@ 2020-10-01 16:58 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-01 16:58 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Thu, Oct 1, 2020 at 3:25 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Oct 01, 2020 at 11:25:34AM +0200, Jiri Olsa wrote:
> > On Wed, Sep 30, 2020 at 07:00:05PM -0700, Ian Rogers wrote:
> > > On Wed, Sep 30, 2020 at 10:15 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > Adding test for build id cache that adds binary
> > > > with sha1 and md5 build ids and verifies it's
> > > > added properly.
> > > >
> > > > The test updates build id cache with perf record
> > > > and perf buildid-cache -a.
> > > >
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > >
> > > Acked-by: Ian Rogers <irogers@google.com>
> > >
> > > This is great! If I build perf and test from the build directory the
> > > test gets run. If I build using O=/tmp/perf and run from that
> > > directory then ./tests/shell isn't found and the test doesn't run.
> > > Similarly the install directory doesn't contain the executables and so
> > > the test is skipped. Is there any way to get the test running in these
> > > other scenarios?
> >
> > ok, if there's already some way to get the build path I did not see that
> > I'll check and add something if it's missing
>
> would the patch below work for you?
Thanks! I'm ok with the patch as is, this addition also looks good. I
think the ideal we should aim for is perf test passing from the
installed location. I appreciate this change is setting up groundwork
that other tests could build upon. I'd particularly like a shell test
to run the libperf tests, it should be little more than just running
the executables. The build/install set up and the paths are the
difficult part.
Thanks,
Ian
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> index 57fcd28bc4bd..dd9f9c306c34 100755
> --- a/tools/perf/tests/shell/buildid.sh
> +++ b/tools/perf/tests/shell/buildid.sh
> @@ -2,12 +2,23 @@
> # build id cache operations
> # SPDX-License-Identifier: GPL-2.0
>
> +ex_md5=buildid-ex-md5
> +ex_sha1=buildid-ex-sha1
> +
> # skip if there are no test binaries
> if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
> - echo "failed: no test binaries"
> - exit 2
> + ex_dir=$(dirname `which perf`)
> + ex_md5=${ex_dir}/buildid-ex-md5
> + ex_sha1=${ex_dir}/buildid-ex-sha1
> +
> + if [ ! -x ${ex_sha1} -a ! -x ${ex_md5} ]; then
> + echo "failed: no test binaries"
> + exit 2
> + fi
> fi
>
> +echo "test binaries: ${ex_sha1} ${ex_md5}"
> +
> # skip if there's no readelf
> if [ ! -x `which readelf` ]; then
> echo "failed: no readelf, install binutils"
> @@ -80,11 +91,11 @@ test_record()
> }
>
> # add binaries manual via perf buildid-cache -a
> -test_add buildid-ex-sha1
> -test_add buildid-ex-md5
> +test_add ${ex_sha1}
> +test_add ${ex_md5}
>
> # add binaries via perf record post processing
> -test_record buildid-ex-sha1
> -test_record buildid-ex-md5
> +test_record ${ex_sha1}
> +test_record ${ex_md5}
>
> exit ${err}
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCHv2 1/9] perf tools: Add build id shell test
2020-09-30 17:15 ` [PATCH 1/9] perf tools: Add build id shell test Jiri Olsa
2020-10-01 2:00 ` Ian Rogers
@ 2020-10-01 19:05 ` Jiri Olsa
2020-10-02 13:07 ` Namhyung Kim
2020-10-13 16:13 ` [PATCH " Arnaldo Carvalho de Melo
2 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-10-01 19:05 UTC (permalink / raw)
To: Jiri Olsa, Arnaldo Carvalho de Melo
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Adding test for build id cache that adds binary
with sha1 and md5 build ids and verifies it's
added properly.
The test updates build id cache with perf record
and perf buildid-cache -a.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
v2 changes:
- detect perf build directory when checking for build-ex* binaries
tools/perf/Makefile.perf | 14 +++++
tools/perf/tests/shell/buildid.sh | 101 ++++++++++++++++++++++++++++++
2 files changed, 115 insertions(+)
create mode 100755 tools/perf/tests/shell/buildid.sh
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 920d8afb9238..b2aeefa64e92 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -126,6 +126,8 @@ include ../scripts/utilities.mak
#
# Define NO_LIBDEBUGINFOD if you do not want support debuginfod
#
+# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
+#
# As per kernel Makefile, avoid funny character set dependencies
unexport LC_ALL
@@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
PROGRAMS += $(OUTPUT)perf-read-vdsox32
endif
+ifndef NO_BUILDID_EX
+PROGRAMS += $(OUTPUT)buildid-ex-sha1
+PROGRAMS += $(OUTPUT)buildid-ex-md5
+endif
+
LIBJVMTI = libperf-jvmti.so
ifndef NO_JVMTI
@@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
$(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
endif
+ifndef NO_BUILDID_EX
+$(OUTPUT)buildid-ex-sha1:
+ $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
+$(OUTPUT)buildid-ex-md5:
+ $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
+endif
+
ifndef NO_JVMTI
LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
new file mode 100755
index 000000000000..dd9f9c306c34
--- /dev/null
+++ b/tools/perf/tests/shell/buildid.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+# build id cache operations
+# SPDX-License-Identifier: GPL-2.0
+
+ex_md5=buildid-ex-md5
+ex_sha1=buildid-ex-sha1
+
+# skip if there are no test binaries
+if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
+ ex_dir=$(dirname `which perf`)
+ ex_md5=${ex_dir}/buildid-ex-md5
+ ex_sha1=${ex_dir}/buildid-ex-sha1
+
+ if [ ! -x ${ex_sha1} -a ! -x ${ex_md5} ]; then
+ echo "failed: no test binaries"
+ exit 2
+ fi
+fi
+
+echo "test binaries: ${ex_sha1} ${ex_md5}"
+
+# skip if there's no readelf
+if [ ! -x `which readelf` ]; then
+ echo "failed: no readelf, install binutils"
+ exit 2
+fi
+
+check()
+{
+ id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
+
+ echo "build id: ${id}"
+
+ link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
+ echo "link: ${link}"
+
+ if [ ! -h $link ]; then
+ echo "failed: link ${link} does not exist"
+ exit 1
+ fi
+
+ file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
+ echo "file: ${file}"
+
+ if [ ! -x $file ]; then
+ echo "failed: file ${file} does not exist"
+ exit 1
+ fi
+
+ diff ${file} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: ${file} do not match"
+ exit 1
+ fi
+
+ echo "OK for ${1}"
+}
+
+test_add()
+{
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} buildid-cache -v -a ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: add ${1} to build id cache"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+}
+
+test_record()
+{
+ data=$(mktemp /tmp/perf.data.XXX)
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} record --buildid-all -o ${data} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: record ${1}"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+ rm -rf ${data}
+}
+
+# add binaries manual via perf buildid-cache -a
+test_add ${ex_sha1}
+test_add ${ex_md5}
+
+# add binaries via perf record post processing
+test_record ${ex_sha1}
+test_record ${ex_md5}
+
+exit ${err}
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCHv2 1/9] perf tools: Add build id shell test
2020-10-01 19:05 ` [PATCHv2 " Jiri Olsa
@ 2020-10-02 13:07 ` Namhyung Kim
2020-10-02 17:34 ` Ian Rogers
0 siblings, 1 reply; 33+ messages in thread
From: Namhyung Kim @ 2020-10-02 13:07 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
Ian Rogers, Stephane Eranian
Hi Jiri,
On Fri, Oct 2, 2020 at 4:05 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> Adding test for build id cache that adds binary
> with sha1 and md5 build ids and verifies it's
> added properly.
>
> The test updates build id cache with perf record
> and perf buildid-cache -a.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> v2 changes:
> - detect perf build directory when checking for build-ex* binaries
>
> tools/perf/Makefile.perf | 14 +++++
> tools/perf/tests/shell/buildid.sh | 101 ++++++++++++++++++++++++++++++
> 2 files changed, 115 insertions(+)
> create mode 100755 tools/perf/tests/shell/buildid.sh
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 920d8afb9238..b2aeefa64e92 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
> #
> # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
> #
> +# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
> +#
>
> # As per kernel Makefile, avoid funny character set dependencies
> unexport LC_ALL
> @@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
> PROGRAMS += $(OUTPUT)perf-read-vdsox32
> endif
>
> +ifndef NO_BUILDID_EX
> +PROGRAMS += $(OUTPUT)buildid-ex-sha1
> +PROGRAMS += $(OUTPUT)buildid-ex-md5
> +endif
> +
> LIBJVMTI = libperf-jvmti.so
>
> ifndef NO_JVMTI
> @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> endif
>
> +ifndef NO_BUILDID_EX
> +$(OUTPUT)buildid-ex-sha1:
> + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> +$(OUTPUT)buildid-ex-md5:
> + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> +endif
Can we just build them in the test shell script instead?
Thanks
Namhyung
> +
> ifndef NO_JVMTI
> LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
>
> diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> new file mode 100755
> index 000000000000..dd9f9c306c34
> --- /dev/null
> +++ b/tools/perf/tests/shell/buildid.sh
> @@ -0,0 +1,101 @@
> +#!/bin/sh
> +# build id cache operations
> +# SPDX-License-Identifier: GPL-2.0
> +
> +ex_md5=buildid-ex-md5
> +ex_sha1=buildid-ex-sha1
> +
> +# skip if there are no test binaries
> +if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
> + ex_dir=$(dirname `which perf`)
> + ex_md5=${ex_dir}/buildid-ex-md5
> + ex_sha1=${ex_dir}/buildid-ex-sha1
> +
> + if [ ! -x ${ex_sha1} -a ! -x ${ex_md5} ]; then
> + echo "failed: no test binaries"
> + exit 2
> + fi
> +fi
> +
> +echo "test binaries: ${ex_sha1} ${ex_md5}"
> +
> +# skip if there's no readelf
> +if [ ! -x `which readelf` ]; then
> + echo "failed: no readelf, install binutils"
> + exit 2
> +fi
> +
> +check()
> +{
> + id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> +
> + echo "build id: ${id}"
> +
> + link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> + echo "link: ${link}"
> +
> + if [ ! -h $link ]; then
> + echo "failed: link ${link} does not exist"
> + exit 1
> + fi
> +
> + file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
> + echo "file: ${file}"
> +
> + if [ ! -x $file ]; then
> + echo "failed: file ${file} does not exist"
> + exit 1
> + fi
> +
> + diff ${file} ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: ${file} do not match"
> + exit 1
> + fi
> +
> + echo "OK for ${1}"
> +}
> +
> +test_add()
> +{
> + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> + perf="perf --buildid-dir ${build_id_dir}"
> +
> + ${perf} buildid-cache -v -a ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: add ${1} to build id cache"
> + exit 1
> + fi
> +
> + check ${1}
> +
> + rm -rf ${build_id_dir}
> +}
> +
> +test_record()
> +{
> + data=$(mktemp /tmp/perf.data.XXX)
> + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> + perf="perf --buildid-dir ${build_id_dir}"
> +
> + ${perf} record --buildid-all -o ${data} ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: record ${1}"
> + exit 1
> + fi
> +
> + check ${1}
> +
> + rm -rf ${build_id_dir}
> + rm -rf ${data}
> +}
> +
> +# add binaries manual via perf buildid-cache -a
> +test_add ${ex_sha1}
> +test_add ${ex_md5}
> +
> +# add binaries via perf record post processing
> +test_record ${ex_sha1}
> +test_record ${ex_md5}
> +
> +exit ${err}
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv2 1/9] perf tools: Add build id shell test
2020-10-02 13:07 ` Namhyung Kim
@ 2020-10-02 17:34 ` Ian Rogers
2020-10-02 19:29 ` Jiri Olsa
2020-10-02 20:55 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-02 17:34 UTC (permalink / raw)
To: Namhyung Kim
Cc: Jiri Olsa, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Fri, Oct 2, 2020 at 6:07 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Jiri,
>
> On Fri, Oct 2, 2020 at 4:05 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > Adding test for build id cache that adds binary
> > with sha1 and md5 build ids and verifies it's
> > added properly.
> >
> > The test updates build id cache with perf record
> > and perf buildid-cache -a.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > v2 changes:
> > - detect perf build directory when checking for build-ex* binaries
> >
> > tools/perf/Makefile.perf | 14 +++++
> > tools/perf/tests/shell/buildid.sh | 101 ++++++++++++++++++++++++++++++
> > 2 files changed, 115 insertions(+)
> > create mode 100755 tools/perf/tests/shell/buildid.sh
> >
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index 920d8afb9238..b2aeefa64e92 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
> > #
> > # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
> > #
> > +# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
> > +#
> >
> > # As per kernel Makefile, avoid funny character set dependencies
> > unexport LC_ALL
> > @@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
> > PROGRAMS += $(OUTPUT)perf-read-vdsox32
> > endif
> >
> > +ifndef NO_BUILDID_EX
> > +PROGRAMS += $(OUTPUT)buildid-ex-sha1
> > +PROGRAMS += $(OUTPUT)buildid-ex-md5
> > +endif
> > +
> > LIBJVMTI = libperf-jvmti.so
> >
> > ifndef NO_JVMTI
> > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> > endif
> >
> > +ifndef NO_BUILDID_EX
> > +$(OUTPUT)buildid-ex-sha1:
> > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> > +$(OUTPUT)buildid-ex-md5:
> > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> > +endif
>
> Can we just build them in the test shell script instead?
>
> Thanks
> Namhyung
That'd mean perf test having a dependency on a compiler :-/ That said
there are some existing dependencies for BPF compilers.
Thanks,
Ian
>
> > +
> > ifndef NO_JVMTI
> > LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
> >
> > diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> > new file mode 100755
> > index 000000000000..dd9f9c306c34
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/buildid.sh
> > @@ -0,0 +1,101 @@
> > +#!/bin/sh
> > +# build id cache operations
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +ex_md5=buildid-ex-md5
> > +ex_sha1=buildid-ex-sha1
> > +
> > +# skip if there are no test binaries
> > +if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
> > + ex_dir=$(dirname `which perf`)
> > + ex_md5=${ex_dir}/buildid-ex-md5
> > + ex_sha1=${ex_dir}/buildid-ex-sha1
> > +
> > + if [ ! -x ${ex_sha1} -a ! -x ${ex_md5} ]; then
> > + echo "failed: no test binaries"
> > + exit 2
> > + fi
> > +fi
> > +
> > +echo "test binaries: ${ex_sha1} ${ex_md5}"
> > +
> > +# skip if there's no readelf
> > +if [ ! -x `which readelf` ]; then
> > + echo "failed: no readelf, install binutils"
> > + exit 2
> > +fi
> > +
> > +check()
> > +{
> > + id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> > +
> > + echo "build id: ${id}"
> > +
> > + link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> > + echo "link: ${link}"
> > +
> > + if [ ! -h $link ]; then
> > + echo "failed: link ${link} does not exist"
> > + exit 1
> > + fi
> > +
> > + file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
> > + echo "file: ${file}"
> > +
> > + if [ ! -x $file ]; then
> > + echo "failed: file ${file} does not exist"
> > + exit 1
> > + fi
> > +
> > + diff ${file} ${1}
> > + if [ $? -ne 0 ]; then
> > + echo "failed: ${file} do not match"
> > + exit 1
> > + fi
> > +
> > + echo "OK for ${1}"
> > +}
> > +
> > +test_add()
> > +{
> > + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > + perf="perf --buildid-dir ${build_id_dir}"
> > +
> > + ${perf} buildid-cache -v -a ${1}
> > + if [ $? -ne 0 ]; then
> > + echo "failed: add ${1} to build id cache"
> > + exit 1
> > + fi
> > +
> > + check ${1}
> > +
> > + rm -rf ${build_id_dir}
> > +}
> > +
> > +test_record()
> > +{
> > + data=$(mktemp /tmp/perf.data.XXX)
> > + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > + perf="perf --buildid-dir ${build_id_dir}"
> > +
> > + ${perf} record --buildid-all -o ${data} ${1}
> > + if [ $? -ne 0 ]; then
> > + echo "failed: record ${1}"
> > + exit 1
> > + fi
> > +
> > + check ${1}
> > +
> > + rm -rf ${build_id_dir}
> > + rm -rf ${data}
> > +}
> > +
> > +# add binaries manual via perf buildid-cache -a
> > +test_add ${ex_sha1}
> > +test_add ${ex_md5}
> > +
> > +# add binaries via perf record post processing
> > +test_record ${ex_sha1}
> > +test_record ${ex_md5}
> > +
> > +exit ${err}
> > --
> > 2.26.2
> >
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv2 1/9] perf tools: Add build id shell test
2020-10-02 17:34 ` Ian Rogers
@ 2020-10-02 19:29 ` Jiri Olsa
2020-10-06 1:37 ` Namhyung Kim
2020-10-02 20:55 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-10-02 19:29 UTC (permalink / raw)
To: Ian Rogers
Cc: Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Fri, Oct 02, 2020 at 10:34:51AM -0700, Ian Rogers wrote:
SNIP
> > > +
> > > LIBJVMTI = libperf-jvmti.so
> > >
> > > ifndef NO_JVMTI
> > > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> > > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> > > endif
> > >
> > > +ifndef NO_BUILDID_EX
> > > +$(OUTPUT)buildid-ex-sha1:
> > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> > > +$(OUTPUT)buildid-ex-md5:
> > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> > > +endif
> >
> > Can we just build them in the test shell script instead?
it would solve the build-directory/install-directory
lookup search.. but it'd need to do detect compiler
and depend on it as Ian said
do you have some other reason to compile it in test?
thanks,
jirka
> >
> > Thanks
> > Namhyung
>
> That'd mean perf test having a dependency on a compiler :-/ That said
> there are some existing dependencies for BPF compilers.
>
> Thanks,
> Ian
SNIP
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv2 1/9] perf tools: Add build id shell test
2020-10-02 17:34 ` Ian Rogers
2020-10-02 19:29 ` Jiri Olsa
@ 2020-10-02 20:55 ` Arnaldo Carvalho de Melo
2020-10-05 15:58 ` [PATCHv3 " Jiri Olsa
1 sibling, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-02 20:55 UTC (permalink / raw)
To: Ian Rogers
Cc: Namhyung Kim, Jiri Olsa, Jiri Olsa, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
Stephane Eranian
Em Fri, Oct 02, 2020 at 10:34:51AM -0700, Ian Rogers escreveu:
> On Fri, Oct 2, 2020 at 6:07 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hi Jiri,
> >
> > On Fri, Oct 2, 2020 at 4:05 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > Adding test for build id cache that adds binary
> > > with sha1 and md5 build ids and verifies it's
> > > added properly.
> > >
> > > The test updates build id cache with perf record
> > > and perf buildid-cache -a.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > v2 changes:
> > > - detect perf build directory when checking for build-ex* binaries
> > >
> > > tools/perf/Makefile.perf | 14 +++++
> > > tools/perf/tests/shell/buildid.sh | 101 ++++++++++++++++++++++++++++++
> > > 2 files changed, 115 insertions(+)
> > > create mode 100755 tools/perf/tests/shell/buildid.sh
> > >
> > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > index 920d8afb9238..b2aeefa64e92 100644
> > > --- a/tools/perf/Makefile.perf
> > > +++ b/tools/perf/Makefile.perf
> > > @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
> > > #
> > > # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
> > > #
> > > +# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
> > > +#
> > >
> > > # As per kernel Makefile, avoid funny character set dependencies
> > > unexport LC_ALL
> > > @@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
> > > PROGRAMS += $(OUTPUT)perf-read-vdsox32
> > > endif
> > >
> > > +ifndef NO_BUILDID_EX
> > > +PROGRAMS += $(OUTPUT)buildid-ex-sha1
> > > +PROGRAMS += $(OUTPUT)buildid-ex-md5
> > > +endif
> > > +
> > > LIBJVMTI = libperf-jvmti.so
> > >
> > > ifndef NO_JVMTI
> > > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> > > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> > > endif
> > >
> > > +ifndef NO_BUILDID_EX
> > > +$(OUTPUT)buildid-ex-sha1:
> > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> > > +$(OUTPUT)buildid-ex-md5:
> > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> > > +endif
> >
> > Can we just build them in the test shell script instead?
> >
> > Thanks
> > Namhyung
>
> That'd mean perf test having a dependency on a compiler :-/ That said
> there are some existing dependencies for BPF compilers.
If doing it in the test shell script ends up being advantageous, we
could skip the test if a suitable compiler isn't available.
- Arnaldo
> Thanks,
> Ian
>
> >
> > > +
> > > ifndef NO_JVMTI
> > > LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
> > >
> > > diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> > > new file mode 100755
> > > index 000000000000..dd9f9c306c34
> > > --- /dev/null
> > > +++ b/tools/perf/tests/shell/buildid.sh
> > > @@ -0,0 +1,101 @@
> > > +#!/bin/sh
> > > +# build id cache operations
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +ex_md5=buildid-ex-md5
> > > +ex_sha1=buildid-ex-sha1
> > > +
> > > +# skip if there are no test binaries
> > > +if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
> > > + ex_dir=$(dirname `which perf`)
> > > + ex_md5=${ex_dir}/buildid-ex-md5
> > > + ex_sha1=${ex_dir}/buildid-ex-sha1
> > > +
> > > + if [ ! -x ${ex_sha1} -a ! -x ${ex_md5} ]; then
> > > + echo "failed: no test binaries"
> > > + exit 2
> > > + fi
> > > +fi
> > > +
> > > +echo "test binaries: ${ex_sha1} ${ex_md5}"
> > > +
> > > +# skip if there's no readelf
> > > +if [ ! -x `which readelf` ]; then
> > > + echo "failed: no readelf, install binutils"
> > > + exit 2
> > > +fi
> > > +
> > > +check()
> > > +{
> > > + id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> > > +
> > > + echo "build id: ${id}"
> > > +
> > > + link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> > > + echo "link: ${link}"
> > > +
> > > + if [ ! -h $link ]; then
> > > + echo "failed: link ${link} does not exist"
> > > + exit 1
> > > + fi
> > > +
> > > + file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
> > > + echo "file: ${file}"
> > > +
> > > + if [ ! -x $file ]; then
> > > + echo "failed: file ${file} does not exist"
> > > + exit 1
> > > + fi
> > > +
> > > + diff ${file} ${1}
> > > + if [ $? -ne 0 ]; then
> > > + echo "failed: ${file} do not match"
> > > + exit 1
> > > + fi
> > > +
> > > + echo "OK for ${1}"
> > > +}
> > > +
> > > +test_add()
> > > +{
> > > + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > > + perf="perf --buildid-dir ${build_id_dir}"
> > > +
> > > + ${perf} buildid-cache -v -a ${1}
> > > + if [ $? -ne 0 ]; then
> > > + echo "failed: add ${1} to build id cache"
> > > + exit 1
> > > + fi
> > > +
> > > + check ${1}
> > > +
> > > + rm -rf ${build_id_dir}
> > > +}
> > > +
> > > +test_record()
> > > +{
> > > + data=$(mktemp /tmp/perf.data.XXX)
> > > + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> > > + perf="perf --buildid-dir ${build_id_dir}"
> > > +
> > > + ${perf} record --buildid-all -o ${data} ${1}
> > > + if [ $? -ne 0 ]; then
> > > + echo "failed: record ${1}"
> > > + exit 1
> > > + fi
> > > +
> > > + check ${1}
> > > +
> > > + rm -rf ${build_id_dir}
> > > + rm -rf ${data}
> > > +}
> > > +
> > > +# add binaries manual via perf buildid-cache -a
> > > +test_add ${ex_sha1}
> > > +test_add ${ex_md5}
> > > +
> > > +# add binaries via perf record post processing
> > > +test_record ${ex_sha1}
> > > +test_record ${ex_md5}
> > > +
> > > +exit ${err}
> > > --
> > > 2.26.2
> > >
--
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCHv3 1/9] perf tools: Add build id shell test
2020-10-02 20:55 ` Arnaldo Carvalho de Melo
@ 2020-10-05 15:58 ` Jiri Olsa
0 siblings, 0 replies; 33+ messages in thread
From: Jiri Olsa @ 2020-10-05 15:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Namhyung Kim, Jiri Olsa, lkml, Peter Zijlstra,
Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan,
Stephane Eranian
Adding test for build id cache that adds binary
with sha1 and md5 build ids and verifies it's
added properly.
The test updates build id cache with perf record
and perf buildid-cache -a.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
v3 changes:
- compile examples directly in the test
- using 'command' instead of 'which' for detection
tools/perf/tests/shell/buildid.sh | 101 ++++++++++++++++++++++++++++++
1 file changed, 101 insertions(+)
create mode 100755 tools/perf/tests/shell/buildid.sh
diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
new file mode 100755
index 000000000000..4861a20edee2
--- /dev/null
+++ b/tools/perf/tests/shell/buildid.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+# build id cache operations
+# SPDX-License-Identifier: GPL-2.0
+
+# skip if there's no readelf
+if ! [ -x "$(command -v readelf)" ]; then
+ echo "failed: no readelf, install binutils"
+ exit 2
+fi
+
+# skip if there's no compiler
+if ! [ -x "$(command -v cc)" ]; then
+ echo "failed: no compiler, install gcc"
+ exit 2
+fi
+
+ex_md5=$(mktemp /tmp/perf.ex.MD5.XXX)
+ex_sha1=$(mktemp /tmp/perf.ex.SHA1.XXX)
+
+echo 'int main(void) { return 0; }' | cc -Wl,--build-id=sha1 -o ${ex_sha1} -x c -
+echo 'int main(void) { return 0; }' | cc -Wl,--build-id=md5 -o ${ex_md5} -x c -
+
+echo "test binaries: ${ex_sha1} ${ex_md5}"
+
+check()
+{
+ id=`readelf -n ${1} 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
+
+ echo "build id: ${id}"
+
+ link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
+ echo "link: ${link}"
+
+ if [ ! -h $link ]; then
+ echo "failed: link ${link} does not exist"
+ exit 1
+ fi
+
+ file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
+ echo "file: ${file}"
+
+ if [ ! -x $file ]; then
+ echo "failed: file ${file} does not exist"
+ exit 1
+ fi
+
+ diff ${file} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: ${file} do not match"
+ exit 1
+ fi
+
+ echo "OK for ${1}"
+}
+
+test_add()
+{
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} buildid-cache -v -a ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: add ${1} to build id cache"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+}
+
+test_record()
+{
+ data=$(mktemp /tmp/perf.data.XXX)
+ build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
+ perf="perf --buildid-dir ${build_id_dir}"
+
+ ${perf} record --buildid-all -o ${data} ${1}
+ if [ $? -ne 0 ]; then
+ echo "failed: record ${1}"
+ exit 1
+ fi
+
+ check ${1}
+
+ rm -rf ${build_id_dir}
+ rm -rf ${data}
+}
+
+# add binaries manual via perf buildid-cache -a
+test_add ${ex_sha1}
+test_add ${ex_md5}
+
+# add binaries via perf record post processing
+test_record ${ex_sha1}
+test_record ${ex_md5}
+
+# cleanup
+rm ${ex_sha1} ${ex_md5}
+
+exit ${err}
--
2.26.2
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCHv2 1/9] perf tools: Add build id shell test
2020-10-02 19:29 ` Jiri Olsa
@ 2020-10-06 1:37 ` Namhyung Kim
2020-10-08 9:11 ` Jiri Olsa
0 siblings, 1 reply; 33+ messages in thread
From: Namhyung Kim @ 2020-10-06 1:37 UTC (permalink / raw)
To: Jiri Olsa
Cc: Ian Rogers, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Sat, Oct 3, 2020 at 4:29 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Oct 02, 2020 at 10:34:51AM -0700, Ian Rogers wrote:
>
> SNIP
>
> > > > +
> > > > LIBJVMTI = libperf-jvmti.so
> > > >
> > > > ifndef NO_JVMTI
> > > > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> > > > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> > > > endif
> > > >
> > > > +ifndef NO_BUILDID_EX
> > > > +$(OUTPUT)buildid-ex-sha1:
> > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> > > > +$(OUTPUT)buildid-ex-md5:
> > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> > > > +endif
> > >
> > > Can we just build them in the test shell script instead?
>
> it would solve the build-directory/install-directory
> lookup search.. but it'd need to do detect compiler
> and depend on it as Ian said
>
> do you have some other reason to compile it in test?
No I just wanted to make it easy to find the binaries
and assumed a compiler is available in the test machine
(which is not true for my company setup.... :-/)
But otherwise we should keep the binaries somewhere
in the install directory..
Thanks
Namhyung
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv2 1/9] perf tools: Add build id shell test
2020-10-06 1:37 ` Namhyung Kim
@ 2020-10-08 9:11 ` Jiri Olsa
2020-10-08 15:22 ` Ian Rogers
0 siblings, 1 reply; 33+ messages in thread
From: Jiri Olsa @ 2020-10-08 9:11 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Tue, Oct 06, 2020 at 10:37:45AM +0900, Namhyung Kim wrote:
> On Sat, Oct 3, 2020 at 4:29 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Fri, Oct 02, 2020 at 10:34:51AM -0700, Ian Rogers wrote:
> >
> > SNIP
> >
> > > > > +
> > > > > LIBJVMTI = libperf-jvmti.so
> > > > >
> > > > > ifndef NO_JVMTI
> > > > > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> > > > > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> > > > > endif
> > > > >
> > > > > +ifndef NO_BUILDID_EX
> > > > > +$(OUTPUT)buildid-ex-sha1:
> > > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> > > > > +$(OUTPUT)buildid-ex-md5:
> > > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> > > > > +endif
> > > >
> > > > Can we just build them in the test shell script instead?
> >
> > it would solve the build-directory/install-directory
> > lookup search.. but it'd need to do detect compiler
> > and depend on it as Ian said
> >
> > do you have some other reason to compile it in test?
>
> No I just wanted to make it easy to find the binaries
> and assumed a compiler is available in the test machine
> (which is not true for my company setup.... :-/)
>
> But otherwise we should keep the binaries somewhere
> in the install directory..
hum, could we go in with the v3 and then you guys
could customize it to what would work for you?
thanks,
jirka
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCHv2 1/9] perf tools: Add build id shell test
2020-10-08 9:11 ` Jiri Olsa
@ 2020-10-08 15:22 ` Ian Rogers
0 siblings, 0 replies; 33+ messages in thread
From: Ian Rogers @ 2020-10-08 15:22 UTC (permalink / raw)
To: Jiri Olsa
Cc: Namhyung Kim, Jiri Olsa, Arnaldo Carvalho de Melo, lkml,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Michael Petlan, Stephane Eranian
On Thu, Oct 8, 2020 at 2:12 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Tue, Oct 06, 2020 at 10:37:45AM +0900, Namhyung Kim wrote:
> > On Sat, Oct 3, 2020 at 4:29 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > >
> > > On Fri, Oct 02, 2020 at 10:34:51AM -0700, Ian Rogers wrote:
> > >
> > > SNIP
> > >
> > > > > > +
> > > > > > LIBJVMTI = libperf-jvmti.so
> > > > > >
> > > > > > ifndef NO_JVMTI
> > > > > > @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> > > > > > $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> > > > > > endif
> > > > > >
> > > > > > +ifndef NO_BUILDID_EX
> > > > > > +$(OUTPUT)buildid-ex-sha1:
> > > > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> > > > > > +$(OUTPUT)buildid-ex-md5:
> > > > > > + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> > > > > > +endif
> > > > >
> > > > > Can we just build them in the test shell script instead?
> > >
> > > it would solve the build-directory/install-directory
> > > lookup search.. but it'd need to do detect compiler
> > > and depend on it as Ian said
> > >
> > > do you have some other reason to compile it in test?
> >
> > No I just wanted to make it easy to find the binaries
> > and assumed a compiler is available in the test machine
> > (which is not true for my company setup.... :-/)
> >
> > But otherwise we should keep the binaries somewhere
> > in the install directory..
>
> hum, could we go in with the v3 and then you guys
> could customize it to what would work for you?
v3 is fine with me, it is progress to have a test. Longer term I hope
we can have binary dependencies on shell tests and get them installed,
etc. libperf would be a motivating case.
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> thanks,
> jirka
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] perf tools: Add build id shell test
2020-09-30 17:15 ` [PATCH 1/9] perf tools: Add build id shell test Jiri Olsa
2020-10-01 2:00 ` Ian Rogers
2020-10-01 19:05 ` [PATCHv2 " Jiri Olsa
@ 2020-10-13 16:13 ` Arnaldo Carvalho de Melo
2020-10-13 19:15 ` Jiri Olsa
2 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-10-13 16:13 UTC (permalink / raw)
To: Jiri Olsa
Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim,
Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian
Em Wed, Sep 30, 2020 at 07:15:04PM +0200, Jiri Olsa escreveu:
> Adding test for build id cache that adds binary
> with sha1 and md5 build ids and verifies it's
> added properly.
>
> The test updates build id cache with perf record
> and perf buildid-cache -a.
[root@five ~]# perf test "build id"
82: build id cache operations : Skip
[root@five ~]# set -o vi
[root@five ~]# perf test -v "build id"
82: build id cache operations :
--- start ---
test child forked, pid 88384
failed: no test binaries
test child finished with -2
---- end ----
build id cache operations: Skip
[root@five ~]#
Also the other patches clashed with Namhyung's patch series, can you
please check?
I've just pushed what I have to acme/perf/core
- Arnaldo
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/Makefile.perf | 14 +++++
> tools/perf/tests/shell/buildid.sh | 90 +++++++++++++++++++++++++++++++
> 2 files changed, 104 insertions(+)
> create mode 100755 tools/perf/tests/shell/buildid.sh
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 920d8afb9238..b2aeefa64e92 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -126,6 +126,8 @@ include ../scripts/utilities.mak
> #
> # Define NO_LIBDEBUGINFOD if you do not want support debuginfod
> #
> +# Define NO_BUILDID_EX if you do not want buildid-ex-* binaries
> +#
>
> # As per kernel Makefile, avoid funny character set dependencies
> unexport LC_ALL
> @@ -349,6 +351,11 @@ ifndef NO_PERF_READ_VDSOX32
> PROGRAMS += $(OUTPUT)perf-read-vdsox32
> endif
>
> +ifndef NO_BUILDID_EX
> +PROGRAMS += $(OUTPUT)buildid-ex-sha1
> +PROGRAMS += $(OUTPUT)buildid-ex-md5
> +endif
> +
> LIBJVMTI = libperf-jvmti.so
>
> ifndef NO_JVMTI
> @@ -756,6 +763,13 @@ $(OUTPUT)perf-read-vdsox32: perf-read-vdso.c util/find-map.c
> $(QUIET_CC)$(CC) -mx32 $(filter -static,$(LDFLAGS)) -Wall -Werror -o $@ perf-read-vdso.c
> endif
>
> +ifndef NO_BUILDID_EX
> +$(OUTPUT)buildid-ex-sha1:
> + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=sha1 -o $@ -x c -
> +$(OUTPUT)buildid-ex-md5:
> + $(QUIET_LINK)echo 'int main(void) { return 0; }' | $(CC) -Wl,--build-id=md5 -o $@ -x c -
> +endif
> +
> ifndef NO_JVMTI
> LIBJVMTI_IN := $(OUTPUT)jvmti/jvmti-in.o
>
> diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
> new file mode 100755
> index 000000000000..57fcd28bc4bd
> --- /dev/null
> +++ b/tools/perf/tests/shell/buildid.sh
> @@ -0,0 +1,90 @@
> +#!/bin/sh
> +# build id cache operations
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# skip if there are no test binaries
> +if [ ! -x buildid-ex-sha1 -a ! -x buildid-ex-md5 ]; then
> + echo "failed: no test binaries"
> + exit 2
> +fi
> +
> +# skip if there's no readelf
> +if [ ! -x `which readelf` ]; then
> + echo "failed: no readelf, install binutils"
> + exit 2
> +fi
> +
> +check()
> +{
> + id=`readelf -n $1 2>/dev/null | grep 'Build ID' | awk '{print $3}'`
> +
> + echo "build id: ${id}"
> +
> + link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
> + echo "link: ${link}"
> +
> + if [ ! -h $link ]; then
> + echo "failed: link ${link} does not exist"
> + exit 1
> + fi
> +
> + file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
> + echo "file: ${file}"
> +
> + if [ ! -x $file ]; then
> + echo "failed: file ${file} does not exist"
> + exit 1
> + fi
> +
> + diff ${file} ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: ${file} do not match"
> + exit 1
> + fi
> +
> + echo "OK for ${1}"
> +}
> +
> +test_add()
> +{
> + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> + perf="perf --buildid-dir ${build_id_dir}"
> +
> + ${perf} buildid-cache -v -a ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: add ${1} to build id cache"
> + exit 1
> + fi
> +
> + check ${1}
> +
> + rm -rf ${build_id_dir}
> +}
> +
> +test_record()
> +{
> + data=$(mktemp /tmp/perf.data.XXX)
> + build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
> + perf="perf --buildid-dir ${build_id_dir}"
> +
> + ${perf} record --buildid-all -o ${data} ${1}
> + if [ $? -ne 0 ]; then
> + echo "failed: record ${1}"
> + exit 1
> + fi
> +
> + check ${1}
> +
> + rm -rf ${build_id_dir}
> + rm -rf ${data}
> +}
> +
> +# add binaries manual via perf buildid-cache -a
> +test_add buildid-ex-sha1
> +test_add buildid-ex-md5
> +
> +# add binaries via perf record post processing
> +test_record buildid-ex-sha1
> +test_record buildid-ex-md5
> +
> +exit ${err}
> --
> 2.26.2
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/9] perf tools: Add build id shell test
2020-10-13 16:13 ` [PATCH " Arnaldo Carvalho de Melo
@ 2020-10-13 19:15 ` Jiri Olsa
0 siblings, 0 replies; 33+ messages in thread
From: Jiri Olsa @ 2020-10-13 19:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers,
Stephane Eranian
On Tue, Oct 13, 2020 at 01:13:40PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 30, 2020 at 07:15:04PM +0200, Jiri Olsa escreveu:
> > Adding test for build id cache that adds binary
> > with sha1 and md5 build ids and verifies it's
> > added properly.
> >
> > The test updates build id cache with perf record
> > and perf buildid-cache -a.
>
>
> [root@five ~]# perf test "build id"
> 82: build id cache operations : Skip
> [root@five ~]# set -o vi
> [root@five ~]# perf test -v "build id"
> 82: build id cache operations :
> --- start ---
> test child forked, pid 88384
> failed: no test binaries
> test child finished with -2
> ---- end ----
> build id cache operations: Skip
> [root@five ~]#
hm, output looks like older version of the test
>
> Also the other patches clashed with Namhyung's patch series, can you
> please check?
right, new api is different now
>
> I've just pushed what I have to acme/perf/core
I rebased and pushed perf/build_id_size branch, also will post new version
thanks,
jirka
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2020-10-13 19:15 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-30 17:15 [PATCH 0/9] perf tools: Add support for build id with different sizes Jiri Olsa
2020-09-30 17:15 ` [PATCH 1/9] perf tools: Add build id shell test Jiri Olsa
2020-10-01 2:00 ` Ian Rogers
2020-10-01 9:25 ` Jiri Olsa
2020-10-01 10:25 ` Jiri Olsa
2020-10-01 16:58 ` Ian Rogers
2020-10-01 19:05 ` [PATCHv2 " Jiri Olsa
2020-10-02 13:07 ` Namhyung Kim
2020-10-02 17:34 ` Ian Rogers
2020-10-02 19:29 ` Jiri Olsa
2020-10-06 1:37 ` Namhyung Kim
2020-10-08 9:11 ` Jiri Olsa
2020-10-08 15:22 ` Ian Rogers
2020-10-02 20:55 ` Arnaldo Carvalho de Melo
2020-10-05 15:58 ` [PATCHv3 " Jiri Olsa
2020-10-13 16:13 ` [PATCH " Arnaldo Carvalho de Melo
2020-10-13 19:15 ` Jiri Olsa
2020-09-30 17:15 ` [PATCH 2/9] perf tools: Use build_id object in dso Jiri Olsa
2020-10-01 2:01 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 3/9] perf tools: Pass build_id object to filename__read_build_id Jiri Olsa
2020-10-01 5:17 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 4/9] perf tools: Pass build id object to sysfs__read_build_id Jiri Olsa
2020-10-01 5:20 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 5/9] perf tools: Pass build_id object to build_id__sprintf Jiri Olsa
2020-10-01 5:24 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 6/9] perf tools: Pass build_id object to dso__set_build_id Jiri Olsa
2020-10-01 5:25 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 7/9] perf tools: Pass build_id object to dso__build_id_equal Jiri Olsa
2020-10-01 5:25 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 8/9] perf tools: Add size to struct perf_record_header_build_id Jiri Olsa
2020-10-01 5:31 ` Ian Rogers
2020-09-30 17:15 ` [PATCH 9/9] perf tools: Align buildid list output for short build ids Jiri Olsa
2020-10-01 5:35 ` Ian Rogers
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).