From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Masami Hiramatsu <mhiramat@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 10:45:38 -0300 [thread overview]
Message-ID: <20160426134538.GA20998@kernel.org> (raw)
In-Reply-To: <20160426090221.11891.87192.stgit@devbox>
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.
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@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)) {
next prev parent reply other threads:[~2016-04-26 13:45 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 [this message]
2016-04-26 14:47 ` Masami Hiramatsu
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=20160426134538.GA20998@kernel.org \
--to=acme@kernel.org \
--cc=ananth@linux.vnet.ibm.com \
--cc=hemant@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@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;
as well as URLs for NNTP newsgroup(s).