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

From: Masami Hiramatsu
Date: Tue Apr 26 2016 - 10:47:36 EST

Next message: Parth Sane: "Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis"
Previous message: Masami Hiramatsu: "Re: [PATCH 2/2] perf probe: Fix offline module name missmatch issue"
In reply to: Arnaldo Carvalho de Melo: "Re: [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: "[PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, 26 Apr 2016 10:45:38 -0300
Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:

> 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.

Right, that the next patch does. And I agree with you that it should be
merged to this patch.

>
> 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."?

Ah, this is just a note that someone directly accessing the cache.
But anyway, it is just a design note.

Thank you,

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

--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>

Next message: Parth Sane: "Re: [PATCH v6] rtl8712: Fixed alignment to match open parenthesis"
Previous message: Masami Hiramatsu: "Re: [PATCH 2/2] perf probe: Fix offline module name missmatch issue"
In reply to: Arnaldo Carvalho de Melo: "Re: [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: "[PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]