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

[Arnaldo Carvalho de Melo <acme@kernel.org>]

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);