[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150828160704.GB11407@kernel.org>
Date: Fri, 28 Aug 2015 13:07:04 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>,
linux-kernel@...r.kernel.org,
Adrian Hunter <adrian.hunter@...el.com>,
Ingo Molnar <mingo@...hat.com>,
Paul Mackerras <paulus@...ba.org>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Borislav Petkov <bp@...e.de>,
Hemant Kumar <hemant@...ux.vnet.ibm.com>
Subject: Re: [RFC PATCH perf/core v3 02/17] perf-buildid-cache: Use
path/to/bin/buildid/elf instead of path/to/bin/buildid
Em Sat, Aug 15, 2015 at 08:42:57PM +0900, Masami Hiramatsu escreveu:
> 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.
Please break this patch into multiple ones to ease review, for instance,
the very first one should be:
"This patch makes path/to/bin/buildid become a directory, and the first
content it'll have is the current ELF file, that will be named
path/to/bin/build/ELF, the following patches will add more entries in
the path/to/bin/buildid/ directory with other buildid specific content,
such as kallsyms, SDT metadata, etc"
And this would even fix a problem that we have now that is if we
have a kallsyms in the build-id cache and then, later, install the
matching vmlinux, we keep caching just the kallsyms based metadata for
that buildid (the kernel in this example).
Can you please do that?
Thanks,
- Arnaldo
> Note that the build-id based symlink changes to point to the
> path/to/bin/buildid, not path/to/bin/buildid/elf.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> ---
> Changes in v3:
> - Update for the latest perf/core
> - Use sbuild_id for stringified build_id buffer.
> ---
> 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 4a2c2f0..f65d7c2 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -112,7 +112,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,
> @@ -122,15 +123,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;
> }
>
> #define dsos__for_each_with_build_id(pos, head) \
> @@ -261,7 +282,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;
> @@ -272,8 +294,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)
> @@ -292,7 +315,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;
> @@ -327,7 +351,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) {
> @@ -336,14 +360,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;
> }
> @@ -357,7 +384,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';
> @@ -366,10 +393,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)
> @@ -394,7 +421,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;
> @@ -413,7 +440,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))
> @@ -431,7 +458,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;
> @@ -443,7 +470,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 c73276d..07f37c2 100644
> --- a/tools/perf/util/dso.h
> +++ b/tools/perf/util/dso.h
> @@ -345,6 +345,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 725640f..9ed826c 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1707,7 +1707,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)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists