Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753196AbcD0VX5 (ORCPT ); Wed, 27 Apr 2016 17:23:57 -0400 Received: from mail.kernel.org ([198.145.29.136]:53958 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751103AbcD0VX4 (ORCPT ); Wed, 27 Apr 2016 17:23:56 -0400 Date: Wed, 27 Apr 2016 18:23:50 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: linux-kernel@vger.kernel.org, Namhyung Kim , Peter Zijlstra , Ingo Molnar , Hemant Kumar , Ananth N Mavinakayanahalli Subject: Re: [PATCH perf/core v5 02/15] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid Message-ID: <20160427212350.GR11033@kernel.org> References: <20160427183701.23446.15293.stgit@devbox> <20160427183723.23446.47139.stgit@devbox> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160427183723.23446.47139.stgit@devbox> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4761 Lines: 140 Em Thu, Apr 28, 2016 at 03:37:23AM +0900, Masami Hiramatsu escreveu: > From: Masami Hiramatsu > > 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 existing caches are not updated until user adds > or updates the cache. Anyway, if there is the old style build-id > cache it falls back to use it. (IOW, it is backward compatible) > > Signed-off-by: Masami Hiramatsu > Signed-off-by: Masami Hiramatsu > --- > Changes in v5: > - Support old style buildid caches. > + /* Remove old style build-id cache */ > + if (__is_regular_file(dir_name)) > + if (unlink(dir_name)) > + goto out_free; Shouldn't this just fail and require the user to use --force/-f or purge first? - Arnaldo > 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 +479,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 +488,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 +516,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 +535,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 +553,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 +565,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..9463c7d 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c > @@ -1685,13 +1685,18 @@ 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); > - > + /* Try old style kallsyms cache */ > if (access(path, F_OK)) { > - pr_err("No kallsyms or vmlinux with build-id %s was found\n", > - sbuild_id); > - return NULL; > + scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s", > + buildid_dir, sbuild_id); > + > + if (access(path, F_OK)) { > + pr_err("No kallsyms or vmlinux with build-id %s was found\n", > + sbuild_id); > + return NULL; > + } > } > > return strdup(path);