From: Masami Hiramatsu <mhiramat@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-kernel@vger.kernel.org, Namhyung Kim <namhyung@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Hemant Kumar <hemant@linux.vnet.ibm.com>,
Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Subject: Re: [PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid
Date: Tue, 26 Apr 2016 23:47:27 +0900 [thread overview]
Message-ID: <20160426234727.d9901784d226debdc95768d0@kernel.org> (raw)
In-Reply-To: <20160426134538.GA20998@kernel.org>
On Tue, 26 Apr 2016 10:45:38 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Tue, Apr 26, 2016 at 06:02:22PM +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 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@hitachi.com>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> > 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@kernel.org>
next prev parent reply other threads:[~2016-04-26 14:47 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-26 9:02 [PATCH perf/core v4 00/19] perf-probe --cache and SDT support Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings Masami Hiramatsu
2016-04-26 13:36 ` Arnaldo Carvalho de Melo
2016-04-26 14:40 ` Masami Hiramatsu
2016-04-26 14:59 ` Arnaldo Carvalho de Melo
2016-04-27 18:44 ` Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid Masami Hiramatsu
2016-04-26 13:45 ` Arnaldo Carvalho de Melo
2016-04-26 14:47 ` Masami Hiramatsu [this message]
2016-04-26 9:02 ` [PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache Masami Hiramatsu
2016-04-26 13:47 ` Arnaldo Carvalho de Melo
2016-04-26 14:42 ` Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory Masami Hiramatsu
2016-04-26 13:40 ` Arnaldo Carvalho de Melo
2016-04-26 14:07 ` Arnaldo Carvalho de Melo
2016-04-26 14:52 ` Masami Hiramatsu
2016-04-26 15:00 ` Arnaldo Carvalho de Melo
2016-04-27 15:35 ` [tip:perf/core] perf tools: Add lsdir() helper " tip-bot for Masami Hiramatsu
2016-04-26 9:02 ` [PATCH perf/core v4 05/19] perf-buildid-cache: Use lsdir for looking up buildid caches Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 06/19] perf-probe: Let probe_file__add_event return 0 if succeeded Masami Hiramatsu
2016-04-26 13:49 ` Arnaldo Carvalho de Melo
2016-04-27 15:35 ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 07/19] perf probe: Add --cache option to cache the probe definitions Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 08/19] perf probe: Use cache entry if possible Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 09/19] perf probe: Show all cached probes Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 10/19] perf probe: Remove caches when --cache is given Masami Hiramatsu
2016-04-26 9:03 ` [PATCH perf/core v4 11/19] perf/sdt: ELF support for SDT Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 12/19] perf probe: Add group name support Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 13/19] perf-probe: Set default kprobe group name if it is not given Masami Hiramatsu
2016-04-26 13:50 ` Arnaldo Carvalho de Melo
2016-04-27 15:35 ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Masami Hiramatsu
2016-04-27 15:19 ` Hemant Kumar
2016-04-27 15:28 ` Arnaldo Carvalho de Melo
2016-04-27 19:36 ` Masami Hiramatsu
2016-04-27 20:23 ` Hemant Kumar
2016-04-27 20:16 ` Hemant Kumar
2016-04-26 9:04 ` [PATCH perf/core v4 15/19] perf probe: Accept %sdt and %cached event name Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 16/19] perf-list: Show SDT and pre-cached events Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 17/19] perf-list: Skip SDTs placed in invalid binaries Masami Hiramatsu
2016-04-26 9:04 ` [PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events Masami Hiramatsu
2016-04-27 15:34 ` Hemant Kumar
2016-04-27 18:51 ` Masami Hiramatsu
2016-04-26 9:05 ` [PATCH perf/core v4 19/19] perf probe: Support @BUILDID or @FILE suffix for SDT events Masami Hiramatsu
2016-04-27 15:36 ` [PATCH perf/core v4 00/19] perf-probe --cache and SDT support Hemant Kumar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160426234727.d9901784d226debdc95768d0@kernel.org \
--to=mhiramat@kernel.org \
--cc=acme@kernel.org \
--cc=ananth@linux.vnet.ibm.com \
--cc=hemant@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome