perf symbol: Add blocking argument to filename__read_build_id
When synthesizing build-ids, for build ID mmap2 events, they will be
added for data mmaps if -d/--data is specified. The files opened for
their build IDs may block on the open causing perf to hang during
synthesis. There is some robustness in existing calls to
filename__read_build_id by checking the file path is to a regular
file, which unfortunately fails for symlinks. Rather than adding more
is_regular_file calls, switch filename__read_build_id to take a
"block" argument and specify O_NONBLOCK when this is false. The
existing is_regular_file checking callers and the event synthesis
callers are made to pass false and thereby avoiding the hang.
Fixes: 53b00ff358
("perf record: Make --buildid-mmap the default")
Signed-off-by: Ian Rogers <irogers@google.com>
Link: https://lore.kernel.org/r/20250823000024.724394-3-irogers@google.com
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
This commit is contained in:
parent
ba0b7081f7
commit
2c369d91d0
|
@ -85,7 +85,7 @@ static int add_dso(const char *fpath, const struct stat *sb __maybe_unused,
|
|||
if (typeflag == FTW_D || typeflag == FTW_SL)
|
||||
return 0;
|
||||
|
||||
if (filename__read_build_id(fpath, &bid) < 0)
|
||||
if (filename__read_build_id(fpath, &bid, /*block=*/true) < 0)
|
||||
return 0;
|
||||
|
||||
dso->name = realpath(fpath, NULL);
|
||||
|
|
|
@ -180,7 +180,7 @@ static int build_id_cache__add_file(const char *filename, struct nsinfo *nsi)
|
|||
struct nscookie nsc;
|
||||
|
||||
nsinfo__mountns_enter(nsi, &nsc);
|
||||
err = filename__read_build_id(filename, &bid);
|
||||
err = filename__read_build_id(filename, &bid, /*block=*/true);
|
||||
nsinfo__mountns_exit(&nsc);
|
||||
if (err < 0) {
|
||||
pr_debug("Couldn't read a build-id in %s\n", filename);
|
||||
|
@ -204,7 +204,7 @@ static int build_id_cache__remove_file(const char *filename, struct nsinfo *nsi)
|
|||
int err;
|
||||
|
||||
nsinfo__mountns_enter(nsi, &nsc);
|
||||
err = filename__read_build_id(filename, &bid);
|
||||
err = filename__read_build_id(filename, &bid, /*block=*/true);
|
||||
nsinfo__mountns_exit(&nsc);
|
||||
if (err < 0) {
|
||||
pr_debug("Couldn't read a build-id in %s\n", filename);
|
||||
|
@ -280,7 +280,7 @@ static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
|
|||
if (!dso__build_id_filename(dso, filename, sizeof(filename), false))
|
||||
return true;
|
||||
|
||||
if (filename__read_build_id(filename, &bid) == -1) {
|
||||
if (filename__read_build_id(filename, &bid, /*block=*/true) == -1) {
|
||||
if (errno == ENOENT)
|
||||
return false;
|
||||
|
||||
|
@ -309,7 +309,7 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
|
|||
int err;
|
||||
|
||||
nsinfo__mountns_enter(nsi, &nsc);
|
||||
err = filename__read_build_id(filename, &bid);
|
||||
err = filename__read_build_id(filename, &bid, /*block=*/true);
|
||||
nsinfo__mountns_exit(&nsc);
|
||||
if (err < 0) {
|
||||
pr_debug("Couldn't read a build-id in %s\n", filename);
|
||||
|
|
|
@ -680,12 +680,12 @@ static int dso__read_build_id(struct dso *dso)
|
|||
|
||||
mutex_lock(dso__lock(dso));
|
||||
nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
|
||||
if (filename__read_build_id(dso__long_name(dso), &bid) > 0)
|
||||
if (filename__read_build_id(dso__long_name(dso), &bid, /*block=*/true) > 0)
|
||||
dso__set_build_id(dso, &bid);
|
||||
else if (dso__nsinfo(dso)) {
|
||||
char *new_name = dso__filename_with_chroot(dso, dso__long_name(dso));
|
||||
|
||||
if (new_name && filename__read_build_id(new_name, &bid) > 0)
|
||||
if (new_name && filename__read_build_id(new_name, &bid, /*block=*/true) > 0)
|
||||
dso__set_build_id(dso, &bid);
|
||||
free(new_name);
|
||||
}
|
||||
|
|
|
@ -31,7 +31,7 @@ static int build_id_cache__add_file(const char *filename)
|
|||
struct build_id bid = { .size = 0, };
|
||||
int err;
|
||||
|
||||
err = filename__read_build_id(filename, &bid);
|
||||
err = filename__read_build_id(filename, &bid, /*block=*/true);
|
||||
if (err < 0) {
|
||||
pr_debug("Failed to read build id of %s\n", filename);
|
||||
return err;
|
||||
|
|
|
@ -115,7 +115,7 @@ int filename__snprintf_build_id(const char *pathname, char *sbuild_id, size_t sb
|
|||
struct build_id bid = { .size = 0, };
|
||||
int ret;
|
||||
|
||||
ret = filename__read_build_id(pathname, &bid);
|
||||
ret = filename__read_build_id(pathname, &bid, /*block=*/true);
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
|
||||
|
@ -841,7 +841,7 @@ static int filename__read_build_id_ns(const char *filename,
|
|||
int ret;
|
||||
|
||||
nsinfo__mountns_enter(nsi, &nsc);
|
||||
ret = filename__read_build_id(filename, bid);
|
||||
ret = filename__read_build_id(filename, bid, /*block=*/true);
|
||||
nsinfo__mountns_exit(&nsc);
|
||||
|
||||
return ret;
|
||||
|
|
|
@ -110,8 +110,12 @@ struct debuginfo *debuginfo__new(const char *path)
|
|||
if (!dso)
|
||||
goto out;
|
||||
|
||||
/* Set the build id for DSO_BINARY_TYPE__BUILDID_DEBUGINFO */
|
||||
if (is_regular_file(path) && filename__read_build_id(path, &bid) > 0)
|
||||
/*
|
||||
* Set the build id for DSO_BINARY_TYPE__BUILDID_DEBUGINFO. Don't block
|
||||
* incase the path isn't for a regular file.
|
||||
*/
|
||||
assert(!dso__has_build_id(dso));
|
||||
if (filename__read_build_id(path, &bid, /*block=*/false) > 0)
|
||||
dso__set_build_id(dso, &bid);
|
||||
|
||||
for (type = distro_dwarf_types;
|
||||
|
|
|
@ -81,13 +81,13 @@ static int dsos__read_build_ids_cb(struct dso *dso, void *data)
|
|||
return 0;
|
||||
}
|
||||
nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
|
||||
if (filename__read_build_id(dso__long_name(dso), &bid) > 0) {
|
||||
if (filename__read_build_id(dso__long_name(dso), &bid, /*block=*/true) > 0) {
|
||||
dso__set_build_id(dso, &bid);
|
||||
args->have_build_id = true;
|
||||
} else if (errno == ENOENT && dso__nsinfo(dso)) {
|
||||
char *new_name = dso__filename_with_chroot(dso, dso__long_name(dso));
|
||||
|
||||
if (new_name && filename__read_build_id(new_name, &bid) > 0) {
|
||||
if (new_name && filename__read_build_id(new_name, &bid, /*block=*/true) > 0) {
|
||||
dso__set_build_id(dso, &bid);
|
||||
args->have_build_id = true;
|
||||
}
|
||||
|
|
|
@ -902,7 +902,7 @@ out_close:
|
|||
|
||||
#else // HAVE_LIBBFD_BUILDID_SUPPORT
|
||||
|
||||
static int read_build_id(const char *filename, struct build_id *bid)
|
||||
static int read_build_id(const char *filename, struct build_id *bid, bool block)
|
||||
{
|
||||
size_t size = sizeof(bid->data);
|
||||
int fd, err = -1;
|
||||
|
@ -911,7 +911,7 @@ static int read_build_id(const char *filename, struct build_id *bid)
|
|||
if (size < BUILD_ID_SIZE)
|
||||
goto out;
|
||||
|
||||
fd = open(filename, O_RDONLY);
|
||||
fd = open(filename, block ? O_RDONLY : (O_RDONLY | O_NONBLOCK));
|
||||
if (fd < 0)
|
||||
goto out;
|
||||
|
||||
|
@ -934,7 +934,7 @@ out:
|
|||
|
||||
#endif // HAVE_LIBBFD_BUILDID_SUPPORT
|
||||
|
||||
int filename__read_build_id(const char *filename, struct build_id *bid)
|
||||
int filename__read_build_id(const char *filename, struct build_id *bid, bool block)
|
||||
{
|
||||
struct kmod_path m = { .name = NULL, };
|
||||
char path[PATH_MAX];
|
||||
|
@ -958,9 +958,10 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
|
|||
}
|
||||
close(fd);
|
||||
filename = path;
|
||||
block = true;
|
||||
}
|
||||
|
||||
err = read_build_id(filename, bid);
|
||||
err = read_build_id(filename, bid, block);
|
||||
|
||||
if (m.comp)
|
||||
unlink(filename);
|
||||
|
|
|
@ -85,7 +85,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, struct build_id *bid)
|
||||
int filename__read_build_id(const char *filename, struct build_id *bid, bool block)
|
||||
{
|
||||
int fd, ret = -1;
|
||||
bool need_swap = false, elf32;
|
||||
|
@ -102,7 +102,7 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
|
|||
void *phdr, *buf = NULL;
|
||||
ssize_t phdr_size, ehdr_size, buf_size = 0;
|
||||
|
||||
fd = open(filename, O_RDONLY);
|
||||
fd = open(filename, block ? O_RDONLY : (O_RDONLY | O_NONBLOCK));
|
||||
if (fd < 0)
|
||||
return -1;
|
||||
|
||||
|
@ -323,7 +323,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused,
|
|||
if (ret >= 0)
|
||||
RC_CHK_ACCESS(dso)->is_64_bit = ret;
|
||||
|
||||
if (filename__read_build_id(ss->name, &bid) > 0)
|
||||
if (filename__read_build_id(ss->name, &bid, /*block=*/true) > 0)
|
||||
dso__set_build_id(dso, &bid);
|
||||
return 0;
|
||||
}
|
||||
|
|
|
@ -1869,14 +1869,14 @@ int dso__load(struct dso *dso, struct map *map)
|
|||
|
||||
/*
|
||||
* Read the build id if possible. This is required for
|
||||
* DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work
|
||||
* DSO_BINARY_TYPE__BUILDID_DEBUGINFO to work. Don't block in case path
|
||||
* isn't for a regular file.
|
||||
*/
|
||||
if (!dso__has_build_id(dso) &&
|
||||
is_regular_file(dso__long_name(dso))) {
|
||||
if (!dso__has_build_id(dso)) {
|
||||
struct build_id bid = { .size = 0, };
|
||||
|
||||
__symbol__join_symfs(name, PATH_MAX, dso__long_name(dso));
|
||||
if (filename__read_build_id(name, &bid) > 0)
|
||||
if (filename__read_build_id(name, &bid, /*block=*/false) > 0)
|
||||
dso__set_build_id(dso, &bid);
|
||||
}
|
||||
|
||||
|
|
|
@ -140,7 +140,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 filename__read_build_id(const char *filename, struct build_id *id, bool block);
|
||||
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,
|
||||
|
|
|
@ -401,7 +401,7 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
|
|||
nsi = nsinfo__new(event->pid);
|
||||
nsinfo__mountns_enter(nsi, &nc);
|
||||
|
||||
rc = filename__read_build_id(event->filename, &bid) > 0 ? 0 : -1;
|
||||
rc = filename__read_build_id(event->filename, &bid, /*block=*/false) > 0 ? 0 : -1;
|
||||
|
||||
nsinfo__mountns_exit(&nc);
|
||||
nsinfo__put(nsi);
|
||||
|
|
Loading…
Reference in New Issue