Re: [perf/core PATCH v2 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

Arnaldo Carvalho de Melo
2015-02-11T14:49:08Z

Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:
> Add new buildid cache if the update target file is not cached.
> This can happen when an old binary is replaced by new one
> after caching the old one. In this case, user sees his operation
> just failed. But it does not look straight, since user just
> pass the binary "path", not "build-id".
>
> ----
> # ./perf buildid-cache --add ./perf
> (update ./perf to new binary)
> # ./perf buildid-cache --update ./perf
> ./perf wasn't in the cache

Humm, without re-reading the original motivation for the '--update'
operation I would think it was about finding all build-ids in the cache
that are for a binary with that path, remove them and insert this new
one, no?

Checking...

> #
> ----
>
> This patch adds given new binary to cache if the new binary is
> not cached. So we'll not see the above error.
>
> ----
> # ./perf buildid-cache --add ./perf
> (update ./perf to new binary)
> # ./perf buildid-cache --update ./perf
> #
> ----
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 2 ++
> tools/perf/builtin-buildid-cache.c | 6 ++++--
> tools/perf/util/build-id.c | 12 ++++++++++++
> tools/perf/util/build-id.h | 1 +
> 4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index 0294c57..6575dce 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -44,6 +44,8 @@ OPTIONS
> --update::
> Update specified file of the cache. It can be used to update kallsyms
> kernel dso to vmlinux in order to support annotation.
> + If there is no cache which has given binary's build-id, this just adds
> + new one.
> -v::
> --verbose::
> Be more verbose.
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index d929d95..e7568f5 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -255,7 +255,7 @@ static int build_id_cache__update_file(const char *filename)
> u8 build_id[BUILD_ID_SIZE];
> char sbuild_id[BUILD_ID_SIZE * 2 + 1];
>
> - int err;
> + int err = 0;
>
> if (filename__read_build_id(filename, &build_id, sizeof(build_id)) < 0) {
> pr_debug("Couldn't read a build-id in %s\n", filename);
> @@ -263,7 +263,9 @@ static int build_id_cache__update_file(const char *filename)
> }
>
> build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> - err = build_id_cache__remove_s(sbuild_id);
> + if (build_id_cache__cached(sbuild_id))
> + err = build_id_cache__remove_s(sbuild_id);
> +
> if (!err)
> err = build_id_cache__add_s(sbuild_id, filename, false, false);
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index adbc360..0bc33be 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -352,6 +352,18 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
> return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
> }
>
> +bool build_id_cache__cached(const char *sbuild_id)
> +{
> + bool ret = false;
> + char *filename = build_id__filename(sbuild_id, NULL, 0);
> +
> + if (filename && !access(filename, F_OK))
> + ret = true;
> + free(filename);
> +
> + return ret;
> +}
> +
> int build_id_cache__remove_s(const char *sbuild_id)
> {
> const size_t size = PATH_MAX;
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index 31b3c63..2a09498 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
> int perf_session__write_buildid_table(struct perf_session *session, int fd);
> int perf_session__cache_build_ids(struct perf_session *session);
>
> +bool build_id_cache__cached(const char *sbuild_id);
> int build_id_cache__add_s(const char *sbuild_id,
> const char *name, bool is_kallsyms, bool is_vdso);
> int build_id_cache__remove_s(const char *sbuild_id);