Re: [PATCH perf/core v5 02/15] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid

Arnaldo Carvalho de Melo
2016-04-27T21:23:57Z

Em Thu, Apr 28, 2016 at 03:37:23AM +0900, Masami Hiramatsu escreveu:
> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> 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@hitachi.com>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> Changes in v5:
> - Support old style buildid caches.

> + /* Remove old style build-id cache */
> + if (__is_regular_file(dir_name))
> + if (unlink(dir_name))
> + goto out_free;

Shouldn't this just fail and require the user to use --force/-f or purge
first?

- Arnaldo

> 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 +479,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 +488,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 +516,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 +535,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 +553,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 +565,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);