Hi Masami,

On Thu, Apr 28, 2016 at 03:37:23AM +0900, Masami Hiramatsu wrote:
> From: Masami Hiramatsu <[email protected]>
> 
> 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 <[email protected]>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
>  Changes in v5:
>   - Support old style buildid caches.
> ---
>  tools/perf/util/build-id.c |   80 
> ++++++++++++++++++++++++++++++++++----------
>  tools/perf/util/dso.h      |    5 +++
>  tools/perf/util/symbol.c   |   15 ++++++--
>  3 files changed, 76 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index b6ecf87..b035483 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,45 @@ static char *build_id__filename(const char *sbuild_id, 
> char *bf, size_t size)
>       return bf;
>  }
>  
> +static bool __is_regular_file(const char *pathname)
> +{
> +     struct stat sb;
> +     return stat(pathname, &sb) == 0 && S_ISREG(sb.st_mode);
> +}

It looks like that the is_regular_file() is already in the
util/util.c

Thanks,
Namhyung


> +
> +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;
> +
> +     /* Check if old style build_id cache */
> +     if (__is_regular_file(linkname))
> +             ret = asnprintf(&bf, size, "%s", linkname);
> +     else
> +             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 +372,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 +384,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 +405,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 +441,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 +450,22 @@ 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;
>  
> +     /* Remove old style build-id cache */
> +     if (__is_regular_file(dir_name))
> +             if (unlink(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 +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);
> 

Reply via email to