[PATCH perf/core v6 01/14] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid

[Masami Hiramatsu]

From: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>

Use path/to/bin/buildid/elf instead of path/to/bin/buildid
to store corresponding elf binary.
This also stores vdso in buildid/vdso, kallsyms in buildid/kallsyms.

Note that the existing caches are not updated until user adds
or updates the cache. Anyway, if there is the old style build-id
cache it falls back to use it. (IOW, it is backward compatible)

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
---
 Changes in v6:
  - Replace local __is_regular_file with is_regular_file in util.c
    (Thank you Namhyung!)
  - Fix dso__build_id_is_kmod() to check symlink in buildid cache correctly.
 Changes in v5:
  - Support old style buildid caches.
---
 tools/perf/util/build-id.c |   91 +++++++++++++++++++++++++++++++-------------
 tools/perf/util/dso.h      |    5 ++
 tools/perf/util/symbol.c   |   15 +++++--
 3 files changed, 79 insertions(+), 32 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index b6ecf87..387581a 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -144,7 +144,8 @@ static int asnprintf(char **strp, size_t size, const char *fmt, ...)
 	return ret;
 }
 
-static char *build_id__filename(const char *sbuild_id, char *bf, size_t size)
+static char *build_id_cache__linkname(const char *sbuild_id, char *bf,
+				      size_t size)
 {
 	char *tmp = bf;
 	int ret = asnprintf(&bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
@@ -154,23 +155,52 @@ static char *build_id__filename(const char *sbuild_id, char *bf, size_t size)
 	return bf;
 }
 
+static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso)
+{
+	return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : "elf");
+}
+
 char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
 {
-	char build_id_hex[SBUILD_ID_SIZE];
+	bool is_kallsyms = dso__is_kallsyms((struct dso *)dso);
+	bool is_vdso = dso__is_vdso((struct dso *)dso);
+	char sbuild_id[SBUILD_ID_SIZE];
+	char *linkname;
+	bool alloc = (bf == NULL);
+	int ret;
 
 	if (!dso->has_build_id)
 		return NULL;
 
-	build_id__sprintf(dso->build_id, sizeof(dso->build_id), build_id_hex);
-	return build_id__filename(build_id_hex, bf, size);
+	build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
+	linkname = build_id_cache__linkname(sbuild_id, NULL, 0);
+	if (!linkname)
+		return NULL;
+
+	/* Check if old style build_id cache */
+	if (is_regular_file(linkname))
+		ret = asnprintf(&bf, size, "%s", linkname);
+	else
+		ret = asnprintf(&bf, size, "%s/%s", linkname,
+			 build_id_cache__basename(is_kallsyms, is_vdso));
+	if (ret < 0 || (!alloc && size < (unsigned int)ret))
+		bf = NULL;
+	free(linkname);
+
+	return bf;
 }
 
 bool dso__build_id_is_kmod(const struct dso *dso, char *bf, size_t size)
 {
-	char *id_name, *ch;
+	char *id_name = NULL, *ch;
 	struct stat sb;
+	char sbuild_id[SBUILD_ID_SIZE];
 
-	id_name = dso__build_id_filename(dso, bf, size);
+	if (!dso->has_build_id)
+		goto err;
+
+	build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
+	id_name = build_id_cache__linkname(sbuild_id, NULL, 0);
 	if (!id_name)
 		goto err;
 	if (access(id_name, F_OK))
@@ -194,18 +224,14 @@ bool dso__build_id_is_kmod(const struct dso *dso, char *bf, size_t size)
 	if (ch - 3 < bf)
 		goto err;
 
+	free(id_name);
 	return strncmp(".ko", ch - 3, 3) == 0;
 err:
-	/*
-	 * If dso__build_id_filename work, get id_name again,
-	 * because id_name points to bf and is broken.
-	 */
-	if (id_name)
-		id_name = dso__build_id_filename(dso, bf, size);
 	pr_err("Invalid build id: %s\n", id_name ? :
 					 dso->long_name ? :
 					 dso->short_name ? :
 					 "[unknown]");
+	free(id_name);
 	return false;
 }
 
@@ -341,7 +367,8 @@ void disable_buildid_cache(void)
 }
 
 static char *build_id_cache__dirname_from_path(const char *name,
-					       bool is_kallsyms, bool is_vdso)
+					       bool is_kallsyms, bool is_vdso,
+					       const char *sbuild_id)
 {
 	char *realname = (char *)name, *filename;
 	bool slash = is_kallsyms || is_vdso;
@@ -352,8 +379,9 @@ static char *build_id_cache__dirname_from_path(const char *name,
 			return NULL;
 	}
 
-	if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
-		     is_vdso ? DSO__NAME_VDSO : realname) < 0)
+	if (asprintf(&filename, "%s%s%s%s%s", buildid_dir, slash ? "/" : "",
+		     is_vdso ? DSO__NAME_VDSO : realname,
+		     sbuild_id ? "/" : "", sbuild_id ?: "") < 0)
 		filename = NULL;
 
 	if (!slash)
@@ -372,7 +400,8 @@ int build_id_cache__list_build_ids(const char *pathname,
 	int ret = 0;
 
 	list = strlist__new(NULL, NULL);
-	dir_name = build_id_cache__dirname_from_path(pathname, false, false);
+	dir_name = build_id_cache__dirname_from_path(pathname, false, false,
+						     NULL);
 	if (!list || !dir_name) {
 		ret = -ENOMEM;
 		goto out;
@@ -407,7 +436,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 {
 	const size_t size = PATH_MAX;
 	char *realname = NULL, *filename = NULL, *dir_name = NULL,
-	     *linkname = zalloc(size), *targetname, *tmp;
+	     *linkname = zalloc(size), *tmp;
 	int err = -1;
 
 	if (!is_kallsyms) {
@@ -416,14 +445,22 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 			goto out_free;
 	}
 
-	dir_name = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
+	dir_name = build_id_cache__dirname_from_path(name, is_kallsyms,
+						     is_vdso, sbuild_id);
 	if (!dir_name)
 		goto out_free;
 
+	/* Remove old style build-id cache */
+	if (is_regular_file(dir_name))
+		if (unlink(dir_name))
+			goto out_free;
+
 	if (mkdir_p(dir_name, 0755))
 		goto out_free;
 
-	if (asprintf(&filename, "%s/%s", dir_name, sbuild_id) < 0) {
+	/* Save the allocated buildid dirname */
+	if (asprintf(&filename, "%s/%s", dir_name,
+		     build_id_cache__basename(is_kallsyms, is_vdso)) < 0) {
 		filename = NULL;
 		goto out_free;
 	}
@@ -437,7 +474,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 			goto out_free;
 	}
 
-	if (!build_id__filename(sbuild_id, linkname, size))
+	if (!build_id_cache__linkname(sbuild_id, linkname, size))
 		goto out_free;
 	tmp = strrchr(linkname, '/');
 	*tmp = '\0';
@@ -446,10 +483,10 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 		goto out_free;
 
 	*tmp = '/';
-	targetname = filename + strlen(buildid_dir) - 5;
-	memcpy(targetname, "../..", 5);
+	tmp = dir_name + strlen(buildid_dir) - 5;
+	memcpy(tmp, "../..", 5);
 
-	if (symlink(targetname, linkname) == 0)
+	if (symlink(tmp, linkname) == 0)
 		err = 0;
 out_free:
 	if (!is_kallsyms)
@@ -474,7 +511,7 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
 bool build_id_cache__cached(const char *sbuild_id)
 {
 	bool ret = false;
-	char *filename = build_id__filename(sbuild_id, NULL, 0);
+	char *filename = build_id_cache__linkname(sbuild_id, NULL, 0);
 
 	if (filename && !access(filename, F_OK))
 		ret = true;
@@ -493,7 +530,7 @@ int build_id_cache__remove_s(const char *sbuild_id)
 	if (filename == NULL || linkname == NULL)
 		goto out_free;
 
-	if (!build_id__filename(sbuild_id, linkname, size))
+	if (!build_id_cache__linkname(sbuild_id, linkname, size))
 		goto out_free;
 
 	if (access(linkname, F_OK))
@@ -511,7 +548,7 @@ int build_id_cache__remove_s(const char *sbuild_id)
 	tmp = strrchr(linkname, '/') + 1;
 	snprintf(tmp, size - (tmp - linkname), "%s", filename);
 
-	if (unlink(linkname))
+	if (rm_rf(linkname))
 		goto out_free;
 
 	err = 0;
@@ -523,7 +560,7 @@ out_free:
 
 static int dso__cache_build_id(struct dso *dso, struct machine *machine)
 {
-	bool is_kallsyms = dso->kernel && dso->long_name[0] != '/';
+	bool is_kallsyms = dso__is_kallsyms(dso);
 	bool is_vdso = dso__is_vdso(dso);
 	const char *name = dso->long_name;
 	char nm[PATH_MAX];
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 0953280..76d79d0 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -349,6 +349,11 @@ static inline bool dso__is_kcore(struct dso *dso)
 	       dso->binary_type == DSO_BINARY_TYPE__GUEST_KCORE;
 }
 
+static inline bool dso__is_kallsyms(struct dso *dso)
+{
+	return dso->kernel && dso->long_name[0] != '/';
+}
+
 void dso__free_a2l(struct dso *dso);
 
 enum dso_type dso__type(struct dso *dso, struct machine *machine);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 415c4f6..9463c7d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1685,13 +1685,18 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
 	if (!find_matching_kcore(map, path, sizeof(path)))
 		return strdup(path);
 
-	scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s",
+	scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s/kallsyms",
 		  buildid_dir, sbuild_id);
-
+	/* Try old style kallsyms cache */
 	if (access(path, F_OK)) {
-		pr_err("No kallsyms or vmlinux with build-id %s was found\n",
-		       sbuild_id);
-		return NULL;
+		scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s",
+			  buildid_dir, sbuild_id);
+
+		if (access(path, F_OK)) {
+			pr_err("No kallsyms or vmlinux with build-id %s was found\n",
+			       sbuild_id);
+			return NULL;
+		}
 	}
 
 	return strdup(path);