Commit 834ebb56 authored by James Clark's avatar James Clark Committed by Namhyung Kim
Browse files

perf tools: Don't read build-ids from non-regular files 

Simplify the build ID reading code by removing the non-blocking option.
Having to pass the correct option to this function was fragile and a
mistake would result in a hang, see the linked fix. Furthermore,
compressed files are always opened blocking anyway, ignoring the
non-blocking option.

We also don't expect to read build IDs from non-regular files. The only
hits to this function that are non-regular are devices that won't be elf
files with build IDs, for example "/dev/dri/renderD129".

Now instead of opening these as non-blocking and failing to read, we
skip them. Even if something like a pipe or character device did have a
build ID, I don't think it would have worked because you need to call
read() in a loop, check for -EAGAIN and handle timeouts to make
non-blocking reads work.

Link: https://lore.kernel.org/linux-perf-users/20251022-james-perf-fix-dso-block-v1-1-c4faab150546@linaro.org/


Signed-off-by: default avatarJames Clark <james.clark@linaro.org>
Signed-off-by: default avatarNamhyung Kim <namhyung@kernel.org>
parent c9573287

tools/perf/bench/inject-buildid.c

+1 −1
Original line number Diff line number Diff line
@@ -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, /*block=*/true) < 0)

	if (filename__read_build_id(fpath, &bid) < 0)

		return 0;



	dso->name = realpath(fpath, NULL);

tools/perf/builtin-buildid-cache.c

+4 −4
Original line number Diff line number Diff line
@@ -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, /*block=*/true);

	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);
@@ -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, /*block=*/true);

	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);
@@ -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, /*block=*/true) == -1) {

	if (filename__read_build_id(filename, &bid) == -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, /*block=*/true);

	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);

tools/perf/builtin-inject.c

+2 −2
Original line number Diff line number Diff line
@@ -668,12 +668,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, /*block=*/true) > 0)

	if (filename__read_build_id(dso__long_name(dso), &bid) > 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, /*block=*/true) > 0)

		if (new_name && filename__read_build_id(new_name, &bid) > 0)

			dso__set_build_id(dso, &bid);

		free(new_name);

	}

tools/perf/tests/pe-file-parsing.c

+2 −2
Original line number Diff line number Diff line
@@ -37,7 +37,7 @@ static int run_dir(const char *d) 
	size_t idx;



	scnprintf(filename, PATH_MAX, "%s/pe-file.exe", d);

	ret = filename__read_build_id(filename, &bid, /*block=*/true);

	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(bid.data, expect_build_id,
@@ -49,7 +49,7 @@ 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, &bid, /*block=*/true);

	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(bid.data, expect_build_id,

tools/perf/tests/sdt.c

+1 −1
Original line number Diff line number Diff line
@@ -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, /*block=*/true);

	err = filename__read_build_id(filename, &bid);

	if (err < 0) {

		pr_debug("Failed to read build id of %s\n", filename);

		return err;