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

From: Arnaldo Carvalho de Melo
Date: Tue Apr 26 2016 - 09:45:52 EST

Next message: Florian Vallee: "[PATCH] can: m_can: fix bitrate setup on latest silicon"
Previous message: Linus Walleij: "Re: [PATCH v10 4/9] phy: Add Tegra XUSB pad controller support"
In reply to: Masami Hiramatsu: "[PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid"
Next in thread: Masami Hiramatsu: "Re: [PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Tue, Apr 26, 2016 at 06:02:22PM +0900, Masami Hiramatsu escreveu:
> 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 build-id based symlink changes to point to the
> path/to/bin/buildid, not path/to/bin/buildid/elf.

Ok, but what happens to existing caches? Not a problem? I.e. the lookups
are made from ~/.debug/.build-id/, so it will just follow that symlink
and if the file was inserted in the cache before this change, it will
find it there, ok.

If inserted later, ditto.

Now what do you mean by "Note that the build-id based symlink changes to
point to the path/to/bin/buildid, not path/to/bin/buildid/elf."?

Probably the 'perf buildid-cache --purge" will just follow the symlink,
so it'll work.

- Arnaldo

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> ---
> tools/perf/util/build-id.c | 65 +++++++++++++++++++++++++++++++-------------
> tools/perf/util/dso.h | 5 +++
> tools/perf/util/symbol.c | 2 +
> 3 files changed, 52 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index b6ecf87..46a8bcc 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,15 +155,35 @@ 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;
> +
> + 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)
> @@ -341,7 +362,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 +374,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 +395,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 +431,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 +440,17 @@ 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;
>
> 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 +464,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 +473,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 +501,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 +520,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 +538,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 +550,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..c57cb47 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1685,7 +1685,7 @@ 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);
>
> if (access(path, F_OK)) {

Next message: Florian Vallee: "[PATCH] can: m_can: fix bitrate setup on latest silicon"
Previous message: Linus Walleij: "Re: [PATCH v10 4/9] phy: Add Tegra XUSB pad controller support"
In reply to: Masami Hiramatsu: "[PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid"
Next in thread: Masami Hiramatsu: "Re: [PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]